On Sat, 19 Mar 2022 19:34:23 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:
> `isInNewGen()` is throwing an NPE because "heap" is null: > > > public boolean isInNewGen() { > return ((gen != null) && (gen == ((GenCollectedHeap)heap).getGen(0))); > } > > > The call came from here: > > > } else if (isInHeap()) { > if (isInTLAB()) { > tty.print("In thread-local allocation buffer for thread ("); > getTLABThread().printThreadInfoOn(tty); > tty.print(") "); > getTLAB().printOn(tty); // includes "\n" > } else { > if (isInNewGen()) { > tty.print("In new generation "); > } else if (isInOldGen()) { > tty.print("In old generation "); > } else { > tty.print("In unknown section of Java heap"); > } > if (getGeneration() != null) { > getGeneration().printOn(tty); // does not include "\n" > } > tty.println(); > } > > > `isInHeap()` returns true if either "heap" or "gen" is non-null. If "gen" is > non-null and "heap" is null, it is not safe to call `isInNewGen()`. Yet you > can see from the code above that when `isInNewGen()` is called, the only > guarantee is that "heap" or "gen" is non-null, not both, and it turns out > that `PointerFinder.find()` only sets "gen" when the ptr is found to be in a > generation of the heap. It does not set "heap". So the logic in > `PointerFinder.find()` is not in agreement with the logic in > `PointerLocation.printOn()` w.r.t. to the setting up and meaning of the "gen" > and "heap" fields. > > The solution is pretty straight forward. Always set "heap" when the ptr is in > the heap, even if it is in a generation (in which case "gen" is also set) or > in a tlab (in which case "tlab" is also set). `isInHeap()` no longer requires > that "gen" also be set, just "heap". > > I also noticed that the printlns in the following were not being triggered: > > > if (isInNewGen()) { > tty.print("In new generation "); > } else if (isInOldGen()) { > tty.print("In old generation "); > } else { > > > This was true even though "gen" was set and was indeed either pointing to the > new gen or old gen. It turns out that the following returns a newly created > Generation object: > > ` ((GenCollectedHeap)heap).getGen(0)` > > For this reason `equals()` must be used to compare them, not ==. The > `VMObject.equals()` method is what ends up being called, and it compares the > address associated with the underlying `VMObject`. Thanks for the reviews! ------------- PR: https://git.openjdk.java.net/jdk/pull/7873