RFR: 8219687: G1 asserts nmethod should not be unloaded during parallel code cache unloading

[Vladimir Kozlov]

Assert changes look fine to me.

Thanks,
Vladimir

On 6/3/19 8:39 AM, Erik Österlund wrote:
> Hi,
> 
> There is an assert that G1 occasionally hits during parallel code cache unloading.
> When oops_do() is called to compute is_unloading(), the nmethod should not be unloaded says the assert.
> 
> However... imagine the following scenario:
> 
> A bunch of parallel GC threads are walking the code cache in parallel, unloading dead things.
> 
> GC thread 1 finds nmethod A in the code cache iteration. It is_unloading(), and hence its method()->method_holder() 
> dependency context is found to need cleaning, and cleaning starts. In that dependency context, it finds nmethod B, which 
> is asked if it is_unloading(). The result of the question is not cached, and calls oops_do is used to compute the answer.
> 
> GC thread 1 now peempts and wakes up later.
> 
> GC thread 2 finds nmethod B in the code cache iteration. It is_unloading(), and hence made make_unloaded(), eventually 
> changing the state to unloaded.
> 
> GC thread 1 now wakes up and runs the assert on nmethod B that it must not be unloaded, and we get the stack trace above.
> 
> So there is absolutely no harm with calling oops_do on nmethods that racingly become unloaded here, and that assertion 
> should just be silenced.
> 
> Webrev:
> http://cr.openjdk.java.net/~eosterlund/8219687/webrev.00/
> 
> Bug:
> https://bugs.openjdk.java.net/browse/JDK-8219687
> 
> Thanks,
> /Erik