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

[Serguei Spitsyn]

On Wed, 5 Mar 2025 02:24:54 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Yes, that's exactly what's happening. I think the issue with blocking suspend requests while being _thread_blocked is that we might create deadlocks.
>> 
>>> But how is any of this new?
>>>
>> We probably never checked for this scenario until this test was added in JDK-8284161. Since that moment we started checking the single step case. Looking at the code history, method `JvmtiExport::get_jvmti_thread_state()` (that leads to the blocked state) was only added later in JDK-8312174, on September 11, 2023. The first reported failure was September 17, 2023.
>
> I don't think we should block suspend requests in all cases of being `_thread_blocked`, but `get_jvmti_thread_state` is a special case. We either need to have a general suspension suppression mechanism (which I thought already existed - but may it was in old code), or we need to special-case the acquisition of the `Thread_State_lock`.

> Let me see if I have the sequence of events correct:
>  - we have a thread leaving Java to enter the VM, in this case to post a single-step event, which is not suspended
>   - when trying to acquire the JvmtiThreadState_lock the thread blocks and becomes handshake/safepoint-safe and is then suspended
>   - the thread acquires the lock and goes to post the event but because it has now been suspended it is an error to do so

This is almost right but let me add a couple of details.
A suspend request was processed/registered/accepted at the handshake/safepoint-safe point but physical suspension in the `ThreadSelfSuspensionHandshake` has not happened yet which created a problem. Also, the problem is not that the thread goes to post the event. Suspension and event posting are racy by design. The problem is that the `SingleStep` events for this thread are also enabled at the handshake/safepoint-safe point after the suspend request was processed/registered/accepted.

> So what we need is a way to disable suspension so that the transient/temporary switch to _thread_blocked does not allow a suspend request to be processed. I think that is preferable to trying to find where to add a place to check for suspension and actually suspend it.

I'm also thinking toward this direction. The best way to fix this would be to disallow processing/accepting new suspend requests in `MutexLocker mu(JvmtiThreadState_lock)` which is executed in context of `get_jvmti_thread_state()` call:

inline JvmtiThreadState* JvmtiThreadState::state_for(JavaThread *thread, Handle thread_handle) {
  // In a case of unmounted virtual thread the thread can be null.
  JvmtiThreadState* state = thread_handle == nullptr ? thread->jvmti_thread_state() :
                                                java_lang_Thread::jvmti_thread_state(thread_handle());
  if (state == nullptr) {
    MutexLocker mu(JvmtiThreadState_lock);
  . . .

Unfortunately, it looks like it is tricky to do.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23490#discussion_r1982384039