--- a/src/hotspot/share/classfile/dictionary.cpp	Thu Feb 28 22:11:46 2019 +0100
+++ b/src/hotspot/share/classfile/dictionary.cpp	Thu Feb 28 16:30:31 2019 -0500
@@ -24,7 +24,7 @@
 
 #include "precompiled.hpp"
 #include "classfile/classLoaderData.inline.hpp"
-#include "classfile/dictionary.inline.hpp"
+#include "classfile/dictionary.hpp"
 #include "classfile/protectionDomainCache.hpp"
 #include "classfile/systemDictionary.hpp"
 #include "logging/log.hpp"
@@ -35,6 +35,7 @@
 #include "oops/oop.inline.hpp"
 #include "runtime/atomic.hpp"
 #include "runtime/orderAccess.hpp"
+#include "runtime/mutexLocker.hpp"
 #include "runtime/safepointVerifiers.hpp"
 #include "utilities/hashtable.inline.hpp"
 
@@ -80,6 +81,8 @@
 void Dictionary::free_entry(DictionaryEntry* entry) {
   // avoid recursion when deleting linked list
   // pd_set is accessed during a safepoint.
+  // This doesn't require a lock because nothing is reading this
+  // entry anymore.  The ClassLoader is dead.
   while (entry->pd_set() != NULL) {
     ProtectionDomainEntry* to_delete = entry->pd_set();
     entry->set_pd_set(to_delete->next());
@@ -146,11 +149,14 @@
 }
 
 bool DictionaryEntry::contains_protection_domain(oop protection_domain) const {
+  // Lock the pd_set list.  This lock cannot safepoint since the caller holds
+  // a Dictionary entry, which can be moved if the Dictionary is resized.
+  MutexLockerEx ml(ProtectionDomainSet_lock, Mutex::_no_safepoint_check_flag);
 #ifdef ASSERT
   if (oopDesc::equals(protection_domain, instance_klass()->protection_domain())) {
     // Ensure this doesn't show up in the pd_set (invariant)
     bool in_pd_set = false;
-    for (ProtectionDomainEntry* current = pd_set_acquire();
+    for (ProtectionDomainEntry* current = pd_set();
                                 current != NULL;
                                 current = current->next()) {
       if (oopDesc::equals(current->object_no_keepalive(), protection_domain)) {
@@ -170,7 +176,7 @@
     return true;
   }
 
-  for (ProtectionDomainEntry* current = pd_set_acquire();
+  for (ProtectionDomainEntry* current = pd_set();
                               current != NULL;
                               current = current->next()) {
     if (oopDesc::equals(current->object_no_keepalive(), protection_domain)) return true;
@@ -183,13 +189,12 @@
   assert_locked_or_safepoint(SystemDictionary_lock);
   if (!contains_protection_domain(protection_domain())) {
     ProtectionDomainCacheEntry* entry = SystemDictionary::cache_get(protection_domain);
+    // The pd_set in the dictionary entry is protected by a low level lock.
+    // With concurrent PD table cleanup, these links could be broken.
+    MutexLockerEx ml(ProtectionDomainSet_lock, Mutex::_no_safepoint_check_flag);
     ProtectionDomainEntry* new_head =
                 new ProtectionDomainEntry(entry, pd_set());
-    // Warning: Preserve store ordering.  The SystemDictionary is read
-    //          without locks.  The new ProtectionDomainEntry must be
-    //          complete before other threads can be allowed to see it
-    //          via a store to _pd_set.
-    release_set_pd_set(new_head);
+    set_pd_set(new_head);
   }
   LogTarget(Trace, protectiondomain) lt;
   if (lt.is_enabled()) {
@@ -348,6 +353,56 @@
   return entry->is_valid_protection_domain(protection_domain);
 }
 
+// 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() {
+  assert_locked_or_safepoint(SystemDictionary_lock);
+
+  if (loader_data()->is_the_null_class_loader_data()) {
+    // Classes in the boot loader are not loaded with protection domains
+    return;
+  }
+
+  for (int index = 0; index < table_size(); index++) {
+    for (DictionaryEntry* probe = bucket(index);
+                          probe != NULL;
+                          probe = probe->next()) {
+      Klass* e = probe->instance_klass();
+
+      MutexLockerEx ml(ProtectionDomainSet_lock, Mutex::_no_safepoint_check_flag);
+      ProtectionDomainEntry* current = probe->pd_set();
+      ProtectionDomainEntry* prev = NULL;
+      while (current != NULL) {
+        if (current->object_no_keepalive() == NULL) {
+          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(" 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();
+        }
+      }
+    }
+  }
+}
+
+
 SymbolPropertyTable::SymbolPropertyTable(int table_size)
   : Hashtable<Symbol*, mtSymbol>(table_size, sizeof(SymbolPropertyEntry))
 {
@@ -404,6 +459,25 @@
   }
 }
 
+void DictionaryEntry::verify_protection_domain_set() {
+  MutexLockerEx ml(ProtectionDomainSet_lock, Mutex::_no_safepoint_check_flag);
+  for (ProtectionDomainEntry* current = pd_set(); // accessed at a safepoint
+                              current != NULL;
+                              current = current->_next) {
+    guarantee(oopDesc::is_oop_or_null(current->_pd_cache->object_no_keepalive()), "Invalid oop");
+  }
+}
+
+void DictionaryEntry::print_count(outputStream *st) {
+  MutexLockerEx ml(ProtectionDomainSet_lock, Mutex::_no_safepoint_check_flag);
+  int count = 0;
+  for (ProtectionDomainEntry* current = pd_set();  // accessed inside SD lock
+                              current != NULL;
+                              current = current->_next) {
+    count++;
+  }
+  st->print_cr("pd set count = #%d", count);
+}
 
 // ----------------------------------------------------------------------------
 

--- a/src/hotspot/share/classfile/dictionary.hpp	Thu Feb 28 22:11:46 2019 +0100
+++ b/src/hotspot/share/classfile/dictionary.hpp	Thu Feb 28 16:30:31 2019 -0500
@@ -59,8 +59,6 @@
   static bool does_any_dictionary_needs_resizing();
   bool resize_if_needed();
 
-  DictionaryEntry* new_entry(unsigned int hash, InstanceKlass* klass);
-
   void add_klass(unsigned int hash, Symbol* class_name, InstanceKlass* obj);
 
   InstanceKlass* find_class(int index, unsigned int hash, Symbol* name);
@@ -70,7 +68,7 @@
   void all_entries_do(KlassClosure* closure);
   void classes_do(MetaspaceClosure* it);
 
-  void unlink();
+  void clean_cached_protection_domains();
 
   // Protection domains
   InstanceKlass* find(unsigned int hash, Symbol* name, Handle protection_domain);
@@ -83,6 +81,10 @@
 
   void print_on(outputStream* st) const;
   void verify();
+
+ private:
+  DictionaryEntry* new_entry(unsigned int hash, InstanceKlass* klass);
+
   DictionaryEntry* bucket(int i) const {
     return (DictionaryEntry*)Hashtable<InstanceKlass*, mtClass>::bucket(i);
   }
@@ -151,9 +153,6 @@
   ProtectionDomainEntry* pd_set() const            { return _pd_set; }
   void set_pd_set(ProtectionDomainEntry* new_head) {  _pd_set = new_head; }
 
-  ProtectionDomainEntry* pd_set_acquire() const;
-  void release_set_pd_set(ProtectionDomainEntry* new_head);
-
   // Tells whether the initiating class' protection domain can access the klass in this entry
   bool is_valid_protection_domain(Handle protection_domain) {
     if (!ProtectionDomainVerification) return true;
@@ -164,29 +163,14 @@
          : contains_protection_domain(protection_domain());
   }
 
-  void verify_protection_domain_set() {
-    for (ProtectionDomainEntry* current = pd_set(); // accessed at a safepoint
-                                current != NULL;
-                                current = current->_next) {
-      guarantee(oopDesc::is_oop_or_null(current->_pd_cache->object_no_keepalive()), "Invalid oop");
-    }
-  }
+  void verify_protection_domain_set();
 
   bool equals(const Symbol* class_name) const {
     InstanceKlass* klass = (InstanceKlass*)literal();
     return (klass->name() == class_name);
   }
 
-  void print_count(outputStream *st) {
-    int count = 0;
-    for (ProtectionDomainEntry* current = pd_set();  // accessed inside SD lock
-                                current != NULL;
-                                current = current->_next) {
-      count++;
-    }
-    st->print_cr("pd set count = #%d", count);
-  }
-
+  void print_count(outputStream *st);
   void verify();
 };
 

--- a/src/hotspot/share/classfile/dictionary.inline.hpp	Thu Feb 28 22:11:46 2019 +0100
+++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
@@ -1,39 +0,0 @@
-/*
- * Copyright (c) 2018, 2019, 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 SHARE_CLASSFILE_DICTIONARY_INLINE_HPP
-#define SHARE_CLASSFILE_DICTIONARY_INLINE_HPP
-
-#include "classfile/dictionary.hpp"
-#include "runtime/orderAccess.hpp"
-
-inline ProtectionDomainEntry* DictionaryEntry::pd_set_acquire() const {
-  return OrderAccess::load_acquire(&_pd_set);
-}
-
-inline void DictionaryEntry::release_set_pd_set(ProtectionDomainEntry* new_head) {
-  OrderAccess::release_store(&_pd_set, new_head);
-}
-
-#endif // SHARE_CLASSFILE_DICTIONARY_INLINE_HPP

--- a/src/hotspot/share/classfile/protectionDomainCache.cpp	Thu Feb 28 22:11:46 2019 +0100
+++ b/src/hotspot/share/classfile/protectionDomainCache.cpp	Thu Feb 28 16:30:31 2019 -0500
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2017, 2019, 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
@@ -23,6 +23,8 @@
  */
 
 #include "precompiled.hpp"
+#include "classfile/classLoaderDataGraph.hpp"
+#include "classfile/dictionary.hpp"
 #include "classfile/protectionDomainCache.hpp"
 #include "classfile/systemDictionary.hpp"
 #include "logging/log.hpp"
@@ -54,7 +56,27 @@
   Service_lock->notify_all();
 }
 
+class CleanProtectionDomainEntries : public CLDClosure {
+  void do_cld(ClassLoaderData* data) {
+    Dictionary* dictionary = data->dictionary();
+    if (dictionary != NULL) {
+      dictionary->clean_cached_protection_domains();
+    }
+  }
+};
+
 void ProtectionDomainCacheTable::unlink() {
+  {
+    // First clean cached pd lists in loaded CLDs
+    // It's unlikely, but some loaded classes in a dictionary might
+    // point to a protection_domain that has been unloaded.
+    // The dictionary pd_set points at entries in the ProtectionDomainCacheTable.
+    MutexLocker ml(ClassLoaderDataGraph_lock);
+    MutexLocker mldict(SystemDictionary_lock);  // need both.
+    CleanProtectionDomainEntries clean;
+    ClassLoaderDataGraph::loaded_cld_do(&clean);
+  }
+
   MutexLocker ml(SystemDictionary_lock);
   int oops_removed = 0;
   for (int i = 0; i < table_size(); ++i) {

--- a/src/hotspot/share/runtime/mutexLocker.cpp	Thu Feb 28 22:11:46 2019 +0100
+++ b/src/hotspot/share/runtime/mutexLocker.cpp	Thu Feb 28 16:30:31 2019 -0500
@@ -39,6 +39,7 @@
 
 Mutex*   Patching_lock                = NULL;
 Monitor* SystemDictionary_lock        = NULL;
+Mutex*   ProtectionDomainSet_lock     = NULL;
 Mutex*   SharedDictionary_lock        = NULL;
 Mutex*   Module_lock                  = NULL;
 Mutex*   CompiledIC_lock              = NULL;
@@ -254,6 +255,7 @@
   def(JmethodIdCreation_lock       , PaddedMutex  , leaf,        true,  Monitor::_safepoint_check_always);     // used for creating jmethodIDs.
 
   def(SystemDictionary_lock        , PaddedMonitor, leaf,        true,  Monitor::_safepoint_check_always);     // lookups done by VM thread
+  def(ProtectionDomainSet_lock     , PaddedMutex  , leaf-1,      true,  Monitor::_safepoint_check_never);
   def(SharedDictionary_lock        , PaddedMutex  , leaf,        true,  Monitor::_safepoint_check_always);     // lookups done by VM thread
   def(Module_lock                  , PaddedMutex  , leaf+2,      true,  Monitor::_safepoint_check_always);
   def(InlineCacheBuffer_lock       , PaddedMutex  , leaf,        true,  Monitor::_safepoint_check_never);

--- a/src/hotspot/share/runtime/mutexLocker.hpp	Thu Feb 28 22:11:46 2019 +0100
+++ b/src/hotspot/share/runtime/mutexLocker.hpp	Thu Feb 28 16:30:31 2019 -0500
@@ -33,6 +33,7 @@
 
 extern Mutex*   Patching_lock;                   // a lock used to guard code patching of compiled code
 extern Monitor* SystemDictionary_lock;           // a lock on the system dictionary
+extern Mutex*   ProtectionDomainSet_lock;        // a lock on the pd_set list in the system dictionary
 extern Mutex*   SharedDictionary_lock;           // a lock on the CDS shared dictionary
 extern Mutex*   Module_lock;                     // a lock on module and package related data structures
 extern Mutex*   CompiledIC_lock;                 // a lock used to guard compiled IC patching and access

--- a/test/hotspot/jtreg/runtime/Dictionary/ProtectionDomainCacheTest.java	Thu Feb 28 22:11:46 2019 +0100
+++ b/test/hotspot/jtreg/runtime/Dictionary/ProtectionDomainCacheTest.java	Thu Feb 28 16:30:31 2019 -0500
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2019, 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
@@ -23,14 +23,15 @@
 
 /*
  * @test
- * @bug 8151486
+ * @bug 8151486 8218266
  * @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/lib
+ * @modules java.base/jdk.internal.misc
  * @build jdk.test.lib.Utils
  *        jdk.test.lib.util.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
+ * @run main/othervm/policy=test.policy -Djava.security.manager ProtectionDomainCacheTest
  */
 
 import java.net.URL;
@@ -42,48 +43,69 @@
 import java.util.List;
 import jdk.test.lib.Utils;
 import jdk.test.lib.util.JarUtils;
+import java.io.File;
+
+import jdk.test.lib.process.OutputAnalyzer;
+import jdk.test.lib.process.ProcessTools;
 
 /*
  * 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";
+    static class Test {
+        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);
+        }
 
-    // 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"; }
-        };
+        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();
 
-        Class<?> loadClass = Class.forName("ClassForName", true, classLoader);
-        loadClass.newInstance();
+            // 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);
 
-        System.out.println("returning : " + classLoader);
+            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");
+        }
     }
 
-    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");
+    public static void main(String args[]) throws Exception {
+        ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(
+                                      "-Djava.security.policy==" + System.getProperty("test.src") + File.separator + "test.policy",
+                                      "-Dtest.classes=" + System.getProperty("test.classes", "."),
+                                      "-XX:+UnlockDiagnosticVMOptions",
+                                      "-XX:VerifySubSet=dictionary",
+                                      "-XX:+VerifyAfterGC",
+                                      "-Xlog:gc+verify,protectiondomain=debug",
+                                      "-Djava.security.manager",
+                                      Test.class.getName());
+        OutputAnalyzer output = new OutputAnalyzer(pb.start());
+        output.shouldContain("PD in set is not alive");
+        output.shouldHaveExitValue(0);
     }
 }

--- a/test/hotspot/jtreg/runtime/Dictionary/test.policy	Thu Feb 28 22:11:46 2019 +0100
+++ b/test/hotspot/jtreg/runtime/Dictionary/test.policy	Thu Feb 28 16:30:31 2019 -0500
@@ -1,5 +1,5 @@
 grant {
-  permission java.io.FilePermission "<<ALL FILES>>", "read, write, delete";
+  permission java.io.FilePermission "<<ALL FILES>>", "read, write, delete, execute";
   permission java.lang.RuntimePermission "createClassLoader";
   permission java.lang.RuntimePermission "getClassLoader";
   permission java.util.PropertyPermission "*", "read"; /* for Utils */