RFR (S) 8225681: vmTestbase/nsk/jvmti/RedefineClasses/StressRedefine fails due a) MT-unsafe modification of inline cache

[erik.osterlund at oracle.com]

For any race to be observed, wouldn't we need to have concurrent writes 
not protected by the CompiledICLocker? If that indeed happens, then that 
seems like a bug, right? Or maybe I'm missing something.

/Erik

On 9/27/19 2:18 PM, coleen.phillimore at oracle.com wrote:
>
>
> On 9/27/19 8:00 AM, erik.osterlund at oracle.com wrote:
>> Hi Coleen,
>>
>> I don't understand this. It makes the frame-local variable volatile. 
>> But surely the frame local variable is accessed only by the current 
>> thread. And the whole thing is protected by the CompiledICLocker. 
>> Would you mind explaining what we are worried about here?
>
> The worry is that old_method would be fetched multiple times from 
> data() in the assert.  Another thread could be updating data(), if I'm 
> not mistaken.  Making destination volatile might not be necessary, but 
> it was like that in s390, which experienced the race.
>
> Coleen
>
>>
>> Thanks,
>> /Erik
>>
>> On 9/26/19 5:59 PM, coleen.phillimore at oracle.com wrote:
>>>
>>> For the record, I added volatiles after discussions with Goetz:
>>>
>>> http://cr.openjdk.java.net/~coleenp/2019/8225681.02/webrev
>>>
>>> Which builds on s390 and ppc, and verified on Oracle platforms as well.
>>>
>>> Coleen
>>>
>>> On 9/26/19 7:24 AM, coleen.phillimore at oracle.com wrote:
>>>> Thanks Erik!
>>>> Coleen
>>>>
>>>> On 9/26/19 5:47 AM, erik.osterlund at oracle.com wrote:
>>>>> Hi Coleen,
>>>>>
>>>>> Looks good.
>>>>>
>>>>> Thanks,
>>>>> /Erik
>>>>>
>>>>> On 9/25/19 11:22 PM, coleen.phillimore at oracle.com wrote:
>>>>>> Summary: allow old methods in 
>>>>>> CompiledStaticDirectCall::set_to_interpreted
>>>>>>
>>>>>> This is the comment in the bug that describes this race and this 
>>>>>> fix:
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8225681?focusedCommentId=14278441&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-14278441 
>>>>>>
>>>>>>
>>>>>> The rest of the bug and sightings are actually caused by 
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8226690,
>>>>>> and this one might have been caused by it also, but the race that 
>>>>>> Erik describes is possible as well.
>>>>>>
>>>>>> The s390 code had an exception for 
>>>>>> callee->is_compiled_lambda_form() which should probably apply to 
>>>>>> all the platforms, so the code is the same on all the platforms 
>>>>>> with this change.
>>>>>>
>>>>>> Tested with tier1-6.
>>>>>>
>>>>>> open webrev at 
>>>>>> http://cr.openjdk.java.net/~coleenp/2019/8225681.01/webrev
>>>>>> bug link https://bugs.openjdk.java.net/browse/JDK-8225681
>>>>>>
>>>>>> Thanks,
>>>>>> Coleen
>>>>
>>>
>>
>