8367725: Incorrect reading of oop in SuspendResumeManager::suspend while thread is blocked

Reviewed-by: pchilanomate, dholmes, sspitsyn

src/hotspot/share/prims/jvmtiThreadState.cpp

@@ -714,8 +714,7 @@ JvmtiVTSuspender::register_all_vthreads_resume() {
 }
 
 void
-JvmtiVTSuspender::register_vthread_suspend(oop vt) {
-  int64_t id = java_lang_Thread::thread_id(vt);
+JvmtiVTSuspender::register_vthread_suspend(int64_t id) {
   MutexLocker ml(JvmtiVThreadSuspend_lock, Mutex::_no_safepoint_check_flag);
 
   if (_SR_mode == SR_all) {
@@ -730,6 +729,12 @@ JvmtiVTSuspender::register_vthread_suspend(oop vt) {
   }
 }
 
+void
+JvmtiVTSuspender::register_vthread_suspend(oop vt) {
+  int64_t id = java_lang_Thread::thread_id(vt);
+  register_vthread_suspend(id);
+}
+
 void
 JvmtiVTSuspender::register_vthread_resume(oop vt) {
   int64_t id = java_lang_Thread::thread_id(vt);

src/hotspot/share/prims/jvmtiThreadState.hpp

@@ -167,6 +167,7 @@ class JvmtiVTSuspender : AllStatic {
  public:
   static void register_all_vthreads_suspend();
   static void register_all_vthreads_resume();
+  static void register_vthread_suspend(int64_t id);
   static void register_vthread_suspend(oop vt);
   static void register_vthread_resume(oop vt);
   static bool is_vthread_suspended(oop vt);

src/hotspot/share/runtime/suspendResumeManager.cpp

@@ -81,15 +81,28 @@ void SuspendResumeManager::set_suspended(bool is_suspend, bool register_vthread_
   AtomicAccess::store(&_suspended, is_suspend);
 }
 
+void SuspendResumeManager::set_suspended_current_thread(int64_t vthread_id, bool register_vthread_SR) {
+  assert(_target == JavaThread::current(), "should be current thread");
+#if INCLUDE_JVMTI
+  if (register_vthread_SR) {
+    assert(_target->is_vthread_mounted(), "sanity check");
+    JvmtiVTSuspender::register_vthread_suspend(vthread_id);
+  }
+#endif
+  AtomicAccess::store(&_suspended, true);
+}
+
 bool SuspendResumeManager::suspend(bool register_vthread_SR) {
   JVMTI_ONLY(assert(!_target->is_in_VTMS_transition(), "no suspend allowed in VTMS transition");)
   JavaThread* self = JavaThread::current();
   if (_target == self) {
     // If target is the current thread we can bypass the handshake machinery
-    // and just suspend directly
+    // and just suspend directly.
+    // The vthread() oop must only be accessed before state is set to _thread_blocked.
+    int64_t id = java_lang_Thread::thread_id(_target->vthread());
     ThreadBlockInVM tbivm(self);
     MutexLocker ml(_state_lock, Mutex::_no_safepoint_check_flag);
-    set_suspended(true, register_vthread_SR);
+    set_suspended_current_thread(id, register_vthread_SR);
     do_owner_suspend();
     return true;
   } else {

src/hotspot/share/runtime/suspendResumeManager.hpp

@@ -59,6 +59,11 @@ class SuspendResumeManager {
 
   void set_suspended(bool to, bool register_vthread_SR);
 
+  // The specific 'set_suspended' implementation only for self suspend.
+  // It is called when the thread is already blocked, and it is not possible to
+  // read oop for vthread from the _target.
+  void set_suspended_current_thread(int64_t vthread_id, bool register_vthread_SR);
+
   bool is_suspended() {
     return AtomicAccess::load(&_suspended);
   }

test/hotspot/jtreg/serviceability/jvmti/vthread/SelfSuspendDisablerTest/SelfSuspendDisablerTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2021, 2025, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -82,37 +82,47 @@ public class SelfSuspendDisablerTest {
         });
         Thread t2 = Thread.ofVirtual().factory().newThread(() -> {
             testJvmtiThreadState(Thread.currentThread(), RUNNABLE);
-            while(!isSuspended(t1)) {
+            selfSuspend();
+        });
+        Thread t3 = Thread.ofVirtual().factory().newThread(() -> {
+            testJvmtiThreadState(Thread.currentThread(), RUNNABLE);
+            while(!isSuspended(t1) || !isSuspended(t2)) {
               Thread.yield();
             }
             Thread.yield(); // provoke unmount
 
             testJvmtiThreadState(t1, SUSPENDED);
+            testJvmtiThreadState(t2, SUSPENDED);
 
             resume(t1);
+            resume(t2);
 
             suspendAllVirtualThreads();
         });
 
         testJvmtiThreadState(t1, NEW);
         testJvmtiThreadState(t2, NEW);
+        testJvmtiThreadState(t3, NEW);
 
         t1.start();
         t2.start();
+        t3.start();
 
-        while(!isSuspended(t2)) {
+        while(!isSuspended(t3)) {
             sleep(100);
         }
 
-        testJvmtiThreadState(t2, SUSPENDED);
+        testJvmtiThreadState(t3, SUSPENDED);
 
         resumeAllVirtualThreads();
 
+        t3.join();
         t2.join();
         t1.join();
 
         testJvmtiThreadState(t1, TERMINATED);
         testJvmtiThreadState(t2, TERMINATED);
+        testJvmtiThreadState(t3, TERMINATED);
     }
 
 }