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

[Alex Menkov]

On Thu, 17 Oct 2024 02:33:01 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:
> 
>   review test tweaks: add @bug tag; a reliability update

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

> 60:          jmethodID method, jboolean wasPoppedByException) {
> 61:   jvmtiError err;
> 62:   char* expected_method = (char*)last_notify_method;

I don't think caching `last_notify_method` adds any reliability.
`notifyFramePop` deallocates the memory.

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

> 85:   }
> 86:   deallocate(jvmti, jni, csig);
> 87:   deallocate(jvmti, jni, name);

on error `csig` and `name` are deallocated twice

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

> 169:   } else {
> 170:     char* old_notify_method = (char*)last_notify_method;
> 171:     last_notify_method = name;

I don't think this adds reliability.
Control thread suspends main test thread before call this method. AFAIU suspend cannot complete before FramePop callback is completed.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21468#discussion_r1805369268
PR Review Comment: https://git.openjdk.org/jdk/pull/21468#discussion_r1805370875
PR Review Comment: https://git.openjdk.org/jdk/pull/21468#discussion_r1805388024