On Tue, 30 Sep 2025 06:11:14 GMT, David Holmes <[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 >> >> The proposed fix to disable all event generation and then post the last >> (VMDeath) event. After this event is completed change VM phase to death. >> It's guaranteed that no any events are generated after VMDeath completion. >> >> After this fix the VMDeath callback also can't generate any events. >> The alternative is to disable events posting after VMDeath completion and >> before changing VM state. However, seems it is still a gap between vm death >> event completion and disabling events. So user can see events after VMDeath >> completion. >> >> It might be still possible also to wait while all currently executing >> events are completed. It is not required be specification, might add >> unneeded complexity. So I want to apply this fix first and then to check if >> we still any problems. >> Then, I plan to wait with timeout until all current (and queued) jvmti >> events are completed with some reasonable timeout. >> Currently, I haven't seen problems with this fix and >> https://bugs.openjdk.org/browse/JDK-8352654. > > src/hotspot/share/prims/jvmtiEventController.cpp line 1060: > >> 1058: void >> 1059: JvmtiEventControllerPrivate::vm_death() { >> 1060: _execution_finished = true; > > Unless there is some lock guarding this that I cannot see in this diff, if > you want to ensure this will be seen as soon as possible then you need a > `store_fence` (and the variable should be `volatile` - and will be a > candidate for `Atomic<T>`). You are still racing with others threads but this > would at least attempt to minimise the window. I forgot to put this in description and mentioned the first comment. The access to variable is protected with JvmtiThreadState_lock. Am I understand correctly, that it is enough to correctly synchronize it? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/27504#discussion_r2391984202
