8316682: serviceability/jvmti/vthread/SelfSuspendDisablerTest timed out

Reviewed-by: lmesnik, pchilanomate

Author: pchilano

src/hotspot/share/prims/jvmtiEnv.cpp

@@ -988,6 +988,10 @@ JvmtiEnv::SuspendThreadList(jint request_count, const jthread* request_list, jvm
         self_tobj = Handle(current, thread_oop);
         continue; // self suspend after all other suspends
       }
+      if (java_lang_VirtualThread::is_instance(thread_oop)) {
+        oop carrier_thread = java_lang_VirtualThread::carrier_thread(thread_oop);
+        java_thread = carrier_thread == nullptr ? nullptr : java_lang_Thread::thread(carrier_thread);
+      }
       results[i] = suspend_thread(thread_oop, java_thread, /* single_suspend */ true);
     }
   }
@@ -1114,6 +1118,10 @@ JvmtiEnv::ResumeThreadList(jint request_count, const jthread* request_list, jvmt
         continue;
       }
     }
+    if (java_lang_VirtualThread::is_instance(thread_oop)) {
+      oop carrier_thread = java_lang_VirtualThread::carrier_thread(thread_oop);
+      java_thread = carrier_thread == nullptr ? nullptr : java_lang_Thread::thread(carrier_thread);
+    }
     results[i] = resume_thread(thread_oop, java_thread, /* single_resume */ true);
   }
   // per-thread resume results returned via results parameter

src/hotspot/share/prims/jvmtiEnvBase.cpp

@@ -1398,8 +1398,7 @@ JvmtiEnvBase::is_vthread_suspended(oop vt_oop, JavaThread* jt) {
   bool suspended = false;
   if (java_lang_VirtualThread::is_instance(vt_oop)) {
     suspended = JvmtiVTSuspender::is_vthread_suspended(vt_oop);
-  }
-  if (vt_oop->is_a(vmClasses::BoundVirtualThread_klass())) {
+  } else if (vt_oop->is_a(vmClasses::BoundVirtualThread_klass())) {
     suspended = jt->is_suspended();
   }
   return suspended;
@@ -1757,66 +1756,55 @@ JvmtiEnvBase::suspend_thread(oop thread_oop, JavaThread* java_thread, bool singl
   Handle thread_h(current, thread_oop);
   bool is_virtual = java_lang_VirtualThread::is_instance(thread_h());
 
-  if (is_virtual) {
-    if (single_suspend) {
-      if (JvmtiVTSuspender::is_vthread_suspended(thread_h())) {
-        return JVMTI_ERROR_THREAD_SUSPENDED;
-      }
-      JvmtiVTSuspender::register_vthread_suspend(thread_h());
-      // Check if virtual thread is mounted and there is a java_thread.
-      // A non-null java_thread is always passed in the !single_suspend case.
-      oop carrier_thread = java_lang_VirtualThread::carrier_thread(thread_h());
-      java_thread = carrier_thread == nullptr ? nullptr : java_lang_Thread::thread(carrier_thread);
-    }
-    // The java_thread can be still blocked in VTMS transition after a previous JVMTI resume call.
-    // There is no need to suspend the java_thread in this case. After vthread unblocking,
-    // it will check for ext_suspend request and suspend itself if necessary.
-    if (java_thread == nullptr || java_thread->is_suspended()) {
-      // We are done if the virtual thread is unmounted or
-      // the java_thread is externally suspended.
-      return JVMTI_ERROR_NONE;
-    }
-    // The virtual thread is mounted: suspend the java_thread.
-  }
-  // Don't allow hidden thread suspend request.
-  if (java_thread->is_hidden_from_external_view()) {
-    return JVMTI_ERROR_NONE;
-  }
-  bool is_thread_carrying = is_thread_carrying_vthread(java_thread, thread_h());
+  // Unmounted vthread case.
 
-  // A case of non-virtual thread.
-  if (!is_virtual) {
-    // Thread.suspend() is used in some tests. It sets jt->is_suspended() only.
-    if (java_thread->is_carrier_thread_suspended() ||
-        (!is_thread_carrying && java_thread->is_suspended())) {
+  if (is_virtual && java_thread == nullptr) {
+    assert(single_suspend, "sanity check");
+    if (JvmtiVTSuspender::is_vthread_suspended(thread_h())) {
       return JVMTI_ERROR_THREAD_SUSPENDED;
     }
-    java_thread->set_carrier_thread_suspended();
+    JvmtiVTSuspender::register_vthread_suspend(thread_h());
+    return JVMTI_ERROR_NONE;
   }
+
+  // Platform thread or mounted vthread cases.
+
+  assert(java_thread != nullptr, "sanity check");
   assert(!java_thread->is_in_VTMS_transition(), "sanity check");
 
-  assert(!single_suspend || (!is_virtual && java_thread->is_carrier_thread_suspended()) ||
-          (is_virtual && JvmtiVTSuspender::is_vthread_suspended(thread_h())),
-         "sanity check");
+  // Don't allow hidden thread suspend request.
+  if (java_thread->is_hidden_from_external_view()) {
+    return JVMTI_ERROR_NONE;
+  }
 
   // An attempt to handshake-suspend a thread carrying a virtual thread will result in
   // suspension of mounted virtual thread. So, we just mark it as suspended
   // and it will be actually suspended at virtual thread unmount transition.
-  if (!is_thread_carrying) {
+  bool is_thread_carrying = is_thread_carrying_vthread(java_thread, thread_h());
+  if (is_thread_carrying) {
+    return java_thread->set_carrier_thread_suspended() ? JVMTI_ERROR_NONE : JVMTI_ERROR_THREAD_SUSPENDED;
+  } else {
+    // Platform thread (not carrying vthread) or mounted vthread cases.
     assert(thread_h() != nullptr, "sanity check");
     assert(single_suspend || thread_h()->is_a(vmClasses::BaseVirtualThread_klass()),
            "SuspendAllVirtualThreads should never suspend non-virtual threads");
-    // Case of mounted virtual or attached carrier thread.
-    if (!java_thread->java_suspend()) {
+
+    // Ideally we would just need to check java_thread->is_suspended(), but we have to
+    // consider the case of trying to suspend a thread that was previously suspended while
+    // carrying a vthread but has already unmounted it.
+    if (java_thread->is_suspended() || (!is_virtual && java_thread->is_carrier_thread_suspended())) {
+      return JVMTI_ERROR_THREAD_SUSPENDED;
+    }
+    if (!java_thread->java_suspend(is_virtual && single_suspend)) {
       // Thread is already suspended or in process of exiting.
       if (java_thread->is_exiting()) {
         // The thread was in the process of exiting.
         return JVMTI_ERROR_THREAD_NOT_ALIVE;
       }
       return JVMTI_ERROR_THREAD_SUSPENDED;
     }
+    return JVMTI_ERROR_NONE;
   }
-  return JVMTI_ERROR_NONE;
 }
 
 // java_thread - protected by ThreadsListHandle
@@ -1827,54 +1815,51 @@ JvmtiEnvBase::resume_thread(oop thread_oop, JavaThread* java_thread, bool single
   Handle thread_h(current, thread_oop);
   bool is_virtual = java_lang_VirtualThread::is_instance(thread_h());
 
-  if (is_virtual) {
-    if (single_resume) {
-      if (!JvmtiVTSuspender::is_vthread_suspended(thread_h())) {
-        return JVMTI_ERROR_THREAD_NOT_SUSPENDED;
-      }
-      JvmtiVTSuspender::register_vthread_resume(thread_h());
-      // Check if virtual thread is mounted and there is a java_thread.
-      // A non-null java_thread is always passed in the !single_resume case.
-      oop carrier_thread = java_lang_VirtualThread::carrier_thread(thread_h());
-      java_thread = carrier_thread == nullptr ? nullptr : java_lang_Thread::thread(carrier_thread);
-    }
-    // The java_thread can be still blocked in VTMS transition after a previous JVMTI suspend call.
-    // There is no need to resume the java_thread in this case. After vthread unblocking,
-    // it will check for is_vthread_suspended request and remain resumed if necessary.
-    if (java_thread == nullptr || !java_thread->is_suspended()) {
-      // We are done if the virtual thread is unmounted or
-      // the java_thread is not externally suspended.
-      return JVMTI_ERROR_NONE;
+  // Unmounted vthread case.
+
+  if (is_virtual && java_thread == nullptr) {
+    assert(single_resume, "sanity check");
+    if (!JvmtiVTSuspender::is_vthread_suspended(thread_h())) {
+      return JVMTI_ERROR_THREAD_NOT_SUSPENDED;
     }
-    // The virtual thread is mounted and java_thread is supended: resume the java_thread.
+    JvmtiVTSuspender::register_vthread_resume(thread_h());
+    return JVMTI_ERROR_NONE;
   }
+
+  // Platform thread or mounted vthread cases.
+
+  assert(java_thread != nullptr, "sanity check");
+  assert(!java_thread->is_in_VTMS_transition(), "sanity check");
+
   // Don't allow hidden thread resume request.
   if (java_thread->is_hidden_from_external_view()) {
     return JVMTI_ERROR_NONE;
   }
-  bool is_thread_carrying = is_thread_carrying_vthread(java_thread, thread_h());
 
-  // A case of a non-virtual thread.
-  if (!is_virtual) {
-    if (!java_thread->is_carrier_thread_suspended() &&
-        (is_thread_carrying || !java_thread->is_suspended())) {
-      return JVMTI_ERROR_THREAD_NOT_SUSPENDED;
-    }
-    java_thread->clear_carrier_thread_suspended();
-  }
-  assert(!java_thread->is_in_VTMS_transition(), "sanity check");
+  bool is_thread_carrying = is_thread_carrying_vthread(java_thread, thread_h());
+  if (is_thread_carrying) {
+    return java_thread->clear_carrier_thread_suspended() ? JVMTI_ERROR_NONE : JVMTI_ERROR_THREAD_NOT_SUSPENDED;
+  } else {
+    // Platform thread (not carrying vthread) or mounted vthread cases.
 
-  if (!is_thread_carrying) {
     assert(thread_h() != nullptr, "sanity check");
     assert(single_resume || thread_h()->is_a(vmClasses::BaseVirtualThread_klass()),
            "ResumeAllVirtualThreads should never resume non-virtual threads");
-    if (java_thread->is_suspended()) {
-      if (!java_thread->java_resume()) {
-        return JVMTI_ERROR_THREAD_NOT_SUSPENDED;
-      }
+
+    // Ideally we would not have to check this but we have to consider the case
+    // of trying to resume a thread that was previously suspended while carrying
+    // a vthread but has already unmounted it.
+    if (!is_virtual && java_thread->is_carrier_thread_suspended()) {
+      bool res = java_thread->clear_carrier_thread_suspended();
+      assert(res, "resume operations running concurrently?");
+      return JVMTI_ERROR_NONE;
+    }
+
+    if (!java_thread->java_resume(is_virtual && single_resume)) {
+      return JVMTI_ERROR_THREAD_NOT_SUSPENDED;
     }
+    return JVMTI_ERROR_NONE;
   }
-  return JVMTI_ERROR_NONE;
 }
 
 ResourceTracker::ResourceTracker(JvmtiEnv* env) {

src/hotspot/share/prims/jvmtiImpl.cpp

@@ -658,7 +658,7 @@ vframe *VM_GetOrSetLocal::get_vframe() {
 
 javaVFrame *VM_GetOrSetLocal::get_java_vframe() {
   vframe* vf = get_vframe();
-  if (!(_self || _thread->is_carrier_thread_suspended())) {
+  if (!_self && !_thread->is_suspended() && !_thread->is_carrier_thread_suspended()) {
     _result = JVMTI_ERROR_THREAD_NOT_SUSPENDED;
     return nullptr;
   }

src/hotspot/share/prims/jvmtiThreadState.cpp

@@ -254,29 +254,37 @@ JvmtiVTMSTransitionDisabler::print_info() {
 // disable VTMS transitions for one virtual thread
 // disable VTMS transitions for all threads if thread is nullptr or a platform thread
 JvmtiVTMSTransitionDisabler::JvmtiVTMSTransitionDisabler(jthread thread)
-  : _is_SR(false), _thread(thread)
+  : _is_SR(false),
+    _is_virtual(false),
+    _is_self(false),
+    _thread(thread)
 {
   if (!Continuations::enabled()) {
     return; // JvmtiVTMSTransitionDisabler is no-op without virtual threads
   }
   if (Thread::current_or_null() == nullptr) {
     return;  // Detached thread, can be a call from Agent_OnLoad.
   }
+  JavaThread* current = JavaThread::current();
+  oop thread_oop = JNIHandles::resolve_external_guard(thread);
+  _is_virtual = java_lang_VirtualThread::is_instance(thread_oop);
+
+  if (thread == nullptr ||
+      (!_is_virtual && thread_oop == current->threadObj()) ||
+      (_is_virtual && thread_oop == current->vthread())) {
+    _is_self = true;
+    return; // no need for current thread to disable and enable transitions for itself
+  }
   if (!sync_protocol_enabled_permanently()) {
     JvmtiVTMSTransitionDisabler::inc_sync_protocol_enabled_count();
   }
-  oop thread_oop = JNIHandles::resolve_external_guard(thread);
 
   // Target can be virtual or platform thread.
   // If target is a platform thread then we have to disable VTMS transitions for all threads.
   // It is by several reasons:
   // - carrier threads can mount virtual threads which may cause incorrect behavior
   // - there is no mechanism to disable transitions for a specific carrier thread yet
-  if (!java_lang_VirtualThread::is_instance(thread_oop)) {
-    _thread = nullptr; // target is a platform thread, switch to disabling VTMS transitions for all threads
-  }
-
-  if (_thread != nullptr) {
+  if (_is_virtual) {
     VTMS_transition_disable_for_one(); // disable VTMS transitions for one virtual thread
   } else {
     VTMS_transition_disable_for_all(); // disable VTMS transitions for all virtual threads
@@ -285,7 +293,10 @@ JvmtiVTMSTransitionDisabler::JvmtiVTMSTransitionDisabler(jthread thread)
 
 // disable VTMS transitions for all virtual threads
 JvmtiVTMSTransitionDisabler::JvmtiVTMSTransitionDisabler(bool is_SR)
-  : _is_SR(is_SR), _thread(nullptr)
+  : _is_SR(is_SR),
+    _is_virtual(false),
+    _is_self(false),
+    _thread(nullptr)
 {
   if (!Continuations::enabled()) {
     return; // JvmtiVTMSTransitionDisabler is no-op without virtual threads
@@ -309,7 +320,10 @@ JvmtiVTMSTransitionDisabler::~JvmtiVTMSTransitionDisabler() {
   if (Thread::current_or_null() == nullptr) {
     return;  // Detached thread, can be a call from Agent_OnLoad.
   }
-  if (_thread != nullptr) {
+  if (_is_self) {
+    return; // no need for current thread to disable and enable transitions for itself
+  }
+  if (_is_virtual) {
     VTMS_transition_enable_for_one(); // enable VTMS transitions for one virtual thread
   } else {
     VTMS_transition_enable_for_all(); // enable VTMS transitions for all virtual threads
@@ -684,7 +698,7 @@ JvmtiVTSuspender::_not_suspended_list = new VirtualThreadList();
 
 void
 JvmtiVTSuspender::register_all_vthreads_suspend() {
-  MonitorLocker ml(JvmtiVTMSTransition_lock);
+  MutexLocker ml(JvmtiVThreadSuspend_lock, Mutex::_no_safepoint_check_flag);
 
   _SR_mode = SR_all;
   _suspended_list->invalidate();
@@ -693,7 +707,7 @@ JvmtiVTSuspender::register_all_vthreads_suspend() {
 
 void
 JvmtiVTSuspender::register_all_vthreads_resume() {
-  MonitorLocker ml(JvmtiVTMSTransition_lock);
+  MutexLocker ml(JvmtiVThreadSuspend_lock, Mutex::_no_safepoint_check_flag);
 
   _SR_mode = SR_none;
   _suspended_list->invalidate();
@@ -703,7 +717,7 @@ JvmtiVTSuspender::register_all_vthreads_resume() {
 void
 JvmtiVTSuspender::register_vthread_suspend(oop vt) {
   int64_t id = java_lang_Thread::thread_id(vt);
-  MonitorLocker ml(JvmtiVTMSTransition_lock);
+  MutexLocker ml(JvmtiVThreadSuspend_lock, Mutex::_no_safepoint_check_flag);
 
   if (_SR_mode == SR_all) {
     assert(_not_suspended_list->contains(id),
@@ -720,7 +734,7 @@ JvmtiVTSuspender::register_vthread_suspend(oop vt) {
 void
 JvmtiVTSuspender::register_vthread_resume(oop vt) {
   int64_t id = java_lang_Thread::thread_id(vt);
-  MonitorLocker ml(JvmtiVTMSTransition_lock);
+  MutexLocker ml(JvmtiVThreadSuspend_lock, Mutex::_no_safepoint_check_flag);
 
   if (_SR_mode == SR_all) {
     assert(!_not_suspended_list->contains(id),

src/hotspot/share/prims/jvmtiThreadState.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2024, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 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
@@ -86,6 +86,8 @@ class JvmtiVTMSTransitionDisabler {
   static volatile bool _sync_protocol_enabled_permanently; // seen a suspender: JvmtiVTMSTransitionDisabler protocol is enabled permanently
 
   bool _is_SR;                                           // is suspender or resumer
+  bool _is_virtual;                                      // target thread is virtual
+  bool _is_self;                                         // JvmtiVTMSTransitionDisabler is a no-op for current platform, carrier or virtual thread
   jthread _thread;                                       // virtual thread to disable transitions for, no-op if it is a platform thread
 
   DEBUG_ONLY(static void print_info();)