From e5fbc631ca06b40a682149b0903221e190f592aa Mon Sep 17 00:00:00 2001 From: Axel Boldt-Christmas Date: Mon, 1 Jul 2024 10:24:28 +0000 Subject: [PATCH] 8326820: Metadata artificially kept alive Reviewed-by: stefank Backport-of: 5909d54147355dd7da5786ff39ead4c15816705c --- .../share/classfile/classLoaderDataGraph.cpp | 83 ++++++++++--------- .../share/classfile/classLoaderDataGraph.hpp | 13 ++- .../share/classfile/classLoaderStats.cpp | 2 +- .../share/classfile/systemDictionary.hpp | 2 +- src/hotspot/share/prims/jvmtiEnvBase.cpp | 2 +- .../share/prims/jvmtiGetLoadedClasses.cpp | 2 +- 6 files changed, 55 insertions(+), 49 deletions(-) diff --git a/src/hotspot/share/classfile/classLoaderDataGraph.cpp b/src/hotspot/share/classfile/classLoaderDataGraph.cpp index 2046286651e..adec6dbdeee 100644 --- a/src/hotspot/share/classfile/classLoaderDataGraph.cpp +++ b/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. -template -class ClassLoaderDataGraphIteratorBase : public StackObj { +// unloading can remove entries concurrently. +// This iterator does 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. +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) { - if (keep_alive) { - assert_locked_or_safepoint(ClassLoaderDataGraph_lock); - } else { - assert_at_safepoint(); - } + ClassLoaderDataGraphIterator() : _next(ClassLoaderDataGraph::_head), _thread(Thread::current()), _hm(_thread) { + assert_locked_or_safepoint(ClassLoaderDataGraph_lock); } 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; -using ClassLoaderDataGraphIteratorNoKeepAlive = ClassLoaderDataGraphIteratorBase; - 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) { - st->print("Dictionary for "); - cld->print_value_on(st); - st->cr(); - cld->dictionary()->print_on(st); - st->cr(); + ClassLoaderDataGraphIterator iter; + while (ClassLoaderData *cld = iter.get_next()) { + if (cld->dictionary() != nullptr) { + st->print("Dictionary for "); + cld->print_value_on(st); + st->cr(); + cld->dictionary()->print_on(st); + st->cr(); + } } } void ClassLoaderDataGraph::print_table_statistics(outputStream* st) { - FOR_ALL_DICTIONARY(cld) { - ResourceMark rm; // loader_name_and_id - stringStream tempst; - tempst.print("System Dictionary for %s class loader", cld->loader_name_and_id()); - cld->dictionary()->print_table_statistics(st, tempst.freeze()); + ClassLoaderDataGraphIterator iter; + while (ClassLoaderData *cld = iter.get_next()) { + if (cld->dictionary() != nullptr) { + ResourceMark rm; // loader_name_and_id + 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(); } diff --git a/src/hotspot/share/classfile/classLoaderDataGraph.hpp b/src/hotspot/share/classfile/classLoaderDataGraph.hpp index 3de2c10850e..a79c6e21089 100644 --- a/src/hotspot/share/classfile/classLoaderDataGraph.hpp +++ b/src/hotspot/share/classfile/classLoaderDataGraph.hpp @@ -37,10 +37,10 @@ class ClassLoaderDataGraph : public AllStatic { friend class ClassLoaderDataGraphMetaspaceIterator; friend class ClassLoaderDataGraphKlassIteratorAtomic; friend class ClassLoaderDataGraphKlassIteratorStatic; - template - 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(); diff --git a/src/hotspot/share/classfile/classLoaderStats.cpp b/src/hotspot/share/classfile/classLoaderStats.cpp index f09d9968137..6bb49c3a853 100644 --- a/src/hotspot/share/classfile/classLoaderStats.cpp +++ b/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(); } diff --git a/src/hotspot/share/classfile/systemDictionary.hpp b/src/hotspot/share/classfile/systemDictionary.hpp index 9f65cb593d3..6355de9c4ce 100644 --- a/src/hotspot/share/classfile/systemDictionary.hpp +++ b/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 diff --git a/src/hotspot/share/prims/jvmtiEnvBase.cpp b/src/hotspot/share/prims/jvmtiEnvBase.cpp index dc02e8e5cf0..1a6aec4e438 100644 --- a/src/hotspot/share/prims/jvmtiEnvBase.cpp +++ b/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"); diff --git a/src/hotspot/share/prims/jvmtiGetLoadedClasses.cpp b/src/hotspot/share/prims/jvmtiGetLoadedClasses.cpp index 8d6a0736afe..c2e970bae73 100644 --- a/src/hotspot/share/prims/jvmtiGetLoadedClasses.cpp +++ b/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);