[14] RFR (XS): 8235305: Corrupted oops embedded in nmethods due to parallel modification during optional evacuation

[Stefan Karlsson]

Looks good.

StefanK

On 2020-01-17 14:27, Thomas Schatzl wrote:
> Hi Stefan,
> 
> On 17.01.20 10:06, Stefan Johansson wrote:
>> Hi Thomas,
>>
>> On 2020-01-16 14:20, Thomas Schatzl wrote:
>>> Hi all,
>>>
>>>    can I get reviews for this change that fixes a bug in the 
>>> abortable mixed gc algorithm where G1 might corrupt oops embedded in 
>>> nmethods due to parallel modification during an optional evacuation 
>>> phase?
>>>
>>> G1 currently collects embedded oops in nmethods twice: once in the 
>>> optional roots list, and once as nmethods in the strong code roots 
>>> list for a particular region.
>>>
>>> Now it can happen that this oop embedded in in the code stream is 
>>> unaligned, so if that oop is modified during relocation word tearing 
>>> may occur, causing follow-up crashes.
>>>
>>> The fix is to not collect oops from nmethods in the optional code 
>>> root list as the strong code root list for a particular region 
>>> already always contains it anyway.
>>>
>>> Thanks go to stefank, eriko and sjohanss for helping with analyzing, 
>>> testing and the discussion around it.
>>>
>>> CR:
>>> https://bugs.openjdk.java.net/browse/JDK-8235305
>>> Webrev:
>>> http://cr.openjdk.java.net/~tschatzl/8235305/webrev/
>>
>> Fix looks good. 
> 
> Thanks for your review.
> 
>> Just some things around the naming of the template parameter and enum 
>> after adding this. I don't have a much better idea
> [...]
> 
> Talked to them about this and I'm good with their suggestion:
> 
> http://cr.openjdk.java.net/~tschatzl/8235305/webrev.1 (full)
> http://cr.openjdk.java.net/~tschatzl/8235305/webrev.0_to_1 (diff)
> 
> Thanks,
>    Thomas
>