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

[Zhengyu Gu]

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