From 09cf5f19d76b17790ffb899aad247f821a27d46b Mon Sep 17 00:00:00 2001 From: Ioi Lam Date: Tue, 4 Jan 2022 04:52:49 +0000 Subject: [PATCH] 8278602: CDS dynamic dump may access unloaded classes Reviewed-by: coleenp, ccheung --- src/hotspot/share/cds/dumpTimeClassInfo.cpp | 2 +- src/hotspot/share/cds/dumpTimeClassInfo.hpp | 11 ++- .../share/cds/dumpTimeClassInfo.inline.hpp | 74 ++++++++++++++++ src/hotspot/share/cds/dynamicArchive.cpp | 11 ++- .../classfile/systemDictionaryShared.cpp | 8 +- .../classfile/systemDictionaryShared.hpp | 1 + .../appcds/dynamicArchive/LotsUnloadTest.java | 82 ++++++++++++++++++ .../test-classes/LotsUnloadApp.java | 84 +++++++++++++++++++ 8 files changed, 265 insertions(+), 8 deletions(-) create mode 100644 src/hotspot/share/cds/dumpTimeClassInfo.inline.hpp create mode 100644 test/hotspot/jtreg/runtime/cds/appcds/dynamicArchive/LotsUnloadTest.java create mode 100644 test/hotspot/jtreg/runtime/cds/appcds/dynamicArchive/test-classes/LotsUnloadApp.java diff --git a/src/hotspot/share/cds/dumpTimeClassInfo.cpp b/src/hotspot/share/cds/dumpTimeClassInfo.cpp index 5a9028d32255a..77225969b1a4d 100644 --- a/src/hotspot/share/cds/dumpTimeClassInfo.cpp +++ b/src/hotspot/share/cds/dumpTimeClassInfo.cpp @@ -24,7 +24,7 @@ #include "precompiled.hpp" #include "cds/archiveBuilder.hpp" -#include "cds/dumpTimeClassInfo.hpp" +#include "cds/dumpTimeClassInfo.inline.hpp" #include "classfile/classLoader.hpp" #include "classfile/classLoaderData.inline.hpp" #include "classfile/systemDictionaryShared.hpp" diff --git a/src/hotspot/share/cds/dumpTimeClassInfo.hpp b/src/hotspot/share/cds/dumpTimeClassInfo.hpp index 722849954fa47..04b0af221cfff 100644 --- a/src/hotspot/share/cds/dumpTimeClassInfo.hpp +++ b/src/hotspot/share/cds/dumpTimeClassInfo.hpp @@ -168,13 +168,15 @@ inline unsigned DumpTimeSharedClassTable_hash(InstanceKlass* const& k) { } } -class DumpTimeSharedClassTable: public ResourceHashtable< +using DumpTimeSharedClassTableBaseType = ResourceHashtable< InstanceKlass*, DumpTimeClassInfo, 15889, // prime number ResourceObj::C_HEAP, mtClassShared, - &DumpTimeSharedClassTable_hash> + &DumpTimeSharedClassTable_hash>; + +class DumpTimeSharedClassTable: public DumpTimeSharedClassTableBaseType { int _builtin_count; int _unregistered_count; @@ -194,6 +196,11 @@ class DumpTimeSharedClassTable: public ResourceHashtable< return _unregistered_count; } } + + // Overrides ResourceHashtable<>::iterate(ITER*) + template void iterate(ITER* iter) const; +private: + template class IterationHelper; }; #endif // SHARED_CDS_DUMPTIMESHAREDCLASSINFO_HPP diff --git a/src/hotspot/share/cds/dumpTimeClassInfo.inline.hpp b/src/hotspot/share/cds/dumpTimeClassInfo.inline.hpp new file mode 100644 index 0000000000000..f9dbfc7f816c1 --- /dev/null +++ b/src/hotspot/share/cds/dumpTimeClassInfo.inline.hpp @@ -0,0 +1,74 @@ + +/* + * Copyright (c) 2021, 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. + * + */ + +#ifndef SHARED_CDS_DUMPTIMESHAREDCLASSINFO_INLINE_HPP +#define SHARED_CDS_DUMPTIMESHAREDCLASSINFO_INLINE_HPP + +#include "cds/dumpTimeClassInfo.hpp" +#include "classfile/systemDictionaryShared.hpp" +#include "classfile/classLoaderData.inline.hpp" +#include "oops/instanceKlass.hpp" +#include "oops/klass.inline.hpp" +#include "runtime/safepoint.hpp" + +#if INCLUDE_CDS + +// For safety, only iterate over a class if it loader is alive. +// IterationHelper and DumpTimeSharedClassTable::iterate +// must be used only inside a safepoint, where the value of +// k->is_loader_alive() will not change. +template +class DumpTimeSharedClassTable::IterationHelper { + ITER* _iter; +public: + IterationHelper(ITER* iter) { + _iter = iter; + } + bool do_entry(InstanceKlass* k, DumpTimeClassInfo& info) { + assert(SafepointSynchronize::is_at_safepoint(), "invariant"); + assert_lock_strong(DumpTimeTable_lock); + if (k->is_loader_alive()) { + bool result = _iter->do_entry(k, info); + assert(k->is_loader_alive(), "must not change"); + return result; + } else { + if (!SystemDictionaryShared::is_excluded_class(k)) { + SystemDictionaryShared::warn_excluded(k, "Class loader not alive"); + SystemDictionaryShared::set_excluded_locked(k); + } + return true; + } + } +}; + +template +void DumpTimeSharedClassTable::iterate(ITER* iter) const { + IterationHelper helper(iter); + DumpTimeSharedClassTableBaseType::iterate(&helper); +} + +#endif // INCLUDE_CDS + +#endif // SHARED_CDS_DUMPTIMESHAREDCLASSINFO_INLINE_HPP diff --git a/src/hotspot/share/cds/dynamicArchive.cpp b/src/hotspot/share/cds/dynamicArchive.cpp index 64c95e0893309..8592e076c7f31 100644 --- a/src/hotspot/share/cds/dynamicArchive.cpp +++ b/src/hotspot/share/cds/dynamicArchive.cpp @@ -115,6 +115,12 @@ class DynamicArchiveBuilder : public ArchiveBuilder { MutexLocker ml(DumpTimeTable_lock, Mutex::_no_safepoint_check_flag); SystemDictionaryShared::check_excluded_classes(); + if (SystemDictionaryShared::is_dumptime_table_empty()) { + log_warning(cds, dynamic)("There is no class to be included in the dynamic archive."); + SystemDictionaryShared::stop_dumping(); + return; + } + // save dumptime tables SystemDictionaryShared::clone_dumptime_tables(); @@ -171,6 +177,7 @@ class DynamicArchiveBuilder : public ArchiveBuilder { assert(_num_dump_regions_used == _total_dump_regions, "must be"); verify_universe("After CDS dynamic dump"); + SystemDictionaryShared::stop_dumping(); } virtual void iterate_roots(MetaspaceClosure* it, bool is_relocating_pointers) { @@ -342,10 +349,6 @@ class VM_PopulateDynamicDumpSharedSpace: public VM_GC_Sync_Operation { VMOp_Type type() const { return VMOp_PopulateDumpSharedSpace; } void doit() { ResourceMark rm; - if (SystemDictionaryShared::is_dumptime_table_empty()) { - log_warning(cds, dynamic)("There is no class to be included in the dynamic archive."); - return; - } if (AllowArchivingWithJavaAgent) { warning("This archive was created with AllowArchivingWithJavaAgent. It should be used " "for testing purposes only and should not be used in a production environment"); diff --git a/src/hotspot/share/classfile/systemDictionaryShared.cpp b/src/hotspot/share/classfile/systemDictionaryShared.cpp index 3e4bd3cfb34f4..6272503fe934b 100644 --- a/src/hotspot/share/classfile/systemDictionaryShared.cpp +++ b/src/hotspot/share/classfile/systemDictionaryShared.cpp @@ -31,7 +31,7 @@ #include "cds/filemap.hpp" #include "cds/heapShared.hpp" #include "cds/cdsProtectionDomain.hpp" -#include "cds/dumpTimeClassInfo.hpp" +#include "cds/dumpTimeClassInfo.inline.hpp" #include "cds/metaspaceShared.hpp" #include "cds/runTimeClassInfo.hpp" #include "classfile/classFileStream.hpp" @@ -187,6 +187,11 @@ void SystemDictionaryShared::start_dumping() { _dump_in_progress = true; } +void SystemDictionaryShared::stop_dumping() { + assert_lock_strong(DumpTimeTable_lock); + _dump_in_progress = false; +} + DumpTimeClassInfo* SystemDictionaryShared::find_or_allocate_info_for(InstanceKlass* k) { MutexLocker ml(DumpTimeTable_lock, Mutex::_no_safepoint_check_flag); return find_or_allocate_info_for_locked(k); @@ -1528,6 +1533,7 @@ void SystemDictionaryShared::print_table_statistics(outputStream* st) { } bool SystemDictionaryShared::is_dumptime_table_empty() { + assert_lock_strong(DumpTimeTable_lock); if (_dumptime_table == NULL) { return true; } diff --git a/src/hotspot/share/classfile/systemDictionaryShared.hpp b/src/hotspot/share/classfile/systemDictionaryShared.hpp index bbd76c8d7b75f..a05484b429593 100644 --- a/src/hotspot/share/classfile/systemDictionaryShared.hpp +++ b/src/hotspot/share/classfile/systemDictionaryShared.hpp @@ -300,6 +300,7 @@ class SystemDictionaryShared: public SystemDictionary { static void print_table_statistics(outputStream* st) NOT_CDS_RETURN; static bool is_dumptime_table_empty() NOT_CDS_RETURN_(true); static void start_dumping() NOT_CDS_RETURN; + static void stop_dumping() NOT_CDS_RETURN; static bool is_supported_invokedynamic(BootstrapInfo* bsi) NOT_CDS_RETURN_(false); DEBUG_ONLY(static bool no_class_loading_should_happen() {return _no_class_loading_should_happen;}) diff --git a/test/hotspot/jtreg/runtime/cds/appcds/dynamicArchive/LotsUnloadTest.java b/test/hotspot/jtreg/runtime/cds/appcds/dynamicArchive/LotsUnloadTest.java new file mode 100644 index 0000000000000..7cde5683c22ef --- /dev/null +++ b/test/hotspot/jtreg/runtime/cds/appcds/dynamicArchive/LotsUnloadTest.java @@ -0,0 +1,82 @@ +/* + * Copyright (c) 2021, 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 + * @bug 8278602 + * @summary Lots of classes being unloaded while we try to dump a dynamic archive + * @requires vm.cds + * @library /test/lib /test/hotspot/jtreg/runtime/cds/appcds + * /test/hotspot/jtreg/runtime/cds/appcds/dynamicArchive/test-classes + * @build sun.hotspot.WhiteBox + * @build LotsUnloadApp + * @run driver jdk.test.lib.helpers.ClassFileInstaller sun.hotspot.WhiteBox + * @run driver jdk.test.lib.helpers.ClassFileInstaller -jar LotsUnloadApp.jar LotsUnloadApp DefinedAsHiddenKlass + * @run main/othervm -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI -Xbootclasspath/a:. LotsUnloadTest + */ + +// Note: for https://bugs.openjdk.java.net/browse/JDK-8278602, this test case does NOT +// reliably reproduce the problem. Reproduction requires patching ZGC. Please see +// the bug report for instructions. +// +// This test case is included so that it may find a similar bug under stress conditions +// in the CI runs. +import jdk.test.lib.helpers.ClassFileInstaller; + +public class LotsUnloadTest extends DynamicArchiveTestBase { + public static void main(String[] args) throws Exception { + runTest(LotsUnloadTest::test); + } + + static void test() throws Exception { + String topArchiveName = getNewArchiveName(); + String appJar = ClassFileInstaller.getJarPath("LotsUnloadApp.jar"); + String mainClass = "LotsUnloadApp"; + String logging; + + if (Boolean.getBoolean("verbose.LotsUnloadTest")) { + // class+unload logs may change GC timing and cause the bug to be + // less reproducible. + logging = "-Xlog:cds,class+unload"; + } else { + logging = "-Xlog:cds"; + } + + dump(topArchiveName, + logging, + "-Xmx64m", "-Xms32m", + "-cp", appJar, mainClass) + .assertNormalExit(output -> { + output.shouldHaveExitValue(0); + }); + + run(topArchiveName, + logging, + "-Xmx64m", "-Xms32m", + "-cp", appJar, mainClass) + .assertNormalExit(output -> { + output.shouldHaveExitValue(0); + }); + } +} diff --git a/test/hotspot/jtreg/runtime/cds/appcds/dynamicArchive/test-classes/LotsUnloadApp.java b/test/hotspot/jtreg/runtime/cds/appcds/dynamicArchive/test-classes/LotsUnloadApp.java new file mode 100644 index 0000000000000..8ac9275af2c0f --- /dev/null +++ b/test/hotspot/jtreg/runtime/cds/appcds/dynamicArchive/test-classes/LotsUnloadApp.java @@ -0,0 +1,84 @@ +/* + * Copyright (c) 2021, 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. + * + */ + +import java.lang.invoke.MethodType; +import java.lang.invoke.MethodHandles; +import java.lang.invoke.MethodHandles.Lookup; +import static java.lang.invoke.MethodHandles.Lookup.ClassOption.*; + +public class LotsUnloadApp implements Runnable { + static byte[] classdata; + static int exitAfterNumClasses = 1024; + + public static void main(String args[]) throws Throwable { + String resname = DefinedAsHiddenKlass.class.getName() + ".class"; + classdata = LotsUnloadApp.class.getClassLoader().getResourceAsStream(resname).readAllBytes(); + + int numThreads = 4; + try { + numThreads = Integer.parseInt(args[0]); + } catch (Throwable t) {} + + try { + exitAfterNumClasses = Integer.parseInt(args[1]); + } catch (Throwable t) {} + + for (int i = 0; i < numThreads; i++) { + Thread t = new Thread(new LotsUnloadApp()); + t.start(); + } + } + + public void run() { + while (true) { + try { + Lookup lookup = MethodHandles.lookup(); + Class cl = lookup.defineHiddenClass(classdata, false, NESTMATE).lookupClass(); + cl.newInstance(); + add(); + } catch (Throwable t) { + t.printStackTrace(); + } + } + } + + static int n; + static synchronized void add() { + n++; + if (n >= exitAfterNumClasses) { + System.exit(0); + } + } +} + +class DefinedAsHiddenKlass { + // ZGC region size is always a multiple of 2MB on x64. + // Make this slightly smaller than that. + static byte[] array = new byte[2 * 1024 * 1024 - 8 * 1024]; + static String x; + public DefinedAsHiddenKlass() { + // This will generate some lambda forms hidden classes for string concat. + x = "array size is " + array.length + " bytes "; + } +}