RFR: 8246442: nmethod::can_convert_to_zombie() asserts when not called by the sweeper

[Erik Österlund]

Hi Dan,

Thanks for the review.

/Erik

On 2020-06-03 15:31, Daniel D. Daugherty wrote:
> On 6/3/20 7:26 AM, Erik Österlund wrote:
>> Hi,
>>
>> nmethod::can_convert_to_zombie() used to be called only by the 
>> sweeper, after it has checked that
>> an nmethod is not entrant. Due to the single call site, there was an 
>> assert checking that the nmethod
>> should be not entrant (the condition just checked).
>> However, it is now used in 
>> nmethod::post_compiled_method_load_event(), where the result of the 
>> assert is racy.
>>
>> When an nmethod is created, the load event will never be able to see 
>> the nmethod spuriously flipping
>> to zombie, because there simply are no safepoint polls/transitions 
>> between the nmethod being created
>> and the load event firing. However, the jvmti_GenerateEvents can 
>> observe such spurious flips to zombie.
>> It is completely harmless, because the whole operation runs under the 
>> CodeCache_lock, which prevents
>> further nmethods from being flushed. But they can flip to zombie, 
>> which is indeed exactly what the
>> check is design to test. Therefore, this is just a false positive 
>> assertion failure.
>>
>> Proposed fix: make the assert only apply when called from the sweeper 
>> thread.
>>
>> Webrev:
>> http://cr.openjdk.java.net/~eosterlund/8246442/webrev.00/
>
> src/hotspot/share/code/nmethod.cpp
>     No comments.
>
> Thumbs up.
>
> I think Leonid filed a similar bug the other day:
>
> JDK-8245877 assert(_value != __null) failed: resolving NULL _value in 
> JvmtiExport::post_compiled_method_load
>
> so I've added Serguei to this review thread since JDK-8245877
> is assigned to him.
>
> Dan
>
>
>
>>
>> Bug:
>> https://bugs.openjdk.java.net/browse/JDK-8246442
>>
>> Thanks,
>> /Erik
>