On Fri, 11 Oct 2024 09:26:42 GMT, Serguei Spitsyn <sspit...@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
