RFR: 8340698: JVMTI FRAME_POP event is sometimes missed if NotifyFramePop is called as a method is returning [v2]

[Chris Plummer]

On Fri, 11 Oct 2024 09:26:42 GMT, Serguei Spitsyn <sspitsyn at openjdk.org> wrote:

>> There is a race between JVMTI NotifyFramePop function and FramePop event posting code.
>> The fix is to return JVMTI_ERROR_OPAQUE_FRAME if if a FramePop event with depth 0 is requested by NotifyFramePop at the time when the target frame is in exit epilogue, and MethodExit/FramePop events are being posted for it.
>> 
>> Testing:
>>  - verified locally with new test (developed by Chris): `serviceability/jvmti/events/NotifyFramePopStressTest`
>>  - TBD: mach5 tiers 1-6
>
> Serguei Spitsyn has updated the pull request incrementally with one additional commit since the last revision:
> 
>   minor comment tweak

Overall the fix looks good. I made a couple of minor suggestions for the test. Make sure you test on all platforms and with -Xcomp since the test is timing sensitive.

test/hotspot/jtreg/serviceability/jvmti/events/NotifyFramePopStressTest/NotifyFramePopStressTest.java line 57:

> 55:         Thread controlThread = new Thread(() -> control(testThread), "Control Thread");
> 56: 
> 57:         setFramePopNotificationMode(testThread, true);

I think we can get rid of this API and just have the native code default to enabling FRAME_POP events. This was copied from a test that would turn it on and off, but this test always needs it on.

test/hotspot/jtreg/serviceability/jvmti/events/NotifyFramePopStressTest/libNotifyFramePopStressTest.cpp line 156:

> 154: 
> 155:   // We only want to do a NotifyFramePop once for the main method. The sole purpose is
> 156:   // to force the thread into interpOnly mode, which seems to help the tests's timing

Suggestion:

  // to force the thread into interpOnly mode, which seems to help the test's timing

-------------

Marked as reviewed by cjplummer (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/21468#pullrequestreview-2370623582
PR Review Comment: https://git.openjdk.org/jdk/pull/21468#discussion_r1801978159
PR Review Comment: https://git.openjdk.org/jdk/pull/21468#discussion_r1801969813