8311218: fatal error: stuck in JvmtiVTMSTransitionDisabler::VTMS_transition_disable

make/data/hotspot-symbols/symbols-unix

@@ -223,7 +223,7 @@ JVM_VirtualThreadEnd
 JVM_VirtualThreadMount
 JVM_VirtualThreadUnmount
 JVM_VirtualThreadHideFrames
-JVM_VirtualThreadCriticalLock
+JVM_VirtualThreadDisableSuspend
 
 # Scoped values
 JVM_EnsureMaterializedForStackWalk_func

src/hotspot/share/classfile/vmIntrinsics.hpp

@@ -597,7 +597,7 @@ class methodHandle;
   do_intrinsic(_notifyJvmtiVThreadMount, java_lang_VirtualThread, notifyJvmtiMount_name, bool_void_signature, F_RN)     \
   do_intrinsic(_notifyJvmtiVThreadUnmount, java_lang_VirtualThread, notifyJvmtiUnmount_name, bool_void_signature, F_RN) \
   do_intrinsic(_notifyJvmtiVThreadHideFrames, java_lang_VirtualThread, notifyJvmtiHideFrames_name, bool_void_signature, F_RN) \
-  do_intrinsic(_notifyJvmtiVThreadCriticalLock, java_lang_VirtualThread, notifyJvmtiCriticalLock_name, bool_void_signature, F_RN) \
+  do_intrinsic(_notifyJvmtiVThreadDisableSuspend, java_lang_VirtualThread, notifyJvmtiDisableSuspend_name, bool_void_signature, F_RN) \
                                                                                                                         \
   /* support for UnsafeConstants */                                                                                     \
   do_class(jdk_internal_misc_UnsafeConstants,      "jdk/internal/misc/UnsafeConstants")                                 \

src/hotspot/share/classfile/vmSymbols.hpp

@@ -421,7 +421,7 @@ class SerializeClosure;
   template(notifyJvmtiMount_name,                     "notifyJvmtiMount")                         \
   template(notifyJvmtiUnmount_name,                   "notifyJvmtiUnmount")                       \
   template(notifyJvmtiHideFrames_name,                "notifyJvmtiHideFrames")                    \
-  template(notifyJvmtiCriticalLock_name,              "notifyJvmtiCriticalLock")                  \
+  template(notifyJvmtiDisableSuspend_name,            "notifyJvmtiDisableSuspend")                \
   template(doYield_name,                              "doYield")                                  \
   template(enter_name,                                "enter")                                    \
   template(enterSpecial_name,                         "enterSpecial")                             \

src/hotspot/share/include/jvm.h

@@ -1155,7 +1155,7 @@ JNIEXPORT void JNICALL
 JVM_VirtualThreadHideFrames(JNIEnv* env, jobject vthread, jboolean hide);
 
 JNIEXPORT void JNICALL
-JVM_VirtualThreadCriticalLock(JNIEnv* env, jobject vthread, jboolean enter);
+JVM_VirtualThreadDisableSuspend(JNIEnv* env, jobject vthread, jboolean enter);
 
 /*
  * Core reflection support.

src/hotspot/share/jvmci/vmStructs_jvmci.cpp

@@ -220,7 +220,7 @@
   nonstatic_field(JavaThread,                  _lock_stack,                                   LockStack)                             \
   JVMTI_ONLY(nonstatic_field(JavaThread,       _is_in_VTMS_transition,                        bool))                                 \
   JVMTI_ONLY(nonstatic_field(JavaThread,       _is_in_tmp_VTMS_transition,                    bool))                                 \
-  JVMTI_ONLY(nonstatic_field(JavaThread,       _is_in_critical_section,                       bool))                                 \
+  JVMTI_ONLY(nonstatic_field(JavaThread,       _is_disable_suspend,                           bool))                                 \
                                                                                                                                      \
   nonstatic_field(LockStack,                   _top,                                          uint32_t)                              \
                                                                                                                                      \

src/hotspot/share/opto/c2compiler.cpp

@@ -822,7 +822,7 @@ bool C2Compiler::is_intrinsic_supported(vmIntrinsics::ID id) {
   case vmIntrinsics::_notifyJvmtiVThreadMount:
   case vmIntrinsics::_notifyJvmtiVThreadUnmount:
   case vmIntrinsics::_notifyJvmtiVThreadHideFrames:
-  case vmIntrinsics::_notifyJvmtiVThreadCriticalLock:
+  case vmIntrinsics::_notifyJvmtiVThreadDisableSuspend:
 #endif
     break;
 

src/hotspot/share/opto/library_call.cpp

@@ -492,8 +492,8 @@ bool LibraryCallKit::try_to_inline(int predicate) {
                                                                                          "notifyJvmtiMount", false, false);
   case vmIntrinsics::_notifyJvmtiVThreadUnmount: return inline_native_notify_jvmti_funcs(CAST_FROM_FN_PTR(address, OptoRuntime::notify_jvmti_vthread_unmount()),
                                                                                          "notifyJvmtiUnmount", false, false);
-  case vmIntrinsics::_notifyJvmtiVThreadHideFrames: return inline_native_notify_jvmti_hide();
-  case vmIntrinsics::_notifyJvmtiVThreadCriticalLock: return inline_native_notify_jvmti_sync();
+  case vmIntrinsics::_notifyJvmtiVThreadHideFrames:     return inline_native_notify_jvmti_hide();
+  case vmIntrinsics::_notifyJvmtiVThreadDisableSuspend: return inline_native_notify_jvmti_sync();
 #endif
 
 #ifdef JFR_HAVE_INTRINSICS
@@ -2951,18 +2951,18 @@ bool LibraryCallKit::inline_native_notify_jvmti_hide() {
   return true;
 }
 
-// Always update the is_in_critical_section bit.
+// Always update the is_disable_suspend bit.
 bool LibraryCallKit::inline_native_notify_jvmti_sync() {
   if (!DoJVMTIVirtualThreadTransitions) {
     return true;
   }
   IdealKit ideal(this);
 
   {
-    // unconditionally update the is_in_critical_section bit in current JavaThread
+    // unconditionally update the is_disable_suspend bit in current JavaThread
     Node* thread = ideal.thread();
-    Node* arg = _gvn.transform(argument(1)); // argument for critical section notification
-    Node* addr = basic_plus_adr(thread, in_bytes(JavaThread::is_in_critical_section_offset()));
+    Node* arg = _gvn.transform(argument(1)); // argument for notification
+    Node* addr = basic_plus_adr(thread, in_bytes(JavaThread::is_disable_suspend_offset()));
     const TypePtr *addr_type = _gvn.type(addr)->isa_ptr();
 
     sync_kit(ideal);

src/hotspot/share/prims/jvm.cpp

@@ -4008,16 +4008,16 @@ JVM_ENTRY(void, JVM_VirtualThreadHideFrames(JNIEnv* env, jobject vthread, jboole
 #endif
 JVM_END
 
-// Notification from VirtualThread about entering/exiting sync critical section.
+// Notification from VirtualThread about disabling JVMTI Suspend in a sync critical section.
 // Needed to avoid deadlocks with JVMTI suspend mechanism.
-JVM_ENTRY(void, JVM_VirtualThreadCriticalLock(JNIEnv* env, jobject vthread, jboolean enter))
+JVM_ENTRY(void, JVM_VirtualThreadDisableSuspend(JNIEnv* env, jobject vthread, jboolean enter))
 #if INCLUDE_JVMTI
   if (!DoJVMTIVirtualThreadTransitions) {
     assert(!JvmtiExport::can_support_virtual_threads(), "sanity check");
     return;
   }
-  assert(thread->is_in_critical_section() != (bool)enter, "sanity check");
-  thread->toggle_is_in_critical_section();
+  assert(thread->is_disable_suspend() != (bool)enter, "recursive disable suspend is not allowed");
+  thread->toggle_is_disable_suspend();
 #else
   fatal("Should only be called with JVMTI enabled");
 #endif

src/hotspot/share/runtime/handshake.cpp

@@ -487,8 +487,8 @@ HandshakeOperation* HandshakeState::get_op_for_self(bool allow_suspend, bool che
   assert(_handshakee == Thread::current(), "Must be called by self");
   assert(_lock.owned_by_self(), "Lock must be held");
   assert(allow_suspend || !check_async_exception, "invalid case");
-  if (allow_suspend && _handshakee->is_in_critical_section()) {
-    // avoid dealocks between VT critical sections and JVMTI suspend mechanism
+  if (allow_suspend && _handshakee->is_disable_suspend()) {
+    // filter out suspend operations while JavaThread is in disable_suspend mode
     allow_suspend = false;
   }
   if (!allow_suspend) {

src/hotspot/share/runtime/javaThread.cpp

@@ -440,7 +440,7 @@ JavaThread::JavaThread() :
   _carrier_thread_suspended(false),
   _is_in_VTMS_transition(false),
   _is_in_tmp_VTMS_transition(false),
-  _is_in_critical_section(false),
+  _is_disable_suspend(false),
 #ifdef ASSERT
   _is_VTMS_transition_disabler(false),
 #endif

src/hotspot/share/runtime/javaThread.hpp

@@ -317,7 +317,7 @@ class JavaThread: public Thread {
   volatile bool         _carrier_thread_suspended;       // Carrier thread is externally suspended
   bool                  _is_in_VTMS_transition;          // thread is in virtual thread mount state transition
   bool                  _is_in_tmp_VTMS_transition;      // thread is in temporary virtual thread mount state transition
-  bool                  _is_in_critical_section;         // thread is in a locking critical section
+  bool                  _is_disable_suspend;             // JVMTI suspend is temporarily disabled; used on current thread only
 #ifdef ASSERT
   bool                  _is_VTMS_transition_disabler;    // thread currently disabled VTMS transitions
 #endif
@@ -648,8 +648,8 @@ class JavaThread: public Thread {
   void set_is_in_VTMS_transition(bool val);
   void toggle_is_in_tmp_VTMS_transition()        { _is_in_tmp_VTMS_transition = !_is_in_tmp_VTMS_transition; };
 
-  bool is_in_critical_section() const            { return _is_in_critical_section; }
-  void toggle_is_in_critical_section()           { _is_in_critical_section = !_is_in_critical_section; };
+  bool is_disable_suspend() const                { return _is_disable_suspend; }
+  void toggle_is_disable_suspend()               { _is_disable_suspend = !_is_disable_suspend; };
 
 #ifdef ASSERT
   bool is_VTMS_transition_disabler() const       { return _is_VTMS_transition_disabler; }
@@ -815,7 +815,7 @@ class JavaThread: public Thread {
 #if INCLUDE_JVMTI
   static ByteSize is_in_VTMS_transition_offset()     { return byte_offset_of(JavaThread, _is_in_VTMS_transition); }
   static ByteSize is_in_tmp_VTMS_transition_offset() { return byte_offset_of(JavaThread, _is_in_tmp_VTMS_transition); }
-  static ByteSize is_in_critical_section_offset()    { return byte_offset_of(JavaThread, _is_in_critical_section); }
+  static ByteSize is_disable_suspend_offset()        { return byte_offset_of(JavaThread, _is_disable_suspend); }
 #endif
 
   // Returns the jni environment for this thread

src/java.base/share/classes/java/lang/VirtualThread.java

@@ -358,16 +358,11 @@ private void mount() {
         if (interrupted) {
             carrier.setInterrupt();
         } else if (carrier.isInterrupted()) {
-            try {
-                notifyJvmtiCriticalLock(true);
-                synchronized (interruptLock) {
-                    // need to recheck interrupt status
-                    if (!interrupted) {
-                        carrier.clearInterrupt();
-                    }
+            synchronized (interruptLock) {
+                // need to recheck interrupt status
+                if (!interrupted) {
+                    carrier.clearInterrupt();
                 }
-            } finally {
-                notifyJvmtiCriticalLock(false);
             }
         }
 
@@ -386,14 +381,9 @@ private void unmount() {
         Thread carrier = this.carrierThread;
         carrier.setCurrentThread(carrier);
 
-        try {
-            notifyJvmtiCriticalLock(true);
-            // break connection to carrier thread, synchronized with interrupt
-            synchronized (interruptLock) {
-                setCarrierThread(null);
-            }
-        } finally {
-            notifyJvmtiCriticalLock(false);
+        // break connection to carrier thread, synchronized with interrupt
+        synchronized (interruptLock) {
+            setCarrierThread(null);
         }
         carrier.clearInterrupt();
 
@@ -753,7 +743,7 @@ void unpark() {
                 }
             } else if ((s == PINNED) || (s == TIMED_PINNED)) {
                 try {
-                    notifyJvmtiCriticalLock(true);
+                    notifyJvmtiDisableSuspend(true);
                     // unpark carrier thread when pinned
                     synchronized (carrierThreadAccessLock()) {
                         Thread carrier = carrierThread;
@@ -762,7 +752,7 @@ void unpark() {
                         }
                     }
                 } finally {
-                    notifyJvmtiCriticalLock(false);
+                    notifyJvmtiDisableSuspend(false);
                 }
             }
         }
@@ -860,7 +850,7 @@ public void interrupt() {
         if (Thread.currentThread() != this) {
             checkAccess();
             try {
-                notifyJvmtiCriticalLock(true);
+                notifyJvmtiDisableSuspend(true);
                 synchronized (interruptLock) {
                     interrupted = true;
                     Interruptible b = nioBlocker;
@@ -873,7 +863,7 @@ public void interrupt() {
                     if (carrier != null) carrier.setInterrupt();
                 }
             } finally {
-                notifyJvmtiCriticalLock(false);
+                notifyJvmtiDisableSuspend(false);
             }
         } else {
             interrupted = true;
@@ -893,13 +883,13 @@ boolean getAndClearInterrupt() {
         boolean oldValue = interrupted;
         if (oldValue) {
             try {
-                notifyJvmtiCriticalLock(true);
+                notifyJvmtiDisableSuspend(true);
                 synchronized (interruptLock) {
                     interrupted = false;
                     carrierThread.clearInterrupt();
                 }
             } finally {
-                notifyJvmtiCriticalLock(false);
+                notifyJvmtiDisableSuspend(false);
             }
         }
         return oldValue;
@@ -924,7 +914,7 @@ Thread.State threadState() {
                 return Thread.State.RUNNABLE;
             case RUNNING:
                 try {
-                    notifyJvmtiCriticalLock(true);
+                    notifyJvmtiDisableSuspend(true);
                     // if mounted then return state of carrier thread
                     synchronized (carrierThreadAccessLock()) {
                         Thread carrierThread = this.carrierThread;
@@ -933,7 +923,7 @@ Thread.State threadState() {
                         }
                     }
                 } finally {
-                    notifyJvmtiCriticalLock(false);
+                    notifyJvmtiDisableSuspend(false);
                 }
                 // runnable, mounted
                 return Thread.State.RUNNABLE;
@@ -1049,7 +1039,7 @@ public String toString() {
         Thread carrier = carrierThread;
         if (carrier != null) {
             try {
-                notifyJvmtiCriticalLock(true);
+                notifyJvmtiDisableSuspend(true);
                 // include the carrier thread state and name when mounted
                 synchronized (carrierThreadAccessLock()) {
                     carrier = carrierThread;
@@ -1061,7 +1051,7 @@ public String toString() {
                     }
                 }
             } finally {
-                notifyJvmtiCriticalLock(false);
+                notifyJvmtiDisableSuspend(false);
             }
         }
         // include virtual thread state when not mounted
@@ -1161,7 +1151,7 @@ private void setCarrierThread(Thread carrier) {
     private native void notifyJvmtiHideFrames(boolean hide);
 
     @IntrinsicCandidate
-    private native void notifyJvmtiCriticalLock(boolean enter);
+    private native void notifyJvmtiDisableSuspend(boolean enter);
 
     private static native void registerNatives();
     static {

src/java.base/share/native/libjava/VirtualThread.c

@@ -32,12 +32,12 @@
 #define VIRTUAL_THREAD  "Ljava/lang/VirtualThread;"
 
 static JNINativeMethod methods[] = {
-    { "notifyJvmtiStart",        "()V",  (void *)&JVM_VirtualThreadStart },
-    { "notifyJvmtiEnd",          "()V",  (void *)&JVM_VirtualThreadEnd },
-    { "notifyJvmtiMount",        "(Z)V", (void *)&JVM_VirtualThreadMount },
-    { "notifyJvmtiUnmount",      "(Z)V", (void *)&JVM_VirtualThreadUnmount },
-    { "notifyJvmtiHideFrames",   "(Z)V", (void *)&JVM_VirtualThreadHideFrames },
-    { "notifyJvmtiCriticalLock", "(Z)V", (void *)&JVM_VirtualThreadCriticalLock },
+    { "notifyJvmtiStart",          "()V",  (void *)&JVM_VirtualThreadStart },
+    { "notifyJvmtiEnd",            "()V",  (void *)&JVM_VirtualThreadEnd },
+    { "notifyJvmtiMount",          "(Z)V", (void *)&JVM_VirtualThreadMount },
+    { "notifyJvmtiUnmount",        "(Z)V", (void *)&JVM_VirtualThreadUnmount },
+    { "notifyJvmtiHideFrames",     "(Z)V", (void *)&JVM_VirtualThreadHideFrames },
+    { "notifyJvmtiDisableSuspend", "(Z)V", (void *)&JVM_VirtualThreadDisableSuspend },
 };
 
 JNIEXPORT void JNICALL

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

@@ -22,7 +22,16 @@
  */
 
 /**
- * @test
+ * @test id=default
+ * @summary Do not suspend virtual threads in a critical section.
+ * @bug 8311218
+ * @requires vm.continuations
+ * @library /testlibrary
+ * @run main/othervm SuspendWithInterruptLock
+ */
+
+/**
+ * @test id=xint
  * @summary Do not suspend virtual threads in a critical section.
  * @bug 8311218
  * @requires vm.continuations
@@ -33,7 +42,7 @@
 import jvmti.JVMTIUtils;
 
 public class SuspendWithInterruptLock {
-    static boolean done;
+    static volatile boolean done;
 
     public static void main(String[] args) throws Exception {
         Thread yielder = Thread.ofVirtual().name("yielder").start(() -> yielder());