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

[Thomas Schatzl]

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