8297427: Avoid keeping class loaders alive when executing ClassLoaderStatsVMOperation

Reviewed-by: eosterlund, stefank

src/hotspot/share/classfile/classLoaderDataGraph.cpp

@@ -369,6 +369,13 @@ void ClassLoaderDataGraph::loaded_cld_do(CLDClosure* cl) {
   }
 }
 
+void ClassLoaderDataGraph::loaded_cld_do_no_keepalive(CLDClosure* cl) {
+  ClassLoaderDataGraphIteratorNoKeepAlive iter;
+  while (ClassLoaderData* cld = iter.get_next()) {
+    cl->do_cld(cld);
+  }
+}
+
 // These functions assume that the caller has locked the ClassLoaderDataGraph_lock
 // if they are not calling the function from a safepoint.
 void ClassLoaderDataGraph::classes_do(KlassClosure* klass_closure) {

src/hotspot/share/classfile/classLoaderDataGraph.hpp

@@ -74,6 +74,7 @@ class ClassLoaderDataGraph : public AllStatic {
   static void always_strong_cld_do(CLDClosure* cl);
   // Iteration through CLDG not by GC.
   static void loaded_cld_do(CLDClosure* cl);
+  static void loaded_cld_do_no_keepalive(CLDClosure* cl);
   // klass do
   // Walking classes through the ClassLoaderDataGraph include array classes.  It also includes
   // classes that are allocated but not loaded, classes that have errors, and scratch classes

src/hotspot/share/classfile/classLoaderStats.cpp

@@ -46,7 +46,10 @@ public:
 };
 
 void ClassLoaderStatsClosure::do_cld(ClassLoaderData* cld) {
-  oop cl = cld->class_loader();
+  // Class loaders are not kept alive so this closure must only be
+  // used during a safepoint.
+  assert_at_safepoint();
+  oop cl = cld->class_loader_no_keepalive();
 
   // The hashtable key is the ClassLoader oop since we want to account
   // for "real" classes and hidden classes together
@@ -63,7 +66,7 @@ void ClassLoaderStatsClosure::do_cld(ClassLoaderData* cld) {
   }
 
   if (cl != NULL) {
-    cls->_parent = java_lang_ClassLoader::parent(cl);
+    cls->_parent = java_lang_ClassLoader::parent_no_keepalive(cl);
     addEmptyParents(cls->_parent);
   }
 
@@ -149,19 +152,19 @@ void ClassLoaderStatsClosure::addEmptyParents(oop cl) {
     ClassLoaderStats* cls = _stats->put_if_absent(cl, &added);
     if (added) {
       cls->_class_loader = cl;
-      cls->_parent = java_lang_ClassLoader::parent(cl);
+      cls->_parent = java_lang_ClassLoader::parent_no_keepalive(cl);
       _total_loaders++;
     }
     assert(cls->_class_loader == cl, "Sanity");
 
-    cl = java_lang_ClassLoader::parent(cl);
+    cl = java_lang_ClassLoader::parent_no_keepalive(cl);
   }
 }
 
 
 void ClassLoaderStatsVMOperation::doit() {
   ClassLoaderStatsClosure clsc (_out);
-  ClassLoaderDataGraph::loaded_cld_do(&clsc);
+  ClassLoaderDataGraph::loaded_cld_do_no_keepalive(&clsc);
   clsc.print();
 }
 

src/hotspot/share/classfile/javaClasses.cpp

@@ -4670,6 +4670,11 @@ oop java_lang_ClassLoader::parent(oop loader) {
   return loader->obj_field(_parent_offset);
 }
 
+oop java_lang_ClassLoader::parent_no_keepalive(oop loader) {
+  assert(is_instance(loader), "loader must be oop");
+  return loader->obj_field_access<AS_NO_KEEPALIVE>(_parent_offset);
+}
+
 // Returns the name field of this class loader.  If the name field has not
 // been set, null will be returned.
 oop java_lang_ClassLoader::name(oop loader) {

src/hotspot/share/classfile/javaClasses.hpp

@@ -1476,6 +1476,7 @@ class java_lang_ClassLoader : AllStatic {
   static void release_set_loader_data(oop loader, ClassLoaderData* new_data);
 
   static oop parent(oop loader);
+  static oop parent_no_keepalive(oop loader);
   static oop name(oop loader);
   static oop nameAndId(oop loader);
   static bool isAncestor(oop loader, oop cl);

src/hotspot/share/prims/whitebox.cpp

@@ -30,6 +30,7 @@
 #include "cds/heapShared.hpp"
 #include "cds/metaspaceShared.hpp"
 #include "classfile/classLoaderDataGraph.hpp"
+#include "classfile/classLoaderStats.hpp"
 #include "classfile/javaClasses.inline.hpp"
 #include "classfile/modules.hpp"
 #include "classfile/protectionDomainCache.hpp"
@@ -1819,6 +1820,12 @@ WB_ENTRY(void, WB_ForceSafepoint(JNIEnv* env, jobject wb))
   VMThread::execute(&force_safepoint_op);
 WB_END
 
+WB_ENTRY(void, WB_ForceClassLoaderStatsSafepoint(JNIEnv* env, jobject wb))
+  nullStream dev_null;
+  ClassLoaderStatsVMOperation force_op(&dev_null);
+  VMThread::execute(&force_op);
+WB_END
+
 WB_ENTRY(jlong, WB_GetConstantPool(JNIEnv* env, jobject wb, jclass klass))
   InstanceKlass* ik = InstanceKlass::cast(java_lang_Class::as_Klass(JNIHandles::resolve(klass)));
   return (jlong) ik->constants();
@@ -2675,6 +2682,7 @@ static JNINativeMethod methods[] = {
   {CC"deflateIdleMonitors", CC"()Z",                  (void*)&WB_DeflateIdleMonitors },
   {CC"isMonitorInflated0", CC"(Ljava/lang/Object;)Z", (void*)&WB_IsMonitorInflated  },
   {CC"forceSafepoint",     CC"()V",                   (void*)&WB_ForceSafepoint     },
+  {CC"forceClassLoaderStatsSafepoint", CC"()V",       (void*)&WB_ForceClassLoaderStatsSafepoint },
   {CC"getConstantPool0",   CC"(Ljava/lang/Class;)J",  (void*)&WB_GetConstantPool    },
   {CC"getConstantPoolCacheIndexTag0", CC"()I",  (void*)&WB_GetConstantPoolCacheIndexTag},
   {CC"getConstantPoolCacheLength0", CC"(Ljava/lang/Class;)I",  (void*)&WB_GetConstantPoolCacheLength},

test/hotspot/jtreg/runtime/ClassUnload/UnloadTestDuringClassLoaderStatsVMOperation.java

@@ -0,0 +1,152 @@
+/*
+ * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test Make sure class unloading occur even if ClassLoaderStats VM operations are executed
+ * @bug 8297427
+ * @requires vm.opt.final.ClassUnloading
+ * @modules java.base/jdk.internal.misc
+ * @library /test/lib classes
+ * @build jdk.test.whitebox.WhiteBox test.Empty test.LoadInParent test.LoadInChild
+ * @run driver jdk.test.lib.helpers.ClassFileInstaller jdk.test.whitebox.WhiteBox
+ * @run main/othervm -Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI -Xlog:gc*,class+unload=debug UnloadTestDuringClassLoaderStatsVMOperation
+ */
+import java.lang.ref.Reference;
+import java.net.URLClassLoader;
+
+import jdk.test.lib.classloader.ClassUnloadCommon;
+import jdk.test.whitebox.WhiteBox;
+
+public class UnloadTestDuringClassLoaderStatsVMOperation {
+    private static final WhiteBox wb = WhiteBox.getWhiteBox();
+
+    private static String className = "test.Empty";
+    private static String parentClassName = "test.LoadInParent";
+    private static String childClassName = "test.LoadInChild";
+
+    public static void main(String args[]) throws Exception {
+        // Create a thread forcing ClassLoaderStats VM operations.
+        var clsThread = new Thread(() -> {
+            while (true) {
+                wb.forceClassLoaderStatsSafepoint();
+            }
+        });
+        clsThread.setDaemon(true);
+        clsThread.start();
+
+        // Make sure classes can be unloaded even though the class loader
+        // stats VM operation is running.
+        testClassIsUnloaded();
+        testClassLoadedInParentIsUnloaded();
+    }
+
+    public static void testClassIsUnloaded() throws Exception {
+        ClassUnloadCommon.failIf(wb.isClassAlive(className), className + " is not expected to be alive yet");
+
+        // Load a test class and verify that it gets unloaded once we do a major collection.
+        var classLoader = ClassUnloadCommon.newClassLoader();
+        var loaded = classLoader.loadClass(className);
+        var object = loaded.getDeclaredConstructor().newInstance();
+
+        ClassUnloadCommon.failIf(!wb.isClassAlive(className), className + " should be loaded and live");
+
+        // Using reachabilityFence() to ensure the object is live. If the test
+        // is run with -Xcomp and ergonomically triggered GCs occur the class
+        // could otherwise be unloaded before verified to be alive above.
+        Reference.reachabilityFence(object);
+
+        System.out.println("testClassIsUnloaded loaded klass: " + className);
+
+        // Make class unloadable.
+        classLoader = null;
+        loaded = null;
+        object = null;
+
+        // Full/Major collection should always unload classes.
+        wb.fullGC();
+        ClassUnloadCommon.failIf(wb.isClassAlive(className), className + " should have been unloaded");
+    }
+
+    public static void testClassLoadedInParentIsUnloaded() throws Exception {
+        ClassUnloadCommon.failIf(wb.isClassAlive(parentClassName), parentClassName + " is not expected to be alive yet");
+        ClassUnloadCommon.failIf(wb.isClassAlive(childClassName), childClassName + " is not expected to be alive yet");
+
+        // Create two class loaders and load a test class in the parent and
+        // verify that it gets unloaded once we do a major collection.
+        var parentClassLoader = ClassUnloadCommon.newClassLoader();
+        var childClassLoader =  new ChildURLClassLoader((URLClassLoader) parentClassLoader);
+        var loadedParent = parentClassLoader.loadClass(parentClassName);
+        var loadedChild = childClassLoader.loadClass(childClassName);
+        var parent = loadedParent.getDeclaredConstructor().newInstance();
+        var child = loadedChild.getDeclaredConstructor().newInstance();
+
+        ClassUnloadCommon.failIf(!wb.isClassAlive(parentClassName), parentClassName + " should be loaded and live");
+        ClassUnloadCommon.failIf(!wb.isClassAlive(childClassName), childClassName + " should be loaded and live");
+
+        // Using reachabilityFence() to ensure the objects are live. If the test
+        // is run with -Xcomp and ergonomically triggered GCs occur they could
+        // otherwise be unloaded before verified to be alive above.
+        Reference.reachabilityFence(parent);
+        Reference.reachabilityFence(child);
+
+        System.out.println("testClassLoadedInParentIsUnloaded loaded klass: " + loadedParent);
+        System.out.println("testClassLoadedInParentIsUnloaded loaded klass: " + loadedChild);
+
+        // Clear to allow unloading.
+        parentClassLoader = null;
+        childClassLoader = null;
+        loadedParent = null;
+        loadedChild = null;
+        parent = null;
+        child = null;
+
+        // Full/Major collection should always unload classes.
+        wb.fullGC();
+        ClassUnloadCommon.failIf(wb.isClassAlive(parentClassName), parentClassName + " should have been unloaded");
+        ClassUnloadCommon.failIf(wb.isClassAlive(childClassName), childClassName + " should have been unloaded");
+    }
+
+    static class ChildURLClassLoader extends URLClassLoader {
+        public ChildURLClassLoader(URLClassLoader parent) {
+            super("ChildURLClassLoader", parent.getURLs(), parent);
+        }
+
+        @Override
+        public Class<?> loadClass(String cn, boolean resolve) throws ClassNotFoundException {
+            synchronized (getClassLoadingLock(cn)) {
+                Class<?> c = findLoadedClass(cn);
+                if (c == null) {
+                    try {
+                        c = findClass(cn);
+                    } catch (ClassNotFoundException e) {
+                        c = getParent().loadClass(cn);
+                    }
+                }
+                if (resolve) {
+                    resolveClass(c);
+                }
+                return c;
+            }
+        }
+    }
+}

test/hotspot/jtreg/runtime/ClassUnload/classes/test/LoadInChild.java

@@ -0,0 +1,29 @@
+/*
+ * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ *
+ */
+
+package test;
+
+public class LoadInChild {
+    public String toString() { return "Load In Child"; }
+}

test/hotspot/jtreg/runtime/ClassUnload/classes/test/LoadInParent.java

@@ -0,0 +1,29 @@
+/*
+ * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ *
+ */
+
+package test;
+
+public class LoadInParent {
+    public String toString() { return "Load In Parent"; }
+}

test/lib/jdk/test/whitebox/WhiteBox.java

@@ -121,6 +121,8 @@ public class WhiteBox {
 
   public native void forceSafepoint();
 
+  public native void forceClassLoaderStatsSafepoint();
+
   private native long getConstantPool0(Class<?> aClass);
   public         long getConstantPool(Class<?> aClass) {
     Objects.requireNonNull(aClass);