On Mon, 27 Oct 2025 15:36:41 GMT, Leonid Mesnik <[email protected]> wrote:
>> The JVMTI spec says: >> https://docs.oracle.com/en/java/javase/24/docs/specs/jvmti.html#VMDeath >> `The VM death event notifies the agent of the termination of the VM. No >> events will occur after the VMDeath event.` >> >> However, current implementation changes state and only after this start >> disabling events. >> >> It might be not a conformance issue, because there is no way to get thread >> state in the very beginning of event. >> The main practical issue is that currently certain events are generated when >> VM is becoming dead. So any function in event should check error against >> JVMTI_PHASE_DEAD. We can easily trigger it by running tests with enabled >> https://bugs.openjdk.org/browse/JDK-8352654 >> >> Also, it would be useful to guarantee that VM_DEATH is last event so users >> can safely close/destroy all supported all structures used by Jvmti agent >> (like RawMonitors). >> >> The proposed fix is to stop events posting and wait for already executing >> events before vm_death is posted. >> >> Currently, I haven't seen problems with this fix and >> https://bugs.openjdk.org/browse/JDK-8352654. > > Leonid Mesnik has updated the pull request incrementally with one additional > commit since the last revision: > > fixed after David's comments Thank you for updates! It looks great now. I've posted a couple of nits though. src/hotspot/share/prims/jvmtiExport.cpp line 145: > 143: // and the _in_callback_count contains the number of callbacks still in > progress. > 144: #define JVMTI_JAVA_EVENT_CALLBACK_BLOCK(thread) > JvmtiJavaThreadEventTransition jet(thread); if > (JvmtiEventController::is_execution_finished()) { return; } > 145: #define JVMTI_EVENT_CALLBACK_BLOCK(thread) JvmtiThreadEventTransition > jet(thread); if (JvmtiEventController::is_execution_finished()) { return; } Nit: It is better to keep macro names congruent with matching classes: s/JVMTI_JAVA_EVENT_CALLBACK_BLOCK/JVMTI_JAVA_THREAD_EVENT_CALLBACK_BLOCK/g s/JVMTI_EVENT_CALLBACK_BLOCK/JVMTI_THREAD_EVENT_CALLBACK_BLOCK/g Also, I'd suggest to use slashes in these macro definitions: #define JVMTI_JAVA_THREAD_EVENT_CALLBACK_BLOCK(thread) \ JvmtiJavaThreadEventTransition jet(thread); \ if (JvmtiEventController::is_execution_finished()) { \ return; \ } ------------- Marked as reviewed by sspitsyn (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/27504#pullrequestreview-3386029120 PR Review Comment: https://git.openjdk.org/jdk/pull/27504#discussion_r2467495132
