RFR: 8259242: vmTestbase/vm/mlvm/mixed/stress/java/findDeadlock/TestDescription.java timed out

[Coleen Phillimore]

On Wed, 7 Apr 2021 04:38:24 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> This change makes the DictionaryEntry pd_set lock free, because when I set a low SafepointTimeout on the findDeadlock test, these threads were waiting on the ProtectionDomainSet_lock.  This lock is a singleton that's taken for every dictionary entry.
>> 
>> I don't know if this change will make the test never timeout again, because removing this lock caused us to hit a low SafepointTimeout on another _no_safepoint_check lock that is not so easy to eradictate.
>> 
>> Tested with tiers 1-3.  There's a test that exercises this code in runtime/Dictionary/ProtectionDomainCacheTest.java.
>
> src/hotspot/share/classfile/dictionary.cpp line 422:
> 
>> 420: void Dictionary::clean_cached_protection_domains(GrowableArray<ProtectionDomainEntry*>* delete_list) {
>> 421:   assert(Thread::current()->is_Java_thread(), "only called by JavaThread");
>> 422:   assert_locked_or_safepoint(SystemDictionary_lock);
> 
> If the current thread must be a JavaThread then we cannot be at a safepoint and so the second assert can just check the lock is held.

sure.

> src/hotspot/share/classfile/dictionary.hpp line 145:
> 
>> 143: 
>> 144:   ProtectionDomainEntry* pd_set() const            { return Atomic::load(&_pd_set); }
>> 145:   void set_pd_set(ProtectionDomainEntry* entry)    { Atomic::store(&_pd_set, entry); }
> 
> These probably need to be load_acquire and release_store, so that a lock-free traversal can proceed correctly with a locked addition/removal. With suitable name changes for the methods.

I forgot that the method names have to match the implementation details. Thanks I didn't know whether they needed acquire etc.

-------------

PR: https://git.openjdk.java.net/jdk/pull/3362