changeset 59407:b980b0247350

8244203: sun/tools/jhsdb/HeapDumpTestWithActiveProcess.java fails with NullPointerException Reviewed-by: sspitsyn, dtitov
author cjplummer
date Fri, 22 May 2020 13:29:26 -0700
parents 6d7c3a8bfab6
children d982f4e32a78
files src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/classfile/ClassLoaderData.java src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/memory/Dictionary.java src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/oops/InstanceKlass.java test/hotspot/jtreg/serviceability/sa/ClhsdbClasses.java
diffstat 4 files changed, 34 insertions(+), 6 deletions(-) [+]
line wrap: on
line diff
--- a/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/classfile/ClassLoaderData.java	Fri May 22 10:25:40 2020 -0700
+++ b/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/classfile/ClassLoaderData.java	Fri May 22 13:29:26 2020 -0700
@@ -92,7 +92,11 @@
   public Klass find(String className) {
     for (Klass l = getKlasses(); l != null; l = l.getNextLinkKlass()) {
         if (l.getName().equals(className)) {
-            return l;
+            if (l instanceof InstanceKlass && !((InstanceKlass)l).isLoaded()) {
+                return null; // don't return partially loaded classes
+            } else {
+                return l;
+            }
         }
     }
     return null;
@@ -102,14 +106,17 @@
       array klasses */
   public void classesDo(ClassLoaderDataGraph.ClassVisitor v) {
       for (Klass l = getKlasses(); l != null; l = l.getNextLinkKlass()) {
+          // Only visit InstanceKlasses that are at least in the "loaded" init_state. Otherwise
+          // the InstanceKlass won't have some required fields initialized, which can cause problems.
+          if (l instanceof InstanceKlass && !((InstanceKlass)l).isLoaded()) {
+              continue;
+          }
           v.visit(l);
       }
   }
 
   /** Iterate over all klasses in the dictionary, including initiating loader. */
   public void allEntriesDo(ClassLoaderDataGraph.ClassAndLoaderVisitor v) {
-      for (Klass l = getKlasses(); l != null; l = l.getNextLinkKlass()) {
-          dictionary().allEntriesDo(v, getClassLoader());
-      }
+      dictionary().allEntriesDo(v, getClassLoader());
   }
 }
--- a/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/memory/Dictionary.java	Fri May 22 10:25:40 2020 -0700
+++ b/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/memory/Dictionary.java	Fri May 22 13:29:26 2020 -0700
@@ -65,6 +65,11 @@
       for (DictionaryEntry probe = (DictionaryEntry) bucket(index); probe != null;
                                               probe = (DictionaryEntry) probe.next()) {
         Klass k = probe.klass();
+        // Only visit InstanceKlasses that are at least in the "loaded" init_state. Otherwise
+        // the InstanceKlass won't have some required fields initialized, which can cause problems.
+        if (k instanceof InstanceKlass && !((InstanceKlass)k).isLoaded()) {
+            continue;
+        }
         v.visit(k, loader);
       }
     }
--- a/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/oops/InstanceKlass.java	Fri May 22 10:25:40 2020 -0700
+++ b/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/oops/InstanceKlass.java	Fri May 22 13:29:26 2020 -0700
@@ -145,6 +145,18 @@
 
   public InstanceKlass(Address addr) {
     super(addr);
+
+    // If the class hasn't yet reached the "loaded" init state, then don't go any further
+    // or we'll run into problems trying to look at fields that are not yet setup.
+    // Attempted lookups of this InstanceKlass via ClassLoaderDataGraph, ClassLoaderData,
+    // and Dictionary will all refuse to return it. The main purpose of allowing this
+    // InstanceKlass to initialize is so ClassLoaderData.getKlasses() will succeed, allowing
+    // ClassLoaderData.classesDo() to iterate over all Klasses (skipping those that are
+    // not yet fully loaded).
+    if (!isLoaded()) {
+        return;
+    }
+
     if (getJavaFieldsCount() != getAllFieldsCount()) {
       // Exercise the injected field logic
       for (int i = getJavaFieldsCount(); i < getAllFieldsCount(); i++) {
--- a/test/hotspot/jtreg/serviceability/sa/ClhsdbClasses.java	Fri May 22 10:25:40 2020 -0700
+++ b/test/hotspot/jtreg/serviceability/sa/ClhsdbClasses.java	Fri May 22 13:29:26 2020 -0700
@@ -78,8 +78,12 @@
                 expStrMap.put(printCmd,
                               List.of("public class " + APP_DOT_CLASSNAME + " @" + classAddress));
                 expStrMap.put(classesCmd, List.of(
-                        APP_SLASH_CLASSNAME + " @" + classAddress,
-                        "java/lang/Class @0x"));
+                        APP_SLASH_CLASSNAME + " @" + classAddress, // check for app class at right address
+                        "java/lang/Class @0x",                // check for java/lang/Class
+                        "java/lang/Object @0x",               // check for java/lang/Object
+                        "java/lang/Cloneable @0x",            // check for an interface type
+                        "\\[Ljava/lang/String; @0x",          // check for array type
+                        "\\[J @0x", "\\[I @0x", "\\[Z @0x")); // check for array of a few pimitive types
                 test.run(theApp.getPid(), cmds, expStrMap, null);
             }
         } catch (SkippedException se) {