[GR-35557] Fix transient issues with ParkEvents.

PullRequest: graal/10497

Author: christianhaeubl

substratevm/src/com.oracle.svm.core.posix/src/com/oracle/svm/core/posix/thread/PosixVMThreads.java

@@ -83,8 +83,9 @@ public void yield() {
         Sched.NoTransitions.sched_yield();
     }
 
+    @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
     @Override
-    public boolean supportsPatientSafepoints() {
+    public boolean supportsNativeYieldAndSleep() {
         return true;
     }
 

substratevm/src/com.oracle.svm.core.windows/src/com/oracle/svm/core/windows/WindowsVMThreads.java

@@ -86,8 +86,9 @@ public void yield() {
         Process.NoTransitions.SwitchToThread();
     }
 
+    @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
     @Override
-    public boolean supportsPatientSafepoints() {
+    public boolean supportsNativeYieldAndSleep() {
         return true;
     }
 

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/deopt/Deoptimizer.java

@@ -751,7 +751,7 @@ private static void logDeoptSourceFrameOperation(Pointer sp, DeoptimizedFrame de
     };
 
     public static void logRecentDeoptimizationEvents(Log log) {
-        log.string("Recent deoptimization events:").indent(true);
+        log.string("Recent deoptimization events (oldest first):").indent(true);
         recentDeoptimizationEvents.foreach(log, deoptEventsConsumer);
         log.indent(false);
     }

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

@@ -36,9 +36,9 @@
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicLong;
 
-import com.oracle.svm.core.SubstrateDiagnostics;
 import org.graalvm.compiler.api.replacements.Fold;
 import org.graalvm.compiler.core.common.SuppressFBWarnings;
+import org.graalvm.compiler.serviceprovider.GraalUnsafeAccess;
 import org.graalvm.compiler.serviceprovider.JavaVersionUtil;
 import org.graalvm.nativeimage.CurrentIsolate;
 import org.graalvm.nativeimage.ImageSingletons;
@@ -54,6 +54,7 @@
 import org.graalvm.word.PointerBase;
 import org.graalvm.word.WordFactory;
 
+import com.oracle.svm.core.SubstrateDiagnostics;
 import com.oracle.svm.core.SubstrateOptions;
 import com.oracle.svm.core.SubstrateUtil;
 import com.oracle.svm.core.annotate.NeverInline;
@@ -63,7 +64,6 @@
 import com.oracle.svm.core.heap.ReferenceHandlerThreadSupport;
 import com.oracle.svm.core.jdk.StackTraceUtils;
 import com.oracle.svm.core.jdk.UninterruptibleUtils;
-import com.oracle.svm.core.jdk.UninterruptibleUtils.AtomicReference;
 import com.oracle.svm.core.locks.VMMutex;
 import com.oracle.svm.core.log.Log;
 import com.oracle.svm.core.monitor.MonitorSupport;
@@ -78,13 +78,18 @@
 import com.oracle.svm.core.util.TimeUtils;
 import com.oracle.svm.core.util.VMError;
 
-public abstract class JavaThreads {
+// Checkstyle: stop
+import sun.misc.Unsafe;
+// Checkstyle: resume
 
+public abstract class JavaThreads {
     @Fold
     public static JavaThreads singleton() {
         return ImageSingletons.lookup(JavaThreads.class);
     }
 
+    private static final Unsafe UNSAFE = GraalUnsafeAccess.getUnsafe();
+
     /** The {@link java.lang.Thread} for the {@link IsolateThread}. */
     static final FastThreadLocalObject<Thread> currentThread = FastThreadLocalFactory.createObject(Thread.class, "JavaThreads.currentThread").setMaxOffset(FastThreadLocal.BYTE_OFFSET);
 
@@ -183,16 +188,6 @@ public static boolean isInterrupted(Thread thread) {
         return toTarget(thread).interruptedJDK11OrEarlier;
     }
 
-    @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
-    protected static AtomicReference<ParkEvent> getUnsafeParkEvent(Thread thread) {
-        return toTarget(thread).unsafeParkEvent;
-    }
-
-    @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
-    protected static AtomicReference<ParkEvent> getSleepParkEvent(Thread thread) {
-        return toTarget(thread).sleepParkEvent;
-    }
-
     @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
     protected static boolean wasStartedByCurrentIsolate(IsolateThread thread) {
         Thread javaThread = currentThread.get(thread);
@@ -430,8 +425,7 @@ public static void detachThread(IsolateThread vmThread) {
         VMThreads.THREAD_MUTEX.assertIsOwner("Must hold the THREAD_MUTEX.");
 
         Thread thread = currentThread.get(vmThread);
-        ParkEvent.detach(getUnsafeParkEvent(thread));
-        ParkEvent.detach(getSleepParkEvent(thread));
+        toTarget(thread).threadData.detach();
         toTarget(thread).isolateThread = WordFactory.nullPointer();
         if (!thread.isDaemon()) {
             nonDaemonThreads.decrementAndGet();
@@ -747,20 +741,21 @@ static void initializeNewThread(
     /** Interruptibly park the current thread. */
     static void park() {
         VMOperationControl.guaranteeOkayToBlock("[JavaThreads.park(): Should not park when it is not okay to block.]");
-        final Thread thread = Thread.currentThread();
+        Thread thread = Thread.currentThread();
         if (isInterrupted(thread)) { // avoid state changes and synchronization
             return;
         }
-        /*
-         * We can defer assigning a ParkEvent to here because Thread.interrupt() is guaranteed to
-         * assign and unpark one if it doesn't yet exist, otherwise we could lose a wakeup.
-         */
-        final ParkEvent parkEvent = ensureUnsafeParkEvent(thread);
+
+        ParkEvent parkEvent = getCurrentThreadData().ensureUnsafeParkEvent();
         // Change the Java thread state while parking.
-        final int oldStatus = JavaThreads.getThreadStatus(thread);
+        int oldStatus = JavaThreads.getThreadStatus(thread);
         int newStatus = MonitorSupport.singleton().maybeAdjustNewParkStatus(ThreadStatus.PARKED);
         JavaThreads.setThreadStatus(thread, newStatus);
         try {
+            /*
+             * If another thread interrupted this thread in the meanwhile, then the call below won't
+             * block because Thread.interrupt() modifies the ParkEvent.
+             */
             parkEvent.condWait();
         } finally {
             JavaThreads.setThreadStatus(thread, oldStatus);
@@ -770,19 +765,20 @@ static void park() {
     /** Interruptibly park the current thread for the given number of nanoseconds. */
     static void park(long delayNanos) {
         VMOperationControl.guaranteeOkayToBlock("[JavaThreads.park(long): Should not park when it is not okay to block.]");
-        final Thread thread = Thread.currentThread();
+        Thread thread = Thread.currentThread();
         if (isInterrupted(thread)) { // avoid state changes and synchronization
             return;
         }
-        /*
-         * We can defer assigning a ParkEvent to here because Thread.interrupt() is guaranteed to
-         * assign and unpark one if it doesn't yet exist, otherwise we could lose a wakeup.
-         */
-        final ParkEvent parkEvent = ensureUnsafeParkEvent(thread);
-        final int oldStatus = JavaThreads.getThreadStatus(thread);
+
+        ParkEvent parkEvent = getCurrentThreadData().ensureUnsafeParkEvent();
+        int oldStatus = JavaThreads.getThreadStatus(thread);
         int newStatus = MonitorSupport.singleton().maybeAdjustNewParkStatus(ThreadStatus.PARKED_TIMED);
         JavaThreads.setThreadStatus(thread, newStatus);
         try {
+            /*
+             * If another thread interrupted this thread in the meanwhile, then the call below won't
+             * block because Thread.interrupt() modifies the ParkEvent.
+             */
             parkEvent.condTimedWait(delayNanos);
         } finally {
             JavaThreads.setThreadStatus(thread, oldStatus);
@@ -796,12 +792,14 @@ static void park(long delayNanos) {
      * @see #park(long)
      */
     static void unpark(Thread thread) {
-        ensureUnsafeParkEvent(thread).unpark();
-    }
-
-    /** Get the Park event for a thread, initializing it if necessary. */
-    private static ParkEvent ensureUnsafeParkEvent(Thread thread) {
-        return ParkEvent.initializeOnce(JavaThreads.getUnsafeParkEvent(thread), false);
+        ThreadData threadData = acquireThreadData(thread);
+        if (threadData != null) {
+            try {
+                threadData.ensureUnsafeParkEvent().unpark();
+            } finally {
+                threadData.release();
+            }
+        }
     }
 
     /** Sleep for the given number of nanoseconds, dealing with early wakeups and interruptions. */
@@ -817,23 +815,31 @@ static void sleep(long millis) throws InterruptedException {
 
     static void sleep0(long delayNanos) {
         VMOperationControl.guaranteeOkayToBlock("[JavaThreads.sleep(long): Should not sleep when it is not okay to block.]");
-        final Thread thread = Thread.currentThread();
-        final ParkEvent sleepEvent = ParkEvent.initializeOnce(JavaThreads.getSleepParkEvent(thread), true);
+        Thread thread = Thread.currentThread();
+        ParkEvent sleepEvent = getCurrentThreadData().ensureSleepParkEvent();
         sleepEvent.reset();
+
         /*
          * It is critical to reset the event *before* checking for an interrupt to avoid losing a
          * wakeup in the race. This requires that updates to the event's unparked status and updates
-         * to the thread's interrupt status cannot be reordered with regard to each other. Another
-         * important aspect is that the thread must have a sleepParkEvent assigned to it *before*
-         * the interrupted check because if not, the interrupt code will not assign one and the
-         * wakeup will be lost, too.
+         * to the thread's interrupt status cannot be reordered with regard to each other.
+         *
+         * Another important aspect is that the thread must have a sleepParkEvent assigned to it
+         * *before* the interrupted check because if not, the interrupt code will not assign one and
+         * the wakeup will be lost.
          */
+        UNSAFE.fullFence();
+
         if (isInterrupted(thread)) {
             return; // likely leaves a stale unpark which will be reset before the next sleep()
         }
         final int oldStatus = JavaThreads.getThreadStatus(thread);
         JavaThreads.setThreadStatus(thread, ThreadStatus.SLEEPING);
         try {
+            /*
+             * If another thread interrupted this thread in the meanwhile, then the call below won't
+             * block because Thread.interrupt() modifies the ParkEvent.
+             */
             sleepEvent.condTimedWait(delayNanos);
         } finally {
             JavaThreads.setThreadStatus(thread, oldStatus);
@@ -846,16 +852,32 @@ static void sleep0(long delayNanos) {
      * @see #sleep(long)
      */
     static void interrupt(Thread thread) {
-        final ParkEvent sleepEvent = JavaThreads.getSleepParkEvent(thread).get();
-        if (sleepEvent != null) {
-            sleepEvent.unpark();
+        ThreadData threadData = acquireThreadData(thread);
+        if (threadData != null) {
+            try {
+                ParkEvent sleepEvent = threadData.getSleepParkEvent();
+                if (sleepEvent != null) {
+                    sleepEvent.unpark();
+                }
+            } finally {
+                threadData.release();
+            }
         }
     }
 
     static boolean isAlive(int threadStatus) {
         return !(threadStatus == ThreadStatus.NEW || threadStatus == ThreadStatus.TERMINATED);
     }
 
+    private static ThreadData acquireThreadData(Thread thread) {
+        return toTarget(thread).threadData.acquire();
+    }
+
+    @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
+    static ThreadData getCurrentThreadData() {
+        return (ThreadData) toTarget(Thread.currentThread()).threadData;
+    }
+
     /**
      * Builds a list of all application threads. This must be done in a VM operation because only
      * there we are allowed to allocate Java memory while holding the {@link VMThreads#THREAD_MUTEX}