8236901: 8232759 missed a test case

Use jcmd GC.class-histogram because it also works for verifying that the classes are loaded.

Reviewed-by: dholmes, mseledtsov, iklam

src/hotspot/share/memory/heapInspection.cpp

@@ -28,6 +28,8 @@
 #include "classfile/moduleEntry.hpp"
 #include "classfile/systemDictionary.hpp"
 #include "gc/shared/collectedHeap.hpp"
+#include "logging/log.hpp"
+#include "logging/logTag.hpp"
 #include "memory/heapInspection.hpp"
 #include "memory/resourceArea.hpp"
 #include "memory/universe.hpp"
@@ -512,14 +514,14 @@ size_t HeapInspection::populate_table(KlassInfoTable* cit, BoolObjectClosure *fi
 void HeapInspection::heap_inspection(outputStream* st) {
   ResourceMark rm;
 
-  KlassInfoTable cit(true);
+  KlassInfoTable cit(false);
   if (!cit.allocation_failed()) {
     // populate table with object allocation info
     size_t missed_count = populate_table(&cit);
     if (missed_count != 0) {
-      st->print_cr("WARNING: Ran out of C-heap; undercounted " SIZE_FORMAT
-                   " total instances in data below",
-                   missed_count);
+      log_info(gc, classhisto)("WARNING: Ran out of C-heap; undercounted " SIZE_FORMAT
+                               " total instances in data below",
+                               missed_count);
     }
 
     // Sort and print klass instance info

src/hotspot/share/prims/whitebox.cpp

@@ -161,24 +161,23 @@ WB_END
 
 class WBIsKlassAliveClosure : public LockedClassesDo {
     Symbol* _name;
-    bool _found;
+    int _count;
 public:
-    WBIsKlassAliveClosure(Symbol* name) : _name(name), _found(false) {}
+    WBIsKlassAliveClosure(Symbol* name) : _name(name), _count(0) {}
 
     void do_klass(Klass* k) {
-      if (_found) return;
       Symbol* ksym = k->name();
       if (ksym->fast_compare(_name) == 0) {
-        _found = true;
+        _count++;
       }
     }
 
-    bool found() const {
-        return _found;
+    int count() const {
+        return _count;
     }
 };
 
-WB_ENTRY(jboolean, WB_IsClassAlive(JNIEnv* env, jobject target, jstring name))
+WB_ENTRY(jint, WB_CountAliveClasses(JNIEnv* env, jobject target, jstring name))
   oop h_name = JNIHandles::resolve(name);
   if (h_name == NULL) return false;
   Symbol* sym = java_lang_String::as_symbol(h_name);
@@ -187,7 +186,8 @@ WB_ENTRY(jboolean, WB_IsClassAlive(JNIEnv* env, jobject target, jstring name))
   WBIsKlassAliveClosure closure(sym);
   ClassLoaderDataGraph::classes_do(&closure);
 
-  return closure.found();
+  // Return the count of alive classes with this name.
+  return closure.count();
 WB_END
 
 WB_ENTRY(jint, WB_GetSymbolRefcount(JNIEnv* env, jobject unused, jstring name))
@@ -2218,7 +2218,7 @@ static JNINativeMethod methods[] = {
   {CC"getVMLargePageSize",               CC"()J",                   (void*)&WB_GetVMLargePageSize},
   {CC"getHeapSpaceAlignment",            CC"()J",                   (void*)&WB_GetHeapSpaceAlignment},
   {CC"getHeapAlignment",                 CC"()J",                   (void*)&WB_GetHeapAlignment},
-  {CC"isClassAlive0",                    CC"(Ljava/lang/String;)Z", (void*)&WB_IsClassAlive      },
+  {CC"countAliveClasses0",               CC"(Ljava/lang/String;)I", (void*)&WB_CountAliveClasses },
   {CC"getSymbolRefcount",                CC"(Ljava/lang/String;)I", (void*)&WB_GetSymbolRefcount },
   {CC"parseCommandLine0",
       CC"(Ljava/lang/String;C[Lsun/hotspot/parser/DiagnosticCommand;)[Ljava/lang/Object;",

test/hotspot/jtreg/ProblemList.txt

@@ -93,7 +93,6 @@ gc/metaspace/CompressedClassSpaceSizeInJmapHeap.java 8193639 solaris-all
 
 runtime/jni/terminatedThread/TestTerminatedThread.java 8219652 aix-ppc64
 runtime/ReservedStack/ReservedStackTest.java 8231031 generic-all
-runtime/Metaspace/DefineClass.java 8236901 generic-all
 
 #############################################################################
 

test/hotspot/jtreg/runtime/Metaspace/DefineClass.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2020, Oracle and/or its affiliates. All rights reserved.
  * Copyright (c) 2017 SAP SE. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
@@ -29,14 +29,21 @@
  * @summary Failures during class definition can lead to memory leaks in metaspace
  * @requires vm.opt.final.ClassUnloading
  * @library /test/lib
- * @run main/othervm test.DefineClass defineClass
- * @run main/othervm test.DefineClass defineSystemClass
- * @run main/othervm -XX:+AllowParallelDefineClass
-                     test.DefineClass defineClassParallel
- * @run main/othervm -XX:-AllowParallelDefineClass
-                     test.DefineClass defineClassParallel
- * @run main/othervm -Djdk.attach.allowAttachSelf test.DefineClass redefineClass
- * @run main/othervm -Djdk.attach.allowAttachSelf test.DefineClass redefineClassWithError
+ * @build sun.hotspot.WhiteBox
+ * @run main ClassFileInstaller sun.hotspot.WhiteBox
+ *                              sun.hotspot.WhiteBox$WhiteBoxPermission
+ * @run main/othervm -Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI test.DefineClass defineClass
+ * @run main/othervm -Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI test.DefineClass defineSystemClass
+ * @run main/othervm -Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI
+ *                   -XX:+AllowParallelDefineClass
+ *                   test.DefineClass defineClassParallel
+ * @run main/othervm -Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI
+ *                   -XX:-AllowParallelDefineClass
+ *                   test.DefineClass defineClassParallel
+ * @run main/othervm -Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI
+ *                   -Djdk.attach.allowAttachSelf test.DefineClass redefineClass
+ * @run main/othervm -Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI
+ *                   -Djdk.attach.allowAttachSelf test.DefineClass redefineClassWithError
  * @author volker.simonis@gmail.com
  */
 
@@ -48,20 +55,16 @@
 import java.io.InputStream;
 import java.lang.instrument.ClassDefinition;
 import java.lang.instrument.Instrumentation;
-import java.lang.management.ManagementFactory;
-import java.util.Scanner;
 import java.util.concurrent.CountDownLatch;
 import java.util.jar.Attributes;
 import java.util.jar.JarEntry;
 import java.util.jar.JarOutputStream;
 import java.util.jar.Manifest;
 
-import javax.management.MBeanServer;
-import javax.management.ObjectName;
-
 import com.sun.tools.attach.VirtualMachine;
 
 import jdk.test.lib.process.ProcessTools;
+import sun.hotspot.WhiteBox;
 
 public class DefineClass {
 
@@ -205,35 +208,10 @@ private static void replaceString(byte[] buf, String name, int index) {
         }
     }
 
-    private static MBeanServer mbserver = ManagementFactory.getPlatformMBeanServer();
-
-    private static int getClassStats(String pattern) {
-        try {
-            ObjectName diagCmd = new ObjectName("com.sun.management:type=DiagnosticCommand");
+    public static WhiteBox wb = WhiteBox.getWhiteBox();
 
-            String result = (String)mbserver.invoke(diagCmd , "gcClassStats" , new Object[] { null }, new String[] {String[].class.getName()});
-            int count = 0;
-            try (Scanner s = new Scanner(result)) {
-                if (s.hasNextLine()) {
-                    System.out.println(s.nextLine());
-                }
-                while (s.hasNextLine()) {
-                    String l = s.nextLine();
-                    if (l.endsWith(pattern)) {
-                        count++;
-                        System.out.println(l);
-                    }
-                }
-            }
-            return count;
-        }
-        catch (Exception e) {
-            throw new RuntimeException("Test failed because we can't read the class statistics!", e);
-        }
-    }
-
-    private static void printClassStats(int expectedCount, boolean reportError) {
-        int count = getClassStats("DefineClass");
+    private static void checkClasses(int expectedCount, boolean reportError) {
+        int count = wb.countAliveClasses("test.DefineClass");
         String res = "Should have " + expectedCount +
                      " DefineClass instances and we have: " + count;
         System.out.println(res);
@@ -244,6 +222,21 @@ private static void printClassStats(int expectedCount, boolean reportError) {
 
     public static final int ITERATIONS = 10;
 
+    private static void checkClassesAfterGC(int expectedCount) {
+        // The first System.gc() doesn't clean metaspaces but triggers cleaning
+        // for the next safepoint.
+        // In the future the ServiceThread may clean metaspaces, but this loop
+        // should give it enough time to run, when that is changed.
+        // We might need to revisit this test though.
+        for (int i = 0; i < ITERATIONS; i++) {
+            System.gc();
+            System.out.println("System.gc()");
+            // Break if the GC has cleaned metaspace before iterations.
+            if (wb.countAliveClasses("test.DefineClass") == expectedCount) break;
+        }
+        checkClasses(expectedCount, true);
+    }
+
     public static void main(String[] args) throws Exception {
         String myName = DefineClass.class.getName();
         byte[] buf = getBytecodes(myName.substring(myName.lastIndexOf(".") + 1));
@@ -265,11 +258,10 @@ public static void main(String[] args) throws Exception {
             // We expect to have two instances of DefineClass here: the initial version in which we are
             // executing and another version which was loaded into our own classloader 'MyClassLoader'.
             // All the subsequent attempts to reload DefineClass into our 'MyClassLoader' should have failed.
-            printClassStats(2, false);
-            System.gc();
-            System.out.println("System.gc()");
-            // At least after System.gc() the failed loading attempts should leave no instances around!
-            printClassStats(2, true);
+            // The ClassLoaderDataGraph has the failed instances recorded at least until the next GC.
+            checkClasses(2, false);
+            // At least after some System.gc() the failed loading attempts should leave no instances around!
+            checkClassesAfterGC(2);
         }
         else if ("defineSystemClass".equals(args[0])) {
             MyClassLoader cl = new MyClassLoader();
@@ -293,11 +285,9 @@ else if ("defineSystemClass".equals(args[0])) {
             }
             // We expect to stay with one (the initial) instances of DefineClass.
             // All the subsequent attempts to reload DefineClass into the 'java' package should have failed.
-            printClassStats(1, false);
-            System.gc();
-            System.out.println("System.gc()");
-            // At least after System.gc() the failed loading attempts should leave no instances around!
-            printClassStats(1, true);
+            // The ClassLoaderDataGraph has the failed instances recorded at least until the next GC.
+            checkClasses(1, false);
+            checkClassesAfterGC(1);
         }
         else if ("defineClassParallel".equals(args[0])) {
             MyParallelClassLoader pcl = new MyParallelClassLoader();
@@ -315,11 +305,9 @@ else if ("defineClassParallel".equals(args[0])) {
             System.out.print("Counted " + pcl.getLinkageErrors() + " LinkageErrors ");
             System.out.println(pcl.getLinkageErrors() == 0 ?
                     "" : "(use -XX:+AllowParallelDefineClass to avoid this)");
-            System.gc();
-            System.out.println("System.gc()");
             // After System.gc() we expect to remain with two instances: one is the initial version which is
             // kept alive by this main method and another one in the parallel class loader.
-            printClassStats(2, true);
+            checkClassesAfterGC(2);
         }
         else if ("redefineClass".equals(args[0])) {
             loadInstrumentationAgent(myName, buf);
@@ -337,17 +325,15 @@ else if ("redefineClass".equals(args[0])) {
             }
             // We expect to have one instance for each redefinition because they are all kept alive by an activation
             // plus the initial version which is kept active by this main method.
-            printClassStats(iterations + 1, false);
+            checkClasses(iterations + 1, true);
             stop.countDown(); // Let all threads leave the DefineClass.getID() activation..
             // ..and wait until really all of them returned from DefineClass.getID()
             for (int i = 0; i < iterations; i++) {
                 threads[i].join();
             }
-            System.gc();
-            System.out.println("System.gc()");
             // After System.gc() we expect to remain with two instances: one is the initial version which is
             // kept alive by this main method and another one which is the latest redefined version.
-            printClassStats(2, true);
+            checkClassesAfterGC(2);
         }
         else if ("redefineClassWithError".equals(args[0])) {
             loadInstrumentationAgent(myName, buf);
@@ -365,11 +351,10 @@ else if ("redefineClassWithError".equals(args[0])) {
             }
             // We expect just a single DefineClass instance because failed redefinitions should
             // leave no garbage around.
-            printClassStats(1, false);
-            System.gc();
-            System.out.println("System.gc()");
+            // The ClassLoaderDataGraph has the failed instances recorded at least until the next GC.
+            checkClasses(1, false);
             // At least after a System.gc() we should definitely stay with a single instance!
-            printClassStats(1, true);
+            checkClassesAfterGC(1);
         }
     }
 }

test/lib/sun/hotspot/WhiteBox.java

@@ -99,10 +99,15 @@ public         long getObjectSize(Object o) {
 
   // Runtime
   // Make sure class name is in the correct format
+  public int countAliveClasses(String name) {
+    return countAliveClasses0(name.replace('.', '/'));
+  }
+  private native int countAliveClasses0(String name);
+
   public boolean isClassAlive(String name) {
-    return isClassAlive0(name.replace('.', '/'));
+    return countAliveClasses(name) != 0;
   }
-  private native boolean isClassAlive0(String name);
+
   public  native int getSymbolRefcount(String name);
 
   private native boolean isMonitorInflated0(Object obj);