Acquire the interrupt lock on the carrier thread to avoid stuck threads.

Author: peter-hofer

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/thread/JavaThreads.java

@@ -862,10 +862,9 @@ static void initializeNewThread(
     }
 
     /** Interruptibly park the current thread. */
-    static void platformPark() {
+    static void platformOrCarrierPark() {
         VMOperationControl.guaranteeOkayToBlock("[JavaThreads.park(): Should not park when it is not okay to block.]");
-        Thread thread = Thread.currentThread();
-        assert !isVirtual(thread);
+        Thread thread = platformThread.get();
         if (isInterrupted(thread)) { // avoid state changes and synchronization
             return;
         }
@@ -887,10 +886,9 @@ static void platformPark() {
     }
 
     /** Interruptibly park the current thread for the given number of nanoseconds. */
-    static void platformPark(long delayNanos) {
+    static void platformOrCarrierPark(long delayNanos) {
         VMOperationControl.guaranteeOkayToBlock("[JavaThreads.park(long): Should not park when it is not okay to block.]");
-        Thread thread = Thread.currentThread();
-        assert !isVirtual(thread);
+        Thread thread = platformThread.get();
         if (isInterrupted(thread)) { // avoid state changes and synchronization
             return;
         }
@@ -913,8 +911,8 @@ static void platformPark(long delayNanos) {
     /**
      * Unpark a Thread.
      *
-     * @see #platformPark()
-     * @see #platformPark(long)
+     * @see #platformOrCarrierPark()
+     * @see #platformOrCarrierPark(long)
      */
     static void platformUnpark(Thread thread) {
         assert !isVirtual(thread);

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/thread/SubstrateVirtualThread.java

@@ -42,6 +42,7 @@
 import com.oracle.svm.core.annotate.TargetClass;
 import com.oracle.svm.core.jdk.ContinuationsSupported;
 import com.oracle.svm.core.jdk.NotLoomJDK;
+import com.oracle.svm.core.monitor.MonitorSupport;
 import com.oracle.svm.core.stack.JavaFrameAnchor;
 import com.oracle.svm.core.stack.JavaFrameAnchors;
 import com.oracle.svm.core.util.VMError;
@@ -168,10 +169,13 @@ private void mount() {
         if (JavaThreads.isInterrupted(this)) {
             JavaThreads.platformSetInterrupt(carrier);
         } else if (JavaThreads.isInterrupted(carrier)) {
-            synchronized (interruptLock()) {
+            Object token = switchToCarrierAndAcquireInterruptLock();
+            try {
                 if (!JavaThreads.isInterrupted(this)) {
                     JavaThreads.platformGetAndClearInterrupt(carrier);
                 }
+            } finally {
+                releaseInterruptLockAndSwitchBack(token);
             }
         }
 
@@ -374,11 +378,14 @@ void unpark() {
                     submitRunContinuation();
                 }
             } else if (s == PINNED) {
-                synchronized (interruptLock()) {
+                Object token = switchToCarrierAndAcquireInterruptLock();
+                try {
                     Thread carrier = carrierThread;
                     if (carrier != null && state() == PINNED) {
                         U.unpark(carrier);
                     }
+                } finally {
+                    releaseInterruptLockAndSwitchBack(token);
                 }
             }
         }
@@ -428,10 +435,41 @@ private Object interruptLock() {
         return JavaThreads.toTarget(this).blockerLock;
     }
 
+    /** @see #releaseInterruptLockAndSwitchBack */
+    private Object switchToCarrierAndAcquireInterruptLock() {
+        Object token = null;
+        Thread current = Thread.currentThread();
+        if (current instanceof SubstrateVirtualThread) {
+            SubstrateVirtualThread vthread = (SubstrateVirtualThread) current;
+            /*
+             * If we block on the interrupt lock in the virtual thread, we yield, for which we first
+             * unmount. Unmounting also tries to acquire the interrupt lock and we might block
+             * again, this time on the carrier thread. Then, the virtual thread cannot continue to
+             * yield and execute on another thread later, and the carrier thread might not get
+             * unparked, and both threads are stuck.
+             */
+            Thread carrier = vthread.carrierThread;
+            JavaThreads.setCurrentThread(carrier, carrier);
+            token = vthread;
+        }
+        MonitorSupport.singleton().monitorEnter(interruptLock());
+        return token;
+    }
+
+    /** @see #switchToCarrierAndAcquireInterruptLock */
+    private void releaseInterruptLockAndSwitchBack(Object token) {
+        MonitorSupport.singleton().monitorExit(interruptLock());
+        if (token != null) {
+            SubstrateVirtualThread vthread = (SubstrateVirtualThread) token;
+            JavaThreads.setCurrentThread(vthread.carrierThread, vthread);
+        }
+    }
+
     @Override
     public void interrupt() {
         if (Thread.currentThread() != this) {
-            synchronized (interruptLock()) {
+            Object token = switchToCarrierAndAcquireInterruptLock();
+            try {
                 JavaThreads.getAndWriteInterruptedFlag(this, true);
                 Target_sun_nio_ch_Interruptible b = JavaThreads.toTarget(this).blocker;
                 if (b != null) {
@@ -441,6 +479,8 @@ public void interrupt() {
                 if (carrier != null) {
                     JavaThreads.platformSetInterrupt(carrier);
                 }
+            } finally {
+                releaseInterruptLockAndSwitchBack(token);
             }
         } else {
             JavaThreads.getAndWriteInterruptedFlag(this, true);
@@ -451,8 +491,11 @@ public void interrupt() {
 
     boolean getAndClearCarrierInterrupt() {
         assert Thread.currentThread() == this;
-        synchronized (interruptLock()) {
+        Object token = switchToCarrierAndAcquireInterruptLock();
+        try {
             return JavaThreads.platformGetAndClearInterrupt(carrierThread);
+        } finally {
+            releaseInterruptLockAndSwitchBack(token);
         }
     }
 
@@ -496,12 +539,15 @@ public Thread.State getState() {
                 return Thread.State.RUNNABLE;
             case RUNNING:
                 // if mounted then return state of carrier thread
-                synchronized (interruptLock()) {
+                Object token = switchToCarrierAndAcquireInterruptLock();
+                try {
                     @SuppressWarnings("hiding")
                     Thread carrierThread = this.carrierThread;
                     if (carrierThread != null) {
                         return JavaThreads.getThreadState(carrierThread);
                     }
+                } finally {
+                    releaseInterruptLockAndSwitchBack(token);
                 }
                 // runnable, mounted
                 return Thread.State.RUNNABLE;
@@ -533,14 +579,17 @@ public String toString() {
         Thread carrier = carrierThread;
         if (carrier != null) {
             // include the carrier thread state and name when mounted
-            synchronized (interruptLock()) {
+            Object token = switchToCarrierAndAcquireInterruptLock();
+            try {
                 carrier = carrierThread;
                 if (carrier != null) {
                     String stateAsString = JavaThreads.getThreadState(carrier).toString();
                     sb.append(stateAsString.toLowerCase(Locale.ROOT));
                     sb.append('@');
                     sb.append(carrier.getName());
                 }
+            } finally {
+                releaseInterruptLockAndSwitchBack(token);
             }
         }
         // include virtual thread state when not mounted

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/thread/Target_java_lang_Thread.java

@@ -382,7 +382,7 @@ public static boolean interrupted() {
     /**
      * Marks the thread as interrupted and wakes it up.
      *
-     * See {@link JavaThreads#platformPark()}, {@link JavaThreads#platformUnpark} and
+     * See {@link JavaThreads#platformOrCarrierPark()}, {@link JavaThreads#platformUnpark} and
      * {@link JavaThreads#sleep} for vital aspects of the underlying mechanisms.
      */
     @Substitute

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/thread/Target_jdk_internal_misc_Unsafe_JavaThreads.java

@@ -45,11 +45,11 @@ void park(boolean isAbsolute, long time) {
         /* Decide what kind of park I am doing. */
         if (!isAbsolute && time == 0L) {
             /* Park without deadline. */
-            JavaThreads.platformPark();
+            JavaThreads.platformOrCarrierPark();
         } else {
             /* Park with deadline. */
             final long delayNanos = TimeUtils.delayNanos(isAbsolute, time);
-            JavaThreads.platformPark(delayNanos);
+            JavaThreads.platformOrCarrierPark(delayNanos);
         }
         /*
          * Unsafe.park does not distinguish between timing out, being unparked, and being