JDK-8164207 : Checking missing load-acquire in relation to _pd_set in dictionary.cpp
  • Type: Bug
  • Component: hotspot
  • Sub-Component: runtime
  • Affected Version: 9
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2016-08-17
  • Updated: 2019-05-22
  • Resolved: 2017-08-31
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 10
10 b23Fixed
Related Reports
Relates :  
JDK-8158854 - Ensure release_store is paired with load_acquire in lock-free code
Description
./share/vm/classfile/dictionary.cpp contains:

OrderAccess::release_store_ptr(&_pd_set, new_head);

But no load-acquire, and also many plain stores to _pd_set field. 

That said, the supposed lock-free reads of _pd_set seem to always occur at a safepoint:

  void set_strongly_reachable() {
+ assert(SafepointSynchronize::is_at_safepoint(), "must be at safepoint");

   void verify_protection_domain_set() {
+ assert(SafepointSynchronize::is_at_safepoint(), "must be at safepoint");

the above patch passed full nightly testing in RBT, which suggests that it may be the release_store that is in fact unnecessary.

To allow JDK-8158854 to be finalized I'm filing this as a separate issue to follow up on.
Comments
Yes we need a load_acquire in contains_protection_domain because this version of Klass* SystemDictionary::find(Symbol* class_name, Handle class_loader, Handle protection_domain, TRAPS) { doesn't lock the system dictionary but looks in the pd_cache. Without the load-acquire, it might not find it, which is fine because it'll find it eventually. This probably doesn't need this lock: // 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; } } Because it unlocks, adds the PD, which another thread could be doing at the same time. is_valid_protection_domain calls contains_protection_domain for a non-null pd.
16-08-2017