RFR: 8281853 serviceability/sa/ClhsdbThreadContext.java failed with NullPointerException: Cannot invoke "sun.jvm.hotspot.gc.shared.GenCollectedHeap.getGen(int)" because "this.heap" is null

[Chris Plummer]

On Sat, 19 Mar 2022 19:34:23 GMT, Chris Plummer <cjplummer at 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