RFR (S) 8218266: G1 crash in AccessInternal::PostRuntimeDispatch

[coleen.phillimore at oracle.com]

Please review: I've added a lock for the DictionaryEntry::pd_set which 
is a linked list of ProtectionDomainTableEntries.

open webrev at http://cr.openjdk.java.net/~coleenp/2019/8218266.02/webrev

Ran with modified test and hs tier1-3 and jck tests.

Thanks,
Coleen

On 2/25/19 5:48 PM, coleen.phillimore at oracle.com wrote:
>
>
> On 2/25/19 5:22 PM, zgu at redhat.com wrote:
>>>> problem. Therefore, I would like to withdraw my review.
>>> Okay, I can try to explain it.  All of the dictionary entries point
>>> to a
>>> linked list of protection domains that were used to load the
>>> class.
>>> The elements in this list point directly to Hashtable entries in the
>>> ProtectionDomainTable.  The protection domains can be GC'd before
>>> the
>>> dictionary entry is unloaded, so that the elements in the linked
>>> list
>>> need to be cleaned out.  I chose cleaning to happen right before the
>>> protection domain cache table is cleaned up so that the entries are
>>> not
>>> dangling pointers.  Since the table is cleaned out concurrently now,
>>> I
>>> have to do the cleanup there so I know it's happened.
>>>
>> Thanks for educating me.
>>
>> I were not sure if the crash was due to concurrent access, e.g.
>> DictionaryEntry could *leak* outside of Dictionary, then
>> ProtectedDomain can be accessed without protection. It looks like that
>> was not the case, but still should be avoided. How about places these
>> methods under private? it built.
>>
>> http://cr.openjdk.java.net/~zgu/JDK-8218266-review/webrev.00/src/hotspo
>> t/share/classfile/dictionary.hpp.udiff.html
>>
>>
>> I did found an instance of unprotected access -
>> DictionaryEntry::contains_protection_domain() may be unprotected when
>> it walks protection domain list.
>>
>> I added "assert_locked_or_safepoint(SystemDictionary_lock);" inside the
>> method:
>>
>> http://cr.openjdk.java.net/~zgu/JDK-8218266-review/webrev.00/src/hotspo
>> t/share/classfile/dictionary.cpp.udiff.html
>>
>> the assertion failed.
> Hi Zhengyu,   This is an excellent find!  Since we concurrently clean 
> the ProtectionDomainCacheTable, we need to lock access to the pd_lists 
> in the Dictionary entries.  We used to be able to read them lock free.
>
> I have to rework this to add the appropriate locking.
>
> thanks!
> Coleen
>>
>> Thanks,
>>
>> -Zhengyu
>>
>>
>>
>>> Thanks,
>>> Coleen
>>>> Sorry.
>>>>
>>>> -Zhengyu
>>>>
>>>>
>>>>> Coleen
>>>>>> Thanks,
>>>>>>
>>>>>> -Zhengyu
>>>>>>
>>>>>>> Thanks,
>>>>>>> Coleen
>>>
>