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

[Erik Osterlund]

Hi Tobias,

Thanks for the review.

/Erik

> On 9 Jul 2019, at 07:13, Tobias Hartmann <tobias.hartmann at oracle.com> wrote:
> 
> 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