RFR: 8316397: StackTrace/Suspended/GetStackTraceSuspendedStressTest.java failed with: SingleStep event is NOT expected [v2]

[Patricio Chilano Mateo]

On Thu, 13 Feb 2025 17:37:27 GMT, Serguei Spitsyn <sspitsyn at openjdk.org> wrote:

>> The JVMTI functions `SuspendThread()`, `SuspendThreadList()` and `SuspendAllVirtualThreads()` use the runtime `HandshakeState::suspend()` function `SuspendThreadHandshake` class to suspend the `JavaThread` of a mounted virtual thread. They work under protection of the `JvmtiVTMSTransitionDisabler` to make the association of virtual thread with `JavaThread` stable. The function `HandshakeState::suspend()` creates an instance of`SuspendThreadHandshake`, executes it synchronously and then just returns. The `SuspendThreadHandshake:: do_thread()` in its order create an instance of the `ThreadSelfSuspensionHandshake` (which is a subclass of the `AsyncHandshakeClosure`) to force the handshakee's self-suspension asynchronously. The `HandshakeState::suspend()` does not wait for target thread real self-suspension, nor reaching a safe thread state that can be treated as a suspend-equivalent. This creates problems as the target virtual thread's activity can be observable after the JVMTI `Susp
 endThread()` and others are returned. For instance, some `SingleStep` events can be posted.
>> The fix is to wait in the `HandshakeState::suspend()` for the target handshakee to reach a safe thread state. This is done for the virtual thread case only. The suspension of normal platform threads remains the same.
>> 
>> Testing:
>>  - Ran mach5 tiers 1-6
>
> Serguei Spitsyn has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - removed obsolete fragment that was not removed in last update
>  - review: re-fixed the issue as initial fix was wrong

Thanks Serguei, some comments below.

> I treat it as VM/JVMTI bug as it looks silly when events are posted event though they were enabled after suspension. Yes, it is an attempt to do he saner thing.
>
Ok, makes sense.

> make this fix common for platform and virtual threads; I can try to update the failing test to prove we have the same issue for platform threads too; will correct the PR description then
>
For this particular event I don’t think the crash can happen with platform threads because get_jvmti_thread_state() will only block for virtual threads. And I don’t see any other blocking points in InterpreterRuntime::at_safepoint() other than that one.  But for other events it can happen since the thread might have been blocked and suspended at some point before (looking at post_class_prepare/post_class_load for example). This leads to another question, should we add this suspend check for all JvmtiExport::post_xxx methods?

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

PR Comment: https://git.openjdk.org/jdk/pull/23490#issuecomment-2669550122