RFR: 8215491: ICStubInterface::finalize finds zombie nmethod with ZGC concurrent class unloading

[Erik Österlund]

Hi Dean,

On 2018-12-17 20:27, dean.long at oracle.com wrote:
> On 12/17/18 8:56 AM, Erik Österlund wrote:
>> Hi,
>>
>> Today, when converting a not_entrant nmethod to zombie, the sweeper 
>> first clears the IC stubs of that nmethod, and then make_zombie(). 
>> With concurrent class unloading, this ordering is a bit unfortunate. 
>> Between clearing the IC stubs and making it zombie, the concurrent GC 
>> threads may hit a timing window where they will create IC stubs for 
>> concurrently cleaning the ICs of this still is_alive() nmethod. 
> 
> Do we normally have to creates stubs to clean an IC?

Yes if you clean an IC in an nmethod that is_alive(), we currently have 
to do that using IC stubs. I have some ideas going forward for how we 
can stop doing that, but that is outside of scope for now.

> 
>> The result is that during safepoint cleanup, when we finalize the IC 
>> stubs, we find one associated with a zombie.
>>
>> The unregistering of the nmethod from the GC will block during 
>> concurrent nmethod unloading, so clearing the IC stubs after the 
>> nmethod has become zombie is a lot more sane as there can not be any 
>> such races then.
>>
> 
> Where is the unregistering happening?

Inside of make_zombie() we unregister_nmethod() on CollectedHeap. In 
there, ZGC makes sure we wait until concurrent nmethod unloading is over 
before continuing.

> 
> Doing the clearing after it's a zombie does sound safer.

Agreed!

Thanks for reviewing.

/Erik

> dl
> 
>> Bug:
>> https://bugs.openjdk.java.net/browse/JDK-8215491
>>
>> Webrev:
>> http://cr.openjdk.java.net/~eosterlund/8215491/webrev.00/index.html
>>
>> Thanks,
>> /Erik
>