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

[coleen.phillimore at oracle.com]

On 9/27/19 9:20 AM, erik.osterlund at oracle.com wrote:
> 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.

I can't actually answer that because I didn't observe the races. Was 
this always protected by the CompiledICLocker?

I can remove the volatiles if they are protected by the CompiledICLocker.

Coleen

>
> /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
>>>>>
>>>>
>>>
>>
>