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

[Tobias Hartmann]

Hi Erik,

this looks reasonable to me but a second review would be good.

Please test thoroughly before pushing.

Best regards,
Tobias

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