[sh/jdk8u] MetadataOnStackMark::record() is not MT-safe

[Roman Kennke]

Ok then. Go!

Thanks,
Roman



> 11u already encapsulated metadata in nmethod (see 
> nmethod::do_unloading_oops(), which is quite different from 8u), so on 
> stack metadata (when GC scans thread roots) keeps nmethod alive.
> 
> -Zhengyu
> 
> 
> 
> On 7/29/21 10:38 AM, Roman Kennke wrote:
>>> Tested 11u, does not seem affected by this bug.
>>
>> Do you have an idea why? Maybe there is a change in 11 that fixes or 
>> prevents it (and could be backported)?
>>
>> Roman
>>
>>> -Zhengyu
>>>
>>> On 7/29/21 10:22 AM, Roman Kennke wrote:
>>>> Ok.
>>>>
>>>> How about later JDKs? Could they be affected, too?
>>>>
>>>> Thanks,
>>>> Roman
>>>>
>>>>> This is not a Shenandoah specific bug, I believe G1 suffers the 
>>>>> same problem. The reason to fix Shenandoah first, in hope that we 
>>>>> can have a build for our customer to test this fix, as we don't 
>>>>> have a reproducer. I can move the fix to jdk8u if you think otherwise.
>>>>>
>>>>> Apparently, MetadataOnStackMark::record() was never designed for 
>>>>> MT, but G1 and Shenandoah's parallel cleaning code can call 
>>>>> nmethod::do_unloading_parallel() from different worker threads, and 
>>>>> nmethod::do_unloading_parallel() in turn calls 
>>>>> nmethod::clean_ic_if_metadata_is_dead() which may call 
>>>>> Metadata::mark_on_stack() to record metadata to MetadataOnStackMark().
>>>>>
>>>>> Calling MetadataOnStackMark::record() without synchronization, may 
>>>>> corrupt internal buffer, which can result fatal error in 
>>>>> MetadataOnStackMark's destructor, when it walks the buffers.
>>>>>
>>>>>
>>>>> This problem is hard to reproduce, cause it is only possible when 
>>>>> there are redefined classes.
>>>>>
>>>>>
>>>>> Webrev: 
>>>>> http://cr.openjdk.java.net/~zgu/shenandoah/metadata_on_stack/webrev.00/ 
>>>>>
>>>>>
>>>>> Thanks,
>>>>>
>>>>> -Zhengyu
>>>>>
>>>>
>>>
>>
>