[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Integrated: 8281853 serviceability/sa/ClhsdbThreadContext.java failed with NullPointerException: Can
From:       Chris Plummer <cjplummer () openjdk ! java ! net>
Date:       2022-03-29 18:42:49
Message-ID: tXEmTIghdY_JsMla5SCwv_pwBnO6_xRyXBuXaMlnaA4=.d28fed09-f7eb-48bc-af79-401182949bb5 () github ! com
[Download RAW message or body]

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

This pull request has now been integrated.

Changeset: 2fef5d4a
Author:    Chris Plummer <cjplummer@openjdk.org>
URL:       https://git.openjdk.java.net/jdk/commit/2fef5d4a334fd67b5e2a8f342cd7a5143830ddf1
                
Stats:     5 lines in 2 files changed: 1 ins; 1 del; 3 mod

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

Reviewed-by: kevinw, sspitsyn

-------------

PR: https://git.openjdk.java.net/jdk/pull/7873


[prev in list] [next in list] [prev in thread] [next in thread]