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

[Patricio Chilano Mateo]

On Thu, 6 Feb 2025 10:45:29 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 `Suspe
 ndThread()` 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

Hi Serguei,

I investigated this issue but don’t agree with the patch. I added comments below.

Thanks,
Patricio

src/hotspot/share/runtime/handshake.cpp line 800:

> 798:       // for virtual thread to reach a safe state before the JVMTI SuspendThread is returned.
> 799:       while (_handshakee->thread_state() != _thread_blocked &&
> 800:              _handshakee->thread_state() != _thread_in_native) {

We already wait for the target to reach a handshake-safe state when executing the `SuspendThreadHandshake`. It’s just that we don’t wait until the target suspends in `do_self_suspend()` since that can take arbitrarily long (e.g. target just waiting in native).

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

Changes requested by pchilanomate (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/23490#pullrequestreview-2609789365
PR Review Comment: https://git.openjdk.org/jdk/pull/23490#discussion_r1951455308