RFR[13]: 8224674: NMethod state machine is not monotonic

[Erik Österlund]

Any takers?

/Erik

On 2019-07-01 15:12, Erik Österlund wrote:
> Hi,
>
> Today it is up to callers of methods changing state on nmethods like 
> make_not_entrant(), to know all other possible concurrent attempts to 
> transition the nmethod, and know that there are no such attempts 
> trying to make the nmethod more dead.
> There have been multiple occurrences of issues where the caller got it 
> wrong due to the fragile nature of this code. This specific CR deals 
> with a bug where an OSR nmethod was made not entrant (deopt) and made 
> unloaded concurrently.
> The result of such a race can be that it is first made unloaded and 
> then made not entrant, making the nmethod go backwards in its state 
> machine, effectively resurrecting dead nmethods, causing a subsequent 
> GC to feel awkward (crash).
> But I have seen other similar incidents with deopt racing with the 
> sweeper. These non-monotonicity problems are unnecessary to have. So I 
> intend to fix the bug by enforcing monotonicity of the nmethod state 
> machine explicitly, instead of trying to reason about all callers of 
> these make_* functions.
> I swapped the order of unloaded and zombie in the enum as zombies are 
> strictly more dead than unloaded nmethods. All transitions change in 
> the direction of increasing deadness and fail if the transition is not 
> monotonically increasing.
>
> For ZGC I moved OSR nmethod unlinking to before the unlinking (where 
> unlinking code belongs), instead of after the handshake (intended for 
> deleting things safely unlinked).
> Strictly speaking, moving the OSR nmethod unlinking removes the racing 
> between make_not_entrant and make_unloaded, but I still want the 
> monotonicity guards to make this code more robust.
>
> I left AOT methods alone. Since they don't die, they don't have 
> resurrection problems, and hence do not benefit from these guards in 
> the same way.
>
> Bug:
> https://bugs.openjdk.java.net/browse/JDK-8224674
>
> Webrev:
> http://cr.openjdk.java.net/~eosterlund/8224674/webrev.00/
>
> Thanks,
> /Erik