# HG changeset patch
# User coleenp
# Date 1504135102 14400
# Node ID e704f55561c3af127a2e715eacea5b8ac89c73f0
# Parent  e43050e1b17f03adf34fb097f4aeadf5daef16ee
8164207: Checking missing load-acquire in relation to _pd_set in dictionary.cpp
Summary: Use load_acquire for accessing DictionaryEntry::_pd_set since it's accessed outside the SystemDictionary_lock
Reviewed-by: zgu, twisti, dholmes, adinn

diff -r e43050e1b17f -r e704f55561c3 hotspot/src/share/vm/classfile/dictionary.cpp
--- a/hotspot/src/share/vm/classfile/dictionary.cpp	Wed Aug 30 15:48:47 2017 +0200
+++ b/hotspot/src/share/vm/classfile/dictionary.cpp	Wed Aug 30 19:18:22 2017 -0400
@@ -85,6 +85,7 @@
 
 void Dictionary::free_entry(DictionaryEntry* entry) {
   // avoid recursion when deleting linked list
+  // pd_set is accessed during a safepoint.
   while (entry->pd_set() != NULL) {
     ProtectionDomainEntry* to_delete = entry->pd_set();
     entry->set_pd_set(to_delete->next());
@@ -101,7 +102,7 @@
   if (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;
+    for (ProtectionDomainEntry* current = pd_set_acquire();
                                 current != NULL;
                                 current = current->next()) {
       if (current->protection_domain() == protection_domain) {
@@ -121,7 +122,7 @@
     return true;
   }
 
-  for (ProtectionDomainEntry* current = _pd_set;
+  for (ProtectionDomainEntry* current = pd_set_acquire();
                               current != NULL;
                               current = current->next()) {
     if (current->protection_domain() == protection_domain) return true;
@@ -135,12 +136,12 @@
   if (!contains_protection_domain(protection_domain())) {
     ProtectionDomainCacheEntry* entry = SystemDictionary::cache_get(protection_domain);
     ProtectionDomainEntry* new_head =
-                new ProtectionDomainEntry(entry, _pd_set);
+                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.
-    OrderAccess::release_store_ptr(&_pd_set, new_head);
+    release_set_pd_set(new_head);
   }
   LogTarget(Trace, protectiondomain) lt;
   if (lt.is_enabled()) {
diff -r e43050e1b17f -r e704f55561c3 hotspot/src/share/vm/classfile/dictionary.hpp
--- a/hotspot/src/share/vm/classfile/dictionary.hpp	Wed Aug 30 15:48:47 2017 +0200
+++ b/hotspot/src/share/vm/classfile/dictionary.hpp	Wed Aug 30 19:18:22 2017 -0400
@@ -29,6 +29,7 @@
 #include "classfile/systemDictionary.hpp"
 #include "oops/instanceKlass.hpp"
 #include "oops/oop.hpp"
+#include "runtime/orderAccess.hpp"
 #include "utilities/hashtable.hpp"
 #include "utilities/ostream.hpp"
 
@@ -134,7 +135,7 @@
   // It is essentially a cache to avoid repeated Java up-calls to
   // ClassLoader.checkPackageAccess().
   //
-  ProtectionDomainEntry* _pd_set;
+  ProtectionDomainEntry* volatile _pd_set;
 
  public:
   // Tells whether a protection is in the approved set.
@@ -153,8 +154,15 @@
     return (DictionaryEntry**)HashtableEntry<InstanceKlass*, mtClass>::next_addr();
   }
 
-  ProtectionDomainEntry* pd_set() const { return _pd_set; }
-  void set_pd_set(ProtectionDomainEntry* pd_set) { _pd_set = pd_set; }
+  ProtectionDomainEntry* pd_set() const            { return _pd_set; }
+  void set_pd_set(ProtectionDomainEntry* new_head) {  _pd_set = new_head; }
+
+  ProtectionDomainEntry* pd_set_acquire() const    {
+    return (ProtectionDomainEntry*)OrderAccess::load_ptr_acquire(&_pd_set);
+  }
+  void release_set_pd_set(ProtectionDomainEntry* new_head) {
+    OrderAccess::release_store_ptr(&_pd_set, new_head);
+  }
 
   // Tells whether the initiating class' protection domain can access the klass in this entry
   bool is_valid_protection_domain(Handle protection_domain) {
@@ -167,7 +175,7 @@
   }
 
   void verify_protection_domain_set() {
-    for (ProtectionDomainEntry* current = _pd_set;
+    for (ProtectionDomainEntry* current = pd_set(); // accessed at a safepoint
                                 current != NULL;
                                 current = current->_next) {
       current->_pd_cache->protection_domain()->verify();
@@ -181,7 +189,7 @@
 
   void print_count(outputStream *st) {
     int count = 0;
-    for (ProtectionDomainEntry* current = _pd_set;
+    for (ProtectionDomainEntry* current = pd_set();  // accessed inside SD lock
                                 current != NULL;
                                 current = current->_next) {
       count++;
diff -r e43050e1b17f -r e704f55561c3 hotspot/src/share/vm/classfile/systemDictionary.cpp
--- a/hotspot/src/share/vm/classfile/systemDictionary.cpp	Wed Aug 30 15:48:47 2017 +0200
+++ b/hotspot/src/share/vm/classfile/systemDictionary.cpp	Wed Aug 30 19:18:22 2017 -0400
@@ -910,12 +910,9 @@
   if (protection_domain() == NULL) return k;
 
   // Check the protection domain has the right access
-  {
-    MutexLocker mu(SystemDictionary_lock, THREAD);
-    if (dictionary->is_valid_protection_domain(d_index, d_hash, name,
-                                               protection_domain)) {
-      return k;
-    }
+  if (dictionary->is_valid_protection_domain(d_index, d_hash, name,
+                                             protection_domain)) {
+    return k;
   }
 
   // Verify protection domain. If it fails an exception is thrown