RFR: 8355631: The events might be generated after VM_DEATH event [v4]

[Serguei Spitsyn]

On Mon, 27 Oct 2025 15:36:41 GMT, Leonid Mesnik <lmesnik at openjdk.org> 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