--- old/src/hotspot/share/classfile/protectionDomainCache.cpp 2018-08-21 14:24:16.835273095 -0400 +++ new/src/hotspot/share/classfile/protectionDomainCache.cpp 2018-08-21 14:24:16.382231073 -0400 @@ -44,11 +44,17 @@ ProtectionDomainCacheTable::ProtectionDomainCacheTable(int table_size) : Hashtable(table_size, sizeof(ProtectionDomainCacheEntry)) -{ +{ _dead_entries = false; +} + +void ProtectionDomainCacheTable::trigger_cleanup() { + MutexLockerEx ml(Service_lock, Mutex::_no_safepoint_check_flag); + _dead_entries = true; + Service_lock->notify_all(); } void ProtectionDomainCacheTable::unlink() { - assert(SafepointSynchronize::is_at_safepoint(), "must be"); + MutexLocker ml(SystemDictionary_lock); for (int i = 0; i < table_size(); ++i) { ProtectionDomainCacheEntry** p = bucket_addr(i); ProtectionDomainCacheEntry* entry = bucket(i); @@ -57,7 +63,7 @@ if (pd != NULL) { p = entry->next_addr(); } else { - LogTarget(Debug, protectiondomain) lt; + LogTarget(Debug, protectiondomain, table) lt; if (lt.is_enabled()) { LogStream ls(lt); ls.print_cr("protection domain unlinked at %d", i); @@ -69,9 +75,11 @@ entry = *p; } } + _dead_entries = false; } void ProtectionDomainCacheTable::print_on(outputStream* st) const { + assert_locked_or_safepoint(SystemDictionary_lock); st->print_cr("Protection domain cache table (table_size=%d, classes=%d)", table_size(), number_of_entries()); for (int index = 0; index < table_size(); index++) { @@ -124,6 +132,7 @@ } ProtectionDomainCacheEntry* ProtectionDomainCacheTable::find_entry(int index, Handle protection_domain) { + assert_locked_or_safepoint(SystemDictionary_lock); for (ProtectionDomainCacheEntry* e = bucket(index); e != NULL; e = e->next()) { if (oopDesc::equals(e->object_no_keepalive(), protection_domain())) { return e; @@ -138,6 +147,13 @@ assert(index == index_for(protection_domain), "incorrect index?"); assert(find_entry(index, protection_domain) == NULL, "no double entry"); + LogTarget(Debug, protectiondomain, table) lt; + if (lt.is_enabled()) { + LogStream ls(lt); + ls.print("protection domain added "); + protection_domain->print_value_on(&ls); + ls.cr(); + } ClassLoaderWeakHandle w = ClassLoaderWeakHandle::create(protection_domain); ProtectionDomainCacheEntry* p = new_entry(hash, w); Hashtable::add_entry(index, p); --- old/src/hotspot/share/classfile/protectionDomainCache.hpp 2018-08-21 14:24:17.382323836 -0400 +++ new/src/hotspot/share/classfile/protectionDomainCache.hpp 2018-08-21 14:24:16.928281722 -0400 @@ -85,6 +85,8 @@ ProtectionDomainCacheEntry* add_entry(int index, unsigned int hash, Handle protection_domain); ProtectionDomainCacheEntry* find_entry(int index, Handle protection_domain); + bool _dead_entries; + public: ProtectionDomainCacheTable(int table_size); ProtectionDomainCacheEntry* get(Handle protection_domain); @@ -93,6 +95,9 @@ void print_on(outputStream* st) const; void verify(); + + bool has_work() { return _dead_entries; } + void trigger_cleanup(); }; --- old/src/hotspot/share/classfile/systemDictionary.cpp 2018-08-21 14:24:17.924374113 -0400 +++ new/src/hotspot/share/classfile/systemDictionary.cpp 2018-08-21 14:24:17.470331999 -0400 @@ -1878,7 +1878,7 @@ // Oops referenced by the protection domain cache table may get unreachable independently // of the class loader (eg. cached protection domain oops). So we need to // explicitly unlink them here. - _pd_cache_table->unlink(); + _pd_cache_table->trigger_cleanup(); } if (do_cleaning) { --- old/src/hotspot/share/classfile/systemDictionary.hpp 2018-08-21 14:24:18.534430699 -0400 +++ new/src/hotspot/share/classfile/systemDictionary.hpp 2018-08-21 14:24:18.079388492 -0400 @@ -376,6 +376,9 @@ // System loader lock static oop system_loader_lock() { return _system_loader_lock_obj; } + // Protection Domain Table + static ProtectionDomainCacheTable* pd_cache_table() { return _pd_cache_table; } + public: // Sharing support. static void reorder_dictionary_for_sharing() NOT_CDS_RETURN; --- old/src/hotspot/share/runtime/serviceThread.cpp 2018-08-21 14:24:19.094482646 -0400 +++ new/src/hotspot/share/runtime/serviceThread.cpp 2018-08-21 14:24:18.638440346 -0400 @@ -23,8 +23,10 @@ */ #include "precompiled.hpp" +#include "classfile/protectionDomainCache.hpp" #include "classfile/stringTable.hpp" #include "classfile/symbolTable.hpp" +#include "classfile/systemDictionary.hpp" #include "runtime/interfaceSupport.inline.hpp" #include "runtime/javaCalls.hpp" #include "runtime/serviceThread.hpp" @@ -88,6 +90,7 @@ bool stringtable_work = false; bool symboltable_work = false; bool resolved_method_table_work = false; + bool protection_domain_table_work= false; JvmtiDeferredEvent jvmti_event; { // Need state transition ThreadBlockInVM so that this thread @@ -107,7 +110,8 @@ !(has_dcmd_notification_event = DCmdFactory::has_pending_jmx_notification()) && !(stringtable_work = StringTable::has_work()) && !(symboltable_work = SymbolTable::has_work()) && - !(resolved_method_table_work = ResolvedMethodTable::has_work())) { + !(resolved_method_table_work = ResolvedMethodTable::has_work()) && + !(protection_domain_table_work = SystemDictionary::pd_cache_table()->has_work())) { // wait until one of the sensors has pending requests, or there is a // pending JVMTI event or JMX GC notification to post Service_lock->wait(Mutex::_no_safepoint_check_flag); @@ -145,6 +149,10 @@ if (resolved_method_table_work) { ResolvedMethodTable::unlink(); } + + if (protection_domain_table_work) { + SystemDictionary::pd_cache_table()->unlink(); + } } } --- /dev/null 2018-07-27 13:23:51.036000298 -0400 +++ new/test/hotspot/jtreg/runtime/appcds/CleanProtectionDomain.java 2018-08-21 14:24:19.186491180 -0400 @@ -0,0 +1,74 @@ +/* + * Copyright (c) 2017, 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 + * @summary Verifies the creation and cleaup of entries in the Protection Domain Table + * @library /test/lib + * @modules java.base/jdk.internal.misc + * java.compiler + * java.management + * @run main CleanProtectionDomain + */ + +import java.security.ProtectionDomain; +import jdk.test.lib.compiler.InMemoryJavaCompiler; +import jdk.internal.misc.Unsafe; +import static jdk.test.lib.Asserts.*; +import jdk.test.lib.process.OutputAnalyzer; +import jdk.test.lib.process.ProcessTools; + +public class CleanProtectionDomain { + + public static void main(String args[]) throws Exception { + ProcessBuilder pb = ProcessTools.createJavaProcessBuilder( + "-Xlog:protectiondomain+table=debug", + "--add-exports=java.base/jdk.internal.misc=ALL-UNNAMED", + Test.class.getName()); + OutputAnalyzer output = new OutputAnalyzer(pb.start()); + output.shouldContain("protection domain added"); + output.shouldContain("protection domain unlinked"); + output.shouldHaveExitValue(0); + } + + static class Test { + public static void test() throws Exception { + Unsafe unsafe = Unsafe.getUnsafe(); + TestClassLoader classloader = new TestClassLoader(); + ProtectionDomain pd = new ProtectionDomain(null, null); + byte klassbuf[] = InMemoryJavaCompiler.compile("TestClass", "class TestClass { }"); + Class klass = unsafe.defineClass(null, klassbuf, 0, klassbuf.length, classloader, pd); + } + + public static void main(String[] args) throws Exception { + test(); + System.gc(); + } + + private static class TestClassLoader extends ClassLoader { + public TestClassLoader() { + super(); + } + } + } +}