diff --git a/src/hotspot/share/classfile/classLoaderData.hpp b/src/hotspot/share/classfile/classLoaderData.hpp index 278beab6db6..88a318b7092 100644 --- a/src/hotspot/share/classfile/classLoaderData.hpp +++ b/src/hotspot/share/classfile/classLoaderData.hpp @@ -98,7 +98,8 @@ class ClassLoaderData : public CHeapObj<mtClass> { }; friend class ClassLoaderDataGraph; - friend class ClassLoaderDataGraphIterator; + template <bool keep_alive> + friend class ClassLoaderDataGraphIteratorBase; friend class ClassLoaderDataGraphKlassIteratorAtomic; friend class ClassLoaderDataGraphKlassIteratorStatic; friend class ClassLoaderDataGraphMetaspaceIterator; @@ -253,6 +254,7 @@ class ClassLoaderData : public CHeapObj<mtClass> { ClassLoaderMetaspace* metaspace_non_null(); inline oop class_loader() const; + inline oop class_loader_no_keepalive() const; // Returns true if this class loader data is for a loader going away. // Note that this is only safe after the GC has computed if the CLD is diff --git a/src/hotspot/share/classfile/classLoaderData.inline.hpp b/src/hotspot/share/classfile/classLoaderData.inline.hpp index 842843a8d32..3acd2327bf8 100644 --- a/src/hotspot/share/classfile/classLoaderData.inline.hpp +++ b/src/hotspot/share/classfile/classLoaderData.inline.hpp @@ -39,6 +39,12 @@ inline oop ClassLoaderData::class_loader() const { return _class_loader.resolve(); } +inline oop ClassLoaderData::class_loader_no_keepalive() const { + assert(!_unloading, "This oop is not available to unloading class loader data"); + assert(_holder.is_null() || holder_no_keepalive() != NULL , "This class loader data holder must be alive"); + return _class_loader.peek(); +} + inline bool ClassLoaderData::is_boot_class_loader_data() const { return this == _the_null_class_loader_data || class_loader() == NULL; } diff --git a/src/hotspot/share/classfile/classLoaderDataGraph.cpp b/src/hotspot/share/classfile/classLoaderDataGraph.cpp index 4cbee3ec7d8..293931790c2 100644 --- a/src/hotspot/share/classfile/classLoaderDataGraph.cpp +++ b/src/hotspot/share/classfile/classLoaderDataGraph.cpp @@ -314,18 +314,21 @@ LockedClassesDo::~LockedClassesDo() { // Iterating over the CLDG needs to be locked because // unloading can remove entries concurrently soon. -class ClassLoaderDataGraphIterator : public StackObj { +template <bool keep_alive = true> +class ClassLoaderDataGraphIteratorBase : public StackObj { ClassLoaderData* _next; Thread* _thread; HandleMark _hm; // clean up handles when this is done. - Handle _holder; NoSafepointVerifier _nsv; // No safepoints allowed in this scope // unless verifying at a safepoint. public: - ClassLoaderDataGraphIterator() : _next(ClassLoaderDataGraph::_head), _thread(Thread::current()), _hm(_thread) { - _thread = Thread::current(); - assert_locked_or_safepoint(ClassLoaderDataGraph_lock); + ClassLoaderDataGraphIteratorBase() : _next(ClassLoaderDataGraph::_head), _thread(Thread::current()), _hm(_thread) { + if (keep_alive) { + assert_locked_or_safepoint(ClassLoaderDataGraph_lock); + } else { + assert_at_safepoint(); + } } ClassLoaderData* get_next() { @@ -335,8 +338,10 @@ public: cld = cld->next(); } if (cld != NULL) { - // Keep cld that is being returned alive. - _holder = Handle(_thread, cld->holder()); + if (keep_alive) { + // Keep cld that is being returned alive. + Handle(_thread, cld->holder()); + } _next = cld->next(); } else { _next = NULL; @@ -345,6 +350,9 @@ 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()) { @@ -422,16 +430,19 @@ void ClassLoaderDataGraph::classes_unloading_do(void f(Klass* const)) { } } +void ClassLoaderDataGraph::verify_dictionary() { + ClassLoaderDataGraphIteratorNoKeepAlive iter; + while (ClassLoaderData* cld = iter.get_next()) { + if (cld->dictionary() != nullptr) { + cld->dictionary()->verify(); + } + } +} + #define FOR_ALL_DICTIONARY(X) ClassLoaderDataGraphIterator iter; \ while (ClassLoaderData* X = iter.get_next()) \ if (X->dictionary() != NULL) -void ClassLoaderDataGraph::verify_dictionary() { - FOR_ALL_DICTIONARY(cld) { - cld->dictionary()->verify(); - } -} - void ClassLoaderDataGraph::print_dictionary(outputStream* st) { FOR_ALL_DICTIONARY(cld) { st->print("Dictionary for "); @@ -648,7 +659,7 @@ Klass* ClassLoaderDataGraphKlassIteratorAtomic::next_klass() { } void ClassLoaderDataGraph::verify() { - ClassLoaderDataGraphIterator iter; + ClassLoaderDataGraphIteratorNoKeepAlive 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 fd5fdca633e..c690ac47ec9 100644 --- a/src/hotspot/share/classfile/classLoaderDataGraph.hpp +++ b/src/hotspot/share/classfile/classLoaderDataGraph.hpp @@ -37,7 +37,8 @@ class ClassLoaderDataGraph : public AllStatic { friend class ClassLoaderDataGraphMetaspaceIterator; friend class ClassLoaderDataGraphKlassIteratorAtomic; friend class ClassLoaderDataGraphKlassIteratorStatic; - friend class ClassLoaderDataGraphIterator; + template <bool keep_alive> + friend class ClassLoaderDataGraphIteratorBase; friend class VMStructs; private: // All CLDs (except the null CLD) can be reached by walking _head->_next->... diff --git a/src/hotspot/share/classfile/dictionary.cpp b/src/hotspot/share/classfile/dictionary.cpp index e6f027b332d..12d5baee159 100644 --- a/src/hotspot/share/classfile/dictionary.cpp +++ b/src/hotspot/share/classfile/dictionary.cpp @@ -633,7 +633,7 @@ void Dictionary::verify() { // class loader must be present; a null class loader is the // bootstrap loader guarantee(cld != NULL && - (cld->the_null_class_loader_data() || cld->class_loader()->is_instance()), + (cld->is_the_null_class_loader_data() || cld->class_loader_no_keepalive()->is_instance()), "checking type of class_loader"); ResourceMark rm; diff --git a/test/hotspot/jtreg/runtime/ClassUnload/UnloadTestWithVerifyDuringGC.java b/test/hotspot/jtreg/runtime/ClassUnload/UnloadTestWithVerifyDuringGC.java new file mode 100644 index 00000000000..6228a43777a --- /dev/null +++ b/test/hotspot/jtreg/runtime/ClassUnload/UnloadTestWithVerifyDuringGC.java @@ -0,0 +1,98 @@ +/* + * 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 Class unloading test with concurrent mark + * @summary Make sure that verification during gc does not prevent class unloading + * @bug 8280454 + * @requires vm.gc.G1 + * @requires vm.opt.final.ClassUnloading + * @requires vm.opt.final.ClassUnloadingWithConcurrentMark + * @modules java.base/jdk.internal.misc + * @library /test/lib + * @library classes + * @build sun.hotspot.WhiteBox test.Empty + * @run driver jdk.test.lib.helpers.ClassFileInstaller sun.hotspot.WhiteBox + * @run main/othervm -Xbootclasspath/a:. -Xmn8m -XX:+UseG1GC -XX:+VerifyDuringGC -XX:+AlwaysTenure -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI -Xlog:gc,class+unload=debug UnloadTestWithVerifyDuringGC + */ +import sun.hotspot.WhiteBox; +import jdk.test.lib.classloader.ClassUnloadCommon; + +/** + * Test that verifies that classes are unloaded using concurrent mark with G1 when they are no + * longer reachable even when -XX:+VerifyDuringGC is enabled + * + * The test creates a class loader, uses the loader to load a class and creates an instance + * of that class. The it nulls out all the references to the instance, class and class loader + * and tries to trigger class unloading using a concurrent mark. Then it verifies that the class + * is no longer loaded by the VM. + */ +public class UnloadTestWithVerifyDuringGC { + private static final WhiteBox wb = WhiteBox.getWhiteBox(); + + private static void waitUntilConcMarkFinished() throws Exception { + while (wb.g1InConcurrentMark()) { + try { + Thread.sleep(1); + } catch (InterruptedException e) { + System.out.println("Got InterruptedException while waiting for concurrent mark to finish"); + throw e; + } + } + } + + private static void triggerUnloadingWithConcurrentMark() throws Exception { + // Try to unload classes using concurrent mark. First wait for any currently running concurrent + // cycle. + waitUntilConcMarkFinished(); + wb.g1StartConcMarkCycle(); + waitUntilConcMarkFinished(); + } + + private static String className = "test.Empty"; + + public static void main(String... args) throws Exception { + ClassUnloadCommon.failIf(wb.isClassAlive(className), "is not expected to be alive yet"); + + ClassLoader cl = ClassUnloadCommon.newClassLoader(); + Class<?> c = cl.loadClass(className); + Object o = c.newInstance(); + + ClassUnloadCommon.failIf(!wb.isClassAlive(className), "should be live here"); + + String loaderName = cl.getName(); + int loadedRefcount = wb.getSymbolRefcount(loaderName); + System.out.println("Refcount of symbol " + loaderName + " is " + loadedRefcount); + + // Move everything into the old gen so that concurrent mark can unload. + wb.youngGC(); + cl = null; c = null; o = null; + triggerUnloadingWithConcurrentMark(); + + ClassUnloadCommon.failIf(wb.isClassAlive(className), "should have been unloaded"); + + int unloadedRefcount = wb.getSymbolRefcount(loaderName); + System.out.println("Refcount of symbol " + loaderName + " is " + unloadedRefcount); + ClassUnloadCommon.failIf(unloadedRefcount != (loadedRefcount - 1), "Refcount must be decremented"); + } +}