8206424: Use locking for cleaning ProtectionDomainTable
authorpchilanomate
Fri, 31 Aug 2018 10:22:04 -0400
changeset 51610 cdef4df6b0e7
parent 51609 48a95b70d4af
child 51611 2ef81feac8b8
8206424: Use locking for cleaning ProtectionDomainTable Summary: ServiceThread is now in charge of cleaning ProtectionDomainTable entries Reviewed-by: coleenp, iklam
src/hotspot/share/classfile/protectionDomainCache.cpp
src/hotspot/share/classfile/protectionDomainCache.hpp
src/hotspot/share/classfile/systemDictionary.cpp
src/hotspot/share/classfile/systemDictionary.hpp
src/hotspot/share/prims/whitebox.cpp
src/hotspot/share/runtime/serviceThread.cpp
test/hotspot/jtreg/runtime/Dictionary/CleanProtectionDomain.java
test/lib/sun/hotspot/WhiteBox.java
--- a/src/hotspot/share/classfile/protectionDomainCache.cpp	Fri Aug 31 09:10:27 2018 -0400
+++ b/src/hotspot/share/classfile/protectionDomainCache.cpp	Fri Aug 31 10:22:04 2018 -0400
@@ -44,11 +44,19 @@
 
 ProtectionDomainCacheTable::ProtectionDomainCacheTable(int table_size)
   : Hashtable<ClassLoaderWeakHandle, mtClass>(table_size, sizeof(ProtectionDomainCacheEntry))
-{
+{   _dead_entries = false;
+    _total_oops_removed = 0;
+}
+
+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);
+  int oops_removed = 0;
   for (int i = 0; i < table_size(); ++i) {
     ProtectionDomainCacheEntry** p = bucket_addr(i);
     ProtectionDomainCacheEntry* entry = bucket(i);
@@ -57,7 +65,8 @@
       if (pd != NULL) {
         p = entry->next_addr();
       } else {
-        LogTarget(Debug, protectiondomain) lt;
+        oops_removed++;
+        LogTarget(Debug, protectiondomain, table) lt;
         if (lt.is_enabled()) {
           LogStream ls(lt);
           ls.print_cr("protection domain unlinked at %d", i);
@@ -69,9 +78,12 @@
       entry = *p;
     }
   }
+  _total_oops_removed += oops_removed;
+  _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 +136,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 +151,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<ClassLoaderWeakHandle, mtClass>::add_entry(index, p);
--- a/src/hotspot/share/classfile/protectionDomainCache.hpp	Fri Aug 31 09:10:27 2018 -0400
+++ b/src/hotspot/share/classfile/protectionDomainCache.hpp	Fri Aug 31 10:22:04 2018 -0400
@@ -85,6 +85,9 @@
   ProtectionDomainCacheEntry* add_entry(int index, unsigned int hash, Handle protection_domain);
   ProtectionDomainCacheEntry* find_entry(int index, Handle protection_domain);
 
+  bool _dead_entries;
+  int _total_oops_removed;
+
 public:
   ProtectionDomainCacheTable(int table_size);
   ProtectionDomainCacheEntry* get(Handle protection_domain);
@@ -93,6 +96,11 @@
 
   void print_on(outputStream* st) const;
   void verify();
+
+  bool has_work() { return _dead_entries; }
+  void trigger_cleanup();
+
+  int removed_entries_count() { return _total_oops_removed; };
 };
 
 
--- a/src/hotspot/share/classfile/systemDictionary.cpp	Fri Aug 31 09:10:27 2018 -0400
+++ b/src/hotspot/share/classfile/systemDictionary.cpp	Fri Aug 31 10:22:04 2018 -0400
@@ -1884,7 +1884,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) {
--- a/src/hotspot/share/classfile/systemDictionary.hpp	Fri Aug 31 09:10:27 2018 -0400
+++ b/src/hotspot/share/classfile/systemDictionary.hpp	Fri Aug 31 10:22:04 2018 -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;
--- a/src/hotspot/share/prims/whitebox.cpp	Fri Aug 31 09:10:27 2018 -0400
+++ b/src/hotspot/share/prims/whitebox.cpp	Fri Aug 31 10:22:04 2018 -0400
@@ -28,6 +28,7 @@
 
 #include "classfile/classLoaderData.hpp"
 #include "classfile/modules.hpp"
+#include "classfile/protectionDomainCache.hpp"
 #include "classfile/stringTable.hpp"
 #include "code/codeCache.hpp"
 #include "compiler/methodMatcher.hpp"
@@ -1977,6 +1978,10 @@
   return (jint) ResolvedMethodTable::removed_entries_count();
 WB_END
 
+WB_ENTRY(jint, WB_ProtectionDomainRemovedCount(JNIEnv* env, jobject o))
+  return (jint) SystemDictionary::pd_cache_table()->removed_entries_count();
+WB_END
+
 
 #define CC (char*)
 
@@ -2199,6 +2204,7 @@
   {CC"printOsInfo",               CC"()V",            (void*)&WB_PrintOsInfo },
   {CC"disableElfSectionCache",    CC"()V",            (void*)&WB_DisableElfSectionCache },
   {CC"resolvedMethodRemovedCount",     CC"()I",       (void*)&WB_ResolvedMethodRemovedCount },
+  {CC"protectionDomainRemovedCount",   CC"()I",       (void*)&WB_ProtectionDomainRemovedCount },
 };
 
 
--- a/src/hotspot/share/runtime/serviceThread.cpp	Fri Aug 31 09:10:27 2018 -0400
+++ b/src/hotspot/share/runtime/serviceThread.cpp	Fri Aug 31 10:22:04 2018 -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	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/hotspot/jtreg/runtime/Dictionary/CleanProtectionDomain.java	Fri Aug 31 10:22:04 2018 -0400
@@ -0,0 +1,88 @@
+/*
+ * 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
+ * @summary Verifies the creation and cleaup of entries in the Protection Domain Table
+ * @library /test/lib
+ * @modules java.base/jdk.internal.misc
+ * @build sun.hotspot.WhiteBox
+ * @run driver ClassFileInstaller sun.hotspot.WhiteBox sun.hotspot.WhiteBox$WhiteBoxPermission
+ * @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;
+import sun.hotspot.WhiteBox;
+
+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",
+                                  "-XX:+WhiteBoxAPI",
+                                  "-Xbootclasspath/a:.",
+                                  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 {
+      WhiteBox wb = WhiteBox.getWhiteBox();
+      int removedCountOrig =  wb.protectionDomainRemovedCount();
+      int removedCount;
+
+      test();
+
+      System.gc();
+      // Wait until ServiceThread cleans ProtectionDomain table.
+      // When the TestClassLoader is unloaded by GC, at least one
+      // ProtectionDomainCacheEntry will be eligible for removal.
+      do {
+        removedCount = wb.protectionDomainRemovedCount();
+      } while (removedCountOrig == removedCount);
+    }
+
+    private static class TestClassLoader extends ClassLoader {
+      public TestClassLoader() {
+        super();
+      }
+    }
+  }
+}
--- a/test/lib/sun/hotspot/WhiteBox.java	Fri Aug 31 09:10:27 2018 -0400
+++ b/test/lib/sun/hotspot/WhiteBox.java	Fri Aug 31 10:22:04 2018 -0400
@@ -539,4 +539,7 @@
 
   // Resolved Method Table
   public native int resolvedMethodRemovedCount();
+
+  // Protection Domain Table
+  public native int protectionDomainRemovedCount();
 }