From e9b04293e3ef602882affa158f7d355de5b80412 Mon Sep 17 00:00:00 2001 From: Coleen Phillimore Date: Wed, 24 Jan 2018 11:33:18 -0500 Subject: [PATCH] 8175249: VMThread::run fails in VerifyBeforeExit : Universe::verify Protection domain package access cache needs to be walked in unloading Reviewed-by: dholmes, jiangli, zgu, kbarrett --- .../share/classfile/classLoaderData.cpp | 4 +- src/hotspot/share/classfile/dictionary.cpp | 44 +++++++++- src/hotspot/share/classfile/dictionary.hpp | 4 +- .../share/classfile/protectionDomainCache.hpp | 1 + .../runtime/Dictionary/ClassForName.java | 47 ++++++++++ .../Dictionary/ProtectionDomainCacheTest.java | 87 +++++++++++++++++++ .../jtreg/runtime/Dictionary/test.policy | 6 ++ 7 files changed, 189 insertions(+), 4 deletions(-) create mode 100644 test/hotspot/jtreg/runtime/Dictionary/ClassForName.java create mode 100644 test/hotspot/jtreg/runtime/Dictionary/ProtectionDomainCacheTest.java create mode 100644 test/hotspot/jtreg/runtime/Dictionary/test.policy diff --git a/src/hotspot/share/classfile/classLoaderData.cpp b/src/hotspot/share/classfile/classLoaderData.cpp index 29801d2033b..e6b7cd0c870 100644 --- a/src/hotspot/share/classfile/classLoaderData.cpp +++ b/src/hotspot/share/classfile/classLoaderData.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012, 2017, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2012, 2018, 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 @@ -1292,7 +1292,7 @@ bool ClassLoaderDataGraph::do_unloading(BoolObjectClosure* is_alive_closure, // Remove entries in the dictionary of live class loader that have // initiated loading classes in a dead class loader. if (data->dictionary() != NULL) { - data->dictionary()->do_unloading(); + data->dictionary()->do_unloading(is_alive_closure); } // Walk a ModuleEntry's reads, and a PackageEntry's exports // lists to determine if there are modules on those lists that are now diff --git a/src/hotspot/share/classfile/dictionary.cpp b/src/hotspot/share/classfile/dictionary.cpp index 8a4a924a7cd..2f0e08934e8 100644 --- a/src/hotspot/share/classfile/dictionary.cpp +++ b/src/hotspot/share/classfile/dictionary.cpp @@ -212,8 +212,44 @@ void DictionaryEntry::add_protection_domain(Dictionary* dict, Handle protection_ } } +// During class loading we may have cached a protection domain that has +// since been unreferenced, so this entry should be cleared. +void Dictionary::clean_cached_protection_domains(BoolObjectClosure* is_alive, DictionaryEntry* probe) { + assert_locked_or_safepoint(SystemDictionary_lock); -void Dictionary::do_unloading() { + ProtectionDomainEntry* current = probe->pd_set(); + ProtectionDomainEntry* prev = NULL; + while (current != NULL) { + if (!is_alive->do_object_b(current->object_no_keepalive())) { + LogTarget(Debug, protectiondomain) lt; + if (lt.is_enabled()) { + ResourceMark rm; + // Print out trace information + LogStream ls(lt); + ls.print_cr("PD in set is not alive:"); + ls.print("class loader: "); loader_data()->class_loader()->print_value_on(&ls); + ls.print(" protection domain: "); current->object_no_keepalive()->print_value_on(&ls); + ls.print(" loading: "); probe->instance_klass()->print_value_on(&ls); + ls.cr(); + } + if (probe->pd_set() == current) { + probe->set_pd_set(current->next()); + } else { + assert(prev != NULL, "should be set by alive entry"); + prev->set_next(current->next()); + } + ProtectionDomainEntry* to_delete = current; + current = current->next(); + delete to_delete; + } else { + prev = current; + current = current->next(); + } + } +} + + +void Dictionary::do_unloading(BoolObjectClosure* is_alive) { assert(SafepointSynchronize::is_at_safepoint(), "must be at safepoint"); // The NULL class loader doesn't initiate loading classes from other class loaders @@ -239,6 +275,8 @@ void Dictionary::do_unloading() { free_entry(probe); continue; } + // Clean pd_set + clean_cached_protection_domains(is_alive, probe); p = probe->next_addr(); } } @@ -412,6 +450,10 @@ void Dictionary::add_protection_domain(int index, unsigned int hash, entry->add_protection_domain(this, protection_domain); +#ifdef ASSERT + assert(loader_data() != ClassLoaderData::the_null_class_loader_data(), "doesn't make sense"); +#endif + assert(entry->contains_protection_domain(protection_domain()), "now protection domain should be present"); } diff --git a/src/hotspot/share/classfile/dictionary.hpp b/src/hotspot/share/classfile/dictionary.hpp index 5d3d4f523ad..36e34d14eef 100644 --- a/src/hotspot/share/classfile/dictionary.hpp +++ b/src/hotspot/share/classfile/dictionary.hpp @@ -53,6 +53,8 @@ class Dictionary : public Hashtable { DictionaryEntry* get_entry(int index, unsigned int hash, Symbol* name); + void clean_cached_protection_domains(BoolObjectClosure* is_alive, DictionaryEntry* probe); + protected: static size_t entry_size(); public: @@ -84,7 +86,7 @@ public: void remove_classes_in_error_state(); // Unload classes whose defining loaders are unloaded - void do_unloading(); + void do_unloading(BoolObjectClosure* is_alive); // Protection domains InstanceKlass* find(unsigned int hash, Symbol* name, Handle protection_domain); diff --git a/src/hotspot/share/classfile/protectionDomainCache.hpp b/src/hotspot/share/classfile/protectionDomainCache.hpp index d5046c2df76..f2901b8a43e 100644 --- a/src/hotspot/share/classfile/protectionDomainCache.hpp +++ b/src/hotspot/share/classfile/protectionDomainCache.hpp @@ -113,6 +113,7 @@ class ProtectionDomainEntry :public CHeapObj { } ProtectionDomainEntry* next() { return _next; } + void set_next(ProtectionDomainEntry* entry) { _next = entry; } oop object(); oop object_no_keepalive(); }; diff --git a/test/hotspot/jtreg/runtime/Dictionary/ClassForName.java b/test/hotspot/jtreg/runtime/Dictionary/ClassForName.java new file mode 100644 index 00000000000..d7acb15cdd4 --- /dev/null +++ b/test/hotspot/jtreg/runtime/Dictionary/ClassForName.java @@ -0,0 +1,47 @@ +/* + * Copyright (c) 2018, 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.net.URLClassLoader; + +/* + * This class is loaded by the custom URLClassLoader, and then calls + * Class.forName() with the protection domain for the checkPackageAccess + * call created from the code source jar file. + */ +public class ClassForName { + static { + if (!(ClassForName.class.getClassLoader() instanceof URLClassLoader)) { + throw new RuntimeException("Supposed to be loaded by URLClassLoader"); + } + } + + public ClassForName() { + try { + // class_loader = URLClassLoader, protection_domain = ClassForName.getProtectionDomain() + Class.forName(java.util.List.class.getName(), false, + ClassLoader.getSystemClassLoader()); + } catch (Throwable e) { + e.printStackTrace(); + } + } +} diff --git a/test/hotspot/jtreg/runtime/Dictionary/ProtectionDomainCacheTest.java b/test/hotspot/jtreg/runtime/Dictionary/ProtectionDomainCacheTest.java new file mode 100644 index 00000000000..2ec1df1ef6a --- /dev/null +++ b/test/hotspot/jtreg/runtime/Dictionary/ProtectionDomainCacheTest.java @@ -0,0 +1,87 @@ +/* + * Copyright (c) 2018, 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 8151486 + * @summary Call Class.forName() on the system classloader from a class loaded + * from a custom classloader, using the current class's protection domain. + * @library /test/jdk/lib/testlibrary + * @build jdk.testlibrary.Utils JarUtils + * @build ClassForName ProtectionDomainCacheTest + * @run main/othervm/policy=test.policy -XX:+UnlockDiagnosticVMOptions -XX:VerifySubSet=dictionary -XX:+VerifyAfterGC -Xlog:gc+verify=debug,protectiondomain=trace,class+unload:gc.log -Djava.security.manager ProtectionDomainCacheTest + */ + +import java.net.URL; +import java.net.URLClassLoader; +import java.nio.file.FileSystems; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.List; +import jdk.testlibrary.Utils; + +/* + * Create .jar, load ClassForName from .jar using a URLClassLoader + */ +public class ProtectionDomainCacheTest { + private static final long TIMEOUT = (long)(5000.0 * Utils.TIMEOUT_FACTOR); + private static final String TESTCLASSES = System.getProperty("test.classes", "."); + private static final String CLASSFILENAME = "ClassForName.class"; + + // Use a new classloader to load the ClassForName class. + public static void loadAndRun(Path jarFilePath) + throws Exception { + ClassLoader classLoader = new URLClassLoader( + new URL[]{jarFilePath.toUri().toURL()}) { + @Override public String toString() { return "LeakedClassLoader"; } + }; + + Class loadClass = Class.forName("ClassForName", true, classLoader); + loadClass.newInstance(); + + System.out.println("returning : " + classLoader); + } + + public static void main(final String[] args) throws Exception { + // Create a temporary .jar file containing ClassForName.class + Path testClassesDir = Paths.get(TESTCLASSES); + Path jarFilePath = Files.createTempFile("cfn", ".jar"); + JarUtils.createJarFile(jarFilePath, testClassesDir, CLASSFILENAME); + jarFilePath.toFile().deleteOnExit(); + + // Remove the ClassForName.class file that jtreg built, to make sure + // we're loading from the tmp .jar + Path classFile = FileSystems.getDefault().getPath(TESTCLASSES, + CLASSFILENAME); + Files.delete(classFile); + + loadAndRun(jarFilePath); + + // Give the GC a chance to unload protection domains + for (int i = 0; i < 100; i++) { + System.gc(); + } + System.out.println("All Classloaders and protection domain cache entries successfully unloaded"); + } +} diff --git a/test/hotspot/jtreg/runtime/Dictionary/test.policy b/test/hotspot/jtreg/runtime/Dictionary/test.policy new file mode 100644 index 00000000000..46145b9fa41 --- /dev/null +++ b/test/hotspot/jtreg/runtime/Dictionary/test.policy @@ -0,0 +1,6 @@ +grant { + permission java.io.FilePermission "<>", "read, write, delete"; + permission java.lang.RuntimePermission "createClassLoader"; + permission java.lang.RuntimePermission "getClassLoader"; + permission java.util.PropertyPermission "*", "read"; /* for Utils */ +};