RFR (S) 8164207: Checking missing load-acquire in relation to _pd_set in dictionary.cpp

[coleen.phillimore at oracle.com]

Thanks Christian!
Coleen

On 8/25/17 5:16 PM, Christian Thalinger wrote:
>
>> On Aug 25, 2017, at 3:26 AM, coleen.phillimore at oracle.com 
>> <mailto:coleen.phillimore at oracle.com> wrote:
>>
>>
>> Thank you Zhengyu for noticing this change was wrong, and Christian 
>> for the idea.   New webrev:
>>
>> open webrev at http://cr.openjdk.java.net/~coleenp/8164207.02/webrev 
>> <http://cr.openjdk.java.net/%7Ecoleenp/8164207.02/webrev>
>> bug link https://bugs.openjdk.java.net/browse/JDK-8164207
>
> Looks good.
>
>>
>> I reran parallel class loading tests and jck testing is in progress, 
>> but order access requires inspection.
>>
>> Thanks,
>> Coleen
>>
>>
>> On 8/24/17 5:11 PM, coleen.phillimore at oracle.com 
>> <mailto:coleen.phillimore at oracle.com> wrote:
>>>
>>>
>>> On 8/24/17 5:00 PM, Christian Thalinger wrote:
>>>>> On Aug 24, 2017, at 10:54 AM, coleen.phillimore at oracle.com 
>>>>> <mailto:coleen.phillimore at oracle.com> wrote:
>>>>>
>>>>>
>>>>>
>>>>> On 8/24/17 4:07 PM, Zhengyu Gu wrote:
>>>>>> Hi Coleen,
>>>>>>
>>>>>> There are two instances probably overlooked?
>>>>>>
>>>>>> dictionary.cpp #103 and #124
>>>>>>
>>>>>>    for (ProtectionDomainEntry* current = _pd_set;
>>>>>> =>
>>>>>>    for (ProtectionDomainEntry* current = pd_set();
>>>>>>
>>>>>>
>>>>> Oh yeah, you're right.  That's embarrasing.   I'll fix and retest.
>>>> Which also shows that there is a potential for future mistakes. Can 
>>>> we isolate the field better so it’s only accessible via setter and 
>>>> getter?
>>>
>>> Yes, great idea.
>>> Coleen
>>>
>>>>> Thank you!!
>>>>> Coleen
>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> -Zhengyu
>>>>>>
>>>>>> On 08/24/2017 02:28 PM, coleen.phillimore at oracle.com 
>>>>>> <mailto:coleen.phillimore at oracle.com> wrote:
>>>>>>> Summary: Use load_acquire for accessing DictionaryEntry::_pd_set 
>>>>>>> since it's accessed outside the SystemDictionary_lock
>>>>>>>
>>>>>>> Ran parallel class loading tests that we have as well as tier1 
>>>>>>> tests. See bug for details.
>>>>>>>
>>>>>>> open webrev at 
>>>>>>> http://cr.openjdk.java.net/~coleenp/8164207.01/webrev 
>>>>>>> <http://cr.openjdk.java.net/%7Ecoleenp/8164207.01/webrev>
>>>>>>> bug link https://bugs.openjdk.java.net/browse/JDK-8164207
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Coleen
>>>>>>>
>>>
>>
>