8326820: Metadata artificially kept alive

Reviewed-by: eosterlund, stefank, coleenp

src/hotspot/share/classfile/classLoaderDataGraph.cpp

@@ -241,9 +241,14 @@ LockedClassesDo::~LockedClassesDo() {
 
 
 // Iterating over the CLDG needs to be locked because
-// unloading can remove entries concurrently soon.
+// unloading can remove entries concurrently.
-template <bool keep_alive = true>
+// This iterator does not keep the CLD alive.
-class ClassLoaderDataGraphIteratorBase : public StackObj {
+// Any CLD OopHandles (modules, mirrors, resolved refs)
+// resolved must be treated as no keepalive. And requires
+// that its CLD's holder is kept alive if they escape the
+// caller's safepoint or ClassLoaderDataGraph_lock
+// critical section.
+class ClassLoaderDataGraph::ClassLoaderDataGraphIterator : public StackObj {
   ClassLoaderData* _next;
   Thread*          _thread;
   HandleMark       _hm;  // clean up handles when this is done.
@@ -251,12 +256,8 @@ class ClassLoaderDataGraphIteratorBase : public StackObj {
                             // unless verifying at a safepoint.
 
 public:
-  ClassLoaderDataGraphIteratorBase() : _next(ClassLoaderDataGraph::_head), _thread(Thread::current()), _hm(_thread) {
+  ClassLoaderDataGraphIterator() : _next(ClassLoaderDataGraph::_head), _thread(Thread::current()), _hm(_thread) {
-    if (keep_alive) {
+    assert_locked_or_safepoint(ClassLoaderDataGraph_lock);
-      assert_locked_or_safepoint(ClassLoaderDataGraph_lock);
-    } else {
-      assert_at_safepoint();
-    }
   }
 
   ClassLoaderData* get_next() {
@@ -266,10 +267,6 @@ public:
       cld = cld->next();
     }
     if (cld != nullptr) {
-      if (keep_alive) {
-        // Keep cld that is being returned alive.
-        Handle(_thread, cld->holder());
-      }
       _next = cld->next();
     } else {
       _next = nullptr;
@@ -278,9 +275,6 @@ public:
   }
 };
 
-using ClassLoaderDataGraphIterator = ClassLoaderDataGraphIteratorBase<true /* keep_alive */>;
-using ClassLoaderDataGraphIteratorNoKeepAlive = ClassLoaderDataGraphIteratorBase<false /* keep_alive */>;
-
 void ClassLoaderDataGraph::loaded_cld_do(CLDClosure* cl) {
   ClassLoaderDataGraphIterator iter;
   while (ClassLoaderData* cld = iter.get_next()) {
@@ -288,13 +282,6 @@ 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) {
@@ -318,6 +305,16 @@ void ClassLoaderDataGraph::methods_do(void f(Method*)) {
   }
 }
 
+void ClassLoaderDataGraph::modules_do_keepalive(void f(ModuleEntry*)) {
+  assert_locked_or_safepoint(Module_lock);
+  ClassLoaderDataGraphIterator iter;
+  while (ClassLoaderData* cld = iter.get_next()) {
+    // Keep the holder alive.
+    (void)cld->holder();
+    cld->modules_do(f);
+  }
+}
+
 void ClassLoaderDataGraph::modules_do(void f(ModuleEntry*)) {
   assert_locked_or_safepoint(Module_lock);
   ClassLoaderDataGraphIterator iter;
@@ -334,9 +331,11 @@ void ClassLoaderDataGraph::packages_do(void f(PackageEntry*)) {
   }
 }
 
-void ClassLoaderDataGraph::loaded_classes_do(KlassClosure* klass_closure) {
+void ClassLoaderDataGraph::loaded_classes_do_keepalive(KlassClosure* klass_closure) {
   ClassLoaderDataGraphIterator iter;
   while (ClassLoaderData* cld = iter.get_next()) {
+    // Keep the holder alive.
+    (void)cld->holder();
     cld->loaded_classes_do(klass_closure);
   }
 }
@@ -346,7 +345,7 @@ void ClassLoaderDataGraph::classes_unloading_do(void f(Klass* const)) {
 }
 
 void ClassLoaderDataGraph::verify_dictionary() {
-  ClassLoaderDataGraphIteratorNoKeepAlive iter;
+  ClassLoaderDataGraphIterator iter;
   while (ClassLoaderData* cld = iter.get_next()) {
     if (cld->dictionary() != nullptr) {
       cld->dictionary()->verify();
@@ -354,26 +353,28 @@ void ClassLoaderDataGraph::verify_dictionary() {
   }
 }
 
-#define FOR_ALL_DICTIONARY(X)   ClassLoaderDataGraphIterator iter; \
-                                while (ClassLoaderData* X = iter.get_next()) \
-                                  if (X->dictionary() != nullptr)
-
 void ClassLoaderDataGraph::print_dictionary(outputStream* st) {
-  FOR_ALL_DICTIONARY(cld) {
+  ClassLoaderDataGraphIterator iter;
-    st->print("Dictionary for ");
+  while (ClassLoaderData *cld = iter.get_next()) {
-    cld->print_value_on(st);
+    if (cld->dictionary() != nullptr) {
-    st->cr();
+      st->print("Dictionary for ");
-    cld->dictionary()->print_on(st);
+      cld->print_value_on(st);
-    st->cr();
+      st->cr();
+      cld->dictionary()->print_on(st);
+      st->cr();
+    }
   }
 }
 
 void ClassLoaderDataGraph::print_table_statistics(outputStream* st) {
-  FOR_ALL_DICTIONARY(cld) {
+  ClassLoaderDataGraphIterator iter;
-    ResourceMark rm; // loader_name_and_id
+  while (ClassLoaderData *cld = iter.get_next()) {
-    stringStream tempst;
+    if (cld->dictionary() != nullptr) {
-    tempst.print("System Dictionary for %s class loader", cld->loader_name_and_id());
+      ResourceMark rm; // loader_name_and_id
-    cld->dictionary()->print_table_statistics(st, tempst.freeze());
+      stringStream tempst;
+      tempst.print("System Dictionary for %s class loader", cld->loader_name_and_id());
+      cld->dictionary()->print_table_statistics(st, tempst.freeze());
+    }
   }
 }
 
@@ -550,7 +551,7 @@ Klass* ClassLoaderDataGraphKlassIteratorAtomic::next_klass() {
 }
 
 void ClassLoaderDataGraph::verify() {
-  ClassLoaderDataGraphIteratorNoKeepAlive iter;
+  ClassLoaderDataGraphIterator iter;
   while (ClassLoaderData* cld = iter.get_next()) {
     cld->verify();
   }

src/hotspot/share/classfile/classLoaderDataGraph.hpp

@@ -37,10 +37,10 @@ class ClassLoaderDataGraph : public AllStatic {
   friend class ClassLoaderDataGraphMetaspaceIterator;
   friend class ClassLoaderDataGraphKlassIteratorAtomic;
   friend class ClassLoaderDataGraphKlassIteratorStatic;
-  template <bool keep_alive>
-  friend class ClassLoaderDataGraphIteratorBase;
   friend class VMStructs;
  private:
+  class ClassLoaderDataGraphIterator;
+
   // All CLDs (except unlinked CLDs) can be reached by walking _head->_next->...
   static ClassLoaderData* volatile _head;
 
@@ -71,8 +71,12 @@ class ClassLoaderDataGraph : public AllStatic {
   static void roots_cld_do(CLDClosure* strong, CLDClosure* weak);
   static void always_strong_cld_do(CLDClosure* cl);
   // Iteration through CLDG not by GC.
+  // All the do suffixed functions do not keep the CLD alive. Any CLD OopHandles
+  // (modules, mirrors, resolved refs) resolved must be treated as no keepalive.
+  // And requires that its CLD's holder is kept alive if they escape the
+  // caller's safepoint or ClassLoaderDataGraph_lock critical section.
+  // The do_keepalive suffixed functions will keep all CLDs alive.
   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
@@ -81,9 +85,10 @@ class ClassLoaderDataGraph : public AllStatic {
   static void classes_do(KlassClosure* klass_closure);
   static void classes_do(void f(Klass* const));
   static void methods_do(void f(Method*));
+  static void modules_do_keepalive(void f(ModuleEntry*));
   static void modules_do(void f(ModuleEntry*));
   static void packages_do(void f(PackageEntry*));
-  static void loaded_classes_do(KlassClosure* klass_closure);
+  static void loaded_classes_do_keepalive(KlassClosure* klass_closure);
   static void classes_unloading_do(void f(Klass* const));
   static bool do_unloading();
 

src/hotspot/share/classfile/classLoaderStats.cpp

@@ -165,7 +165,7 @@ void ClassLoaderStatsClosure::addEmptyParents(oop cl) {
 
 void ClassLoaderStatsVMOperation::doit() {
   ClassLoaderStatsClosure clsc (_out);
-  ClassLoaderDataGraph::loaded_cld_do_no_keepalive(&clsc);
+  ClassLoaderDataGraph::loaded_cld_do(&clsc);
   clsc.print();
 }
 

src/hotspot/share/classfile/systemDictionary.hpp

@@ -177,7 +177,7 @@ class SystemDictionary : AllStatic {
 
   static void classes_do(MetaspaceClosure* it);
   // Iterate over all methods in all klasses
-
+  // Will not keep metadata alive. See ClassLoaderDataGraph::methods_do.
   static void methods_do(void f(Method*));
 
   // Garbage collection support

src/hotspot/share/prims/jvmtiEnvBase.cpp

@@ -2339,7 +2339,7 @@ JvmtiModuleClosure::get_all_modules(JvmtiEnv* env, jint* module_count_ptr, jobje
   }
 
   // Iterate over all the modules loaded to the system.
-  ClassLoaderDataGraph::modules_do(&do_module);
+  ClassLoaderDataGraph::modules_do_keepalive(&do_module);
 
   jint len = _tbl->length();
   guarantee(len > 0, "at least one module must be present");

src/hotspot/share/prims/jvmtiGetLoadedClasses.cpp

@@ -105,7 +105,7 @@ JvmtiGetLoadedClasses::getLoadedClasses(JvmtiEnv *env, jint* classCountPtr, jcla
     // Iterate through all classes in ClassLoaderDataGraph
     // and collect them using the LoadedClassesClosure
     MutexLocker mcld(ClassLoaderDataGraph_lock);
-    ClassLoaderDataGraph::loaded_classes_do(&closure);
+    ClassLoaderDataGraph::loaded_classes_do_keepalive(&closure);
   }
 
   return closure.get_result(env, classCountPtr, classesPtr);