8299240: rank of JvmtiVTMSTransition_lock can be safepoint

Reviewed-by: dholmes, coleenp, pchilanomate

src/hotspot/share/interpreter/interpreterRuntime.cpp

@@ -1122,10 +1122,6 @@ JRT_ENTRY(void, InterpreterRuntime::at_safepoint(JavaThread* current))
   // JRT_END does an implicit safepoint check, hence we are guaranteed to block
   // if this is called during a safepoint
 
-  if (java_lang_VirtualThread::notify_jvmti_events()) {
-    JvmtiExport::check_vthread_and_suspend_at_safepoint(current);
-  }
-
   if (JvmtiExport::should_post_single_step()) {
     // This function is called by the interpreter when single stepping. Such single
     // stepping could unwind a frame. Then, it is important that we process any frames

src/hotspot/share/prims/jvmtiEnv.cpp

@@ -928,13 +928,15 @@ JvmtiEnv::GetAllThreads(jint* threads_count_ptr, jthread** threads_ptr) {
 jvmtiError
 JvmtiEnv::SuspendThread(jthread thread) {
   JavaThread* current = JavaThread::current();
+  HandleMark hm(current);
+  Handle self_tobj;
 
   jvmtiError err;
-  JavaThread* java_thread = nullptr;
-  oop thread_oop = nullptr;
   {
     JvmtiVTMSTransitionDisabler disabler(true);
     ThreadsListHandle tlh(current);
+    JavaThread* java_thread = nullptr;
+    oop thread_oop = nullptr;
 
     err = get_threadOop_and_JavaThread(tlh.list(), thread, &java_thread, &thread_oop);
     if (err != JVMTI_ERROR_NONE) {
@@ -946,9 +948,11 @@ JvmtiEnv::SuspendThread(jthread thread) {
       err = suspend_thread(thread_oop, java_thread, /* single_suspend */ true, nullptr);
       return err;
     }
+    // protect thread_oop as a safepoint can be reached in disabler destructor
+    self_tobj = Handle(current, thread_oop);
   }
   // Do self suspend for current JavaThread.
-  err = suspend_thread(thread_oop, current, /* single_suspend */ true, nullptr);
+  err = suspend_thread(self_tobj(), current, /* single_suspend */ true, nullptr);
   return err;
 } /* end SuspendThread */
 
@@ -960,7 +964,7 @@ jvmtiError
 JvmtiEnv::SuspendThreadList(jint request_count, const jthread* request_list, jvmtiError* results) {
   JavaThread* current = JavaThread::current();
   HandleMark hm(current);
-  Handle self_tobj = Handle(current, nullptr);
+  Handle self_tobj;
   int self_idx = -1;
 
   {
@@ -1013,7 +1017,7 @@ JvmtiEnv::SuspendAllVirtualThreads(jint except_count, const jthread* except_list
   }
   JavaThread* current = JavaThread::current();
   HandleMark hm(current);
-  Handle self_tobj = Handle(current, nullptr);
+  Handle self_tobj;
 
   {
     ResourceMark rm(current);

src/hotspot/share/prims/jvmtiExport.cpp

@@ -1274,22 +1274,6 @@ bool              JvmtiExport::_should_post_vthread_unmount               = fals
 ////////////////////////////////////////////////////////////////////////////////////////////////
 
 
-void JvmtiExport::check_vthread_and_suspend_at_safepoint(JavaThread *thread) {
-  oop vt = thread->jvmti_vthread();
-
-  if (vt != nullptr && java_lang_VirtualThread::is_instance(vt)) {
-    int64_t id = java_lang_Thread::thread_id(vt);
-
-    ThreadBlockInVM tbivm(thread);
-    MonitorLocker ml(JvmtiVTMSTransition_lock, Mutex::_no_safepoint_check_flag);
-
-    // block while vthread is externally suspended
-    while (JvmtiVTSuspender::is_vthread_suspended(id)) {
-      ml.wait();
-    }
-  }
-}
-
 //
 // JVMTI single step management
 //

src/hotspot/share/prims/jvmtiExport.hpp

@@ -298,8 +298,6 @@ class JvmtiExport : public AllStatic {
   static void decode_version_values(jint version, int * major, int * minor,
                                     int * micro) NOT_JVMTI_RETURN;
 
-  static void check_vthread_and_suspend_at_safepoint(JavaThread *thread) NOT_JVMTI_RETURN;
-
   // single stepping management methods
   static void at_single_stepping_point(JavaThread *thread, Method* method, address location) NOT_JVMTI_RETURN;
   static void expose_single_stepping(JavaThread *thread) NOT_JVMTI_RETURN;

src/hotspot/share/prims/jvmtiThreadState.cpp

@@ -297,8 +297,7 @@ JvmtiVTMSTransitionDisabler::VTMS_transition_disable_for_one() {
   if (!java_lang_VirtualThread::is_instance(vth())) {
     return; // no-op if _thread is not a virtual thread
   }
-  ThreadBlockInVM tbivm(thread);
+  MonitorLocker ml(JvmtiVTMSTransition_lock);
-  MonitorLocker ml(JvmtiVTMSTransition_lock, Mutex::_no_safepoint_check_flag);
 
   while (_SR_mode) { // suspender or resumer is a JvmtiVTMSTransitionDisabler monopolist
     ml.wait(10); // wait while there is an active suspender or resumer
@@ -320,8 +319,7 @@ JvmtiVTMSTransitionDisabler::VTMS_transition_disable_for_all() {
   JavaThread* thread = JavaThread::current();
   int attempts = 50000;
   {
-    ThreadBlockInVM tbivm(thread);
+    MonitorLocker ml(JvmtiVTMSTransition_lock);
-    MonitorLocker ml(JvmtiVTMSTransition_lock, Mutex::_no_safepoint_check_flag);
 
     assert(!thread->is_in_tmp_VTMS_transition(), "sanity check");
     assert(!thread->is_in_VTMS_transition(), "VTMS_transition sanity check");
@@ -368,7 +366,7 @@ JvmtiVTMSTransitionDisabler::VTMS_transition_enable_for_one() {
   if (!java_lang_VirtualThread::is_instance(vth())) {
     return; // no-op if _thread is not a virtual thread
   }
-  MonitorLocker ml(JvmtiVTMSTransition_lock, Mutex::_no_safepoint_check_flag);
+  MonitorLocker ml(JvmtiVTMSTransition_lock);
   java_lang_Thread::dec_VTMS_transition_disable_count(vth());
   Atomic::dec(&_VTMS_transition_disable_for_one_count);
   if (_VTMS_transition_disable_for_one_count == 0 || _is_SR) {
@@ -384,7 +382,7 @@ void
 JvmtiVTMSTransitionDisabler::VTMS_transition_enable_for_all() {
   JavaThread* current = JavaThread::current();
   {
-    MonitorLocker ml(JvmtiVTMSTransition_lock, Mutex::_no_safepoint_check_flag);
+    MonitorLocker ml(JvmtiVTMSTransition_lock);
     assert(_VTMS_transition_disable_for_all_count > 0, "VTMS_transition sanity check");
 
     if (_is_SR) {  // Disabler is suspender or resumer.
@@ -427,8 +425,7 @@ JvmtiVTMSTransitionDisabler::start_VTMS_transition(jthread vthread, bool is_moun
     java_lang_Thread::set_is_in_VTMS_transition(vth(), false);
 
     while (true) {
-      ThreadBlockInVM tbivm(thread);
+      MonitorLocker ml(JvmtiVTMSTransition_lock);
-      MonitorLocker ml(JvmtiVTMSTransition_lock, Mutex::_no_safepoint_check_flag);
 
       // Do not allow suspends inside VTMS transitions.
       // Block while transitions are disabled or there are suspend requests.
@@ -477,7 +474,7 @@ JvmtiVTMSTransitionDisabler::finish_VTMS_transition(jthread vthread, bool is_mou
   // Unblock waiting VTMS transition disablers.
   if (_VTMS_transition_disable_for_one_count > 0 ||
       _VTMS_transition_disable_for_all_count > 0) {
-    MonitorLocker ml(JvmtiVTMSTransition_lock, Mutex::_no_safepoint_check_flag);
+    MonitorLocker ml(JvmtiVTMSTransition_lock);
     ml.notify_all();
   }
   // In unmount case the carrier thread is attached after unmount transition.
@@ -485,8 +482,7 @@ JvmtiVTMSTransitionDisabler::finish_VTMS_transition(jthread vthread, bool is_mou
   int attempts = 10000;
   if (!is_mount && thread->is_carrier_thread_suspended()) {
     while (true) {
-      ThreadBlockInVM tbivm(thread);
+      MonitorLocker ml(JvmtiVTMSTransition_lock);
-      MonitorLocker ml(JvmtiVTMSTransition_lock, Mutex::_no_safepoint_check_flag);
 
       // Block while there are suspend requests.
       if ((!is_mount && thread->is_carrier_thread_suspended()) ||
@@ -527,7 +523,7 @@ JvmtiVTSuspender::_not_suspended_list = new VirtualThreadList();
 
 void
 JvmtiVTSuspender::register_all_vthreads_suspend() {
-  MonitorLocker ml(JvmtiVTMSTransition_lock, Mutex::_no_safepoint_check_flag);
+  MonitorLocker ml(JvmtiVTMSTransition_lock);
 
   _SR_mode = SR_all;
   _suspended_list->invalidate();
@@ -536,7 +532,7 @@ JvmtiVTSuspender::register_all_vthreads_suspend() {
 
 void
 JvmtiVTSuspender::register_all_vthreads_resume() {
-  MonitorLocker ml(JvmtiVTMSTransition_lock, Mutex::_no_safepoint_check_flag);
+  MonitorLocker ml(JvmtiVTMSTransition_lock);
 
   _SR_mode = SR_none;
   _suspended_list->invalidate();
@@ -545,9 +541,9 @@ JvmtiVTSuspender::register_all_vthreads_resume() {
 
 void
 JvmtiVTSuspender::register_vthread_suspend(oop vt) {
-  MonitorLocker ml(JvmtiVTMSTransition_lock, Mutex::_no_safepoint_check_flag);
-
   int64_t id = java_lang_Thread::thread_id(vt);
+  MonitorLocker ml(JvmtiVTMSTransition_lock);
+
   if (_SR_mode == SR_all) {
     assert(_not_suspended_list->contains(id),
            "register_vthread_suspend sanity check");
@@ -562,9 +558,9 @@ JvmtiVTSuspender::register_vthread_suspend(oop vt) {
 
 void
 JvmtiVTSuspender::register_vthread_resume(oop vt) {
-  MonitorLocker ml(JvmtiVTMSTransition_lock, Mutex::_no_safepoint_check_flag);
-
   int64_t id = java_lang_Thread::thread_id(vt);
+  MonitorLocker ml(JvmtiVTMSTransition_lock);
+
   if (_SR_mode == SR_all) {
     assert(!_not_suspended_list->contains(id),
            "register_vthread_resume sanity check");

src/hotspot/share/runtime/mutexLocker.cpp

@@ -288,7 +288,7 @@ void mutex_init() {
 
   def(JvmtiThreadState_lock        , PaddedMutex  , safepoint);   // Used by JvmtiThreadState/JvmtiEventController
   def(EscapeBarrier_lock           , PaddedMonitor, nosafepoint); // Used to synchronize object reallocation/relocking triggered by JVMTI
-  def(JvmtiVTMSTransition_lock     , PaddedMonitor, nosafepoint); // used for Virtual Thread Mount State transition management
+  def(JvmtiVTMSTransition_lock     , PaddedMonitor, safepoint);   // used for Virtual Thread Mount State transition management
   def(Management_lock              , PaddedMutex  , safepoint);   // used for JVM management
 
   def(ConcurrentGCBreakpoints_lock , PaddedMonitor, safepoint, true);