From e972d4cb4c56dd797ff11ed9ac70627d67df370f Mon Sep 17 00:00:00 2001
From: Peter Hofer <peter.hofer@oracle.com>
Date: Mon, 26 Mar 2018 18:39:59 +0200
Subject: [PATCH] Fix race between safepoints and newly attached threads.

---
 .../graal/posix/PosixCEntryPointSnippets.java |  7 ++--
 .../com/oracle/svm/core/thread/VMThreads.java | 34 ++++++-------------
 2 files changed, 16 insertions(+), 25 deletions(-)

diff --git a/substratevm/src/com.oracle.svm.core.graal/src/com/oracle/svm/core/graal/posix/PosixCEntryPointSnippets.java b/substratevm/src/com.oracle.svm.core.graal/src/com/oracle/svm/core/graal/posix/PosixCEntryPointSnippets.java
index 915c1860855c..ef420038c373 100644
--- a/substratevm/src/com.oracle.svm.core.graal/src/com/oracle/svm/core/graal/posix/PosixCEntryPointSnippets.java
+++ b/substratevm/src/com.oracle.svm.core.graal/src/com/oracle/svm/core/graal/posix/PosixCEntryPointSnippets.java
@@ -193,7 +193,11 @@ public static int attachThreadSnippet(Isolate isolate, @ConstantParameter int vm
         if (MultiThreaded.getValue()) {
             writeCurrentVMThread(VMThreads.nullThread());
         }
-        return runtimeCall(ATTACH_THREAD, isolate, vmThreadSize);
+        int result = runtimeCall(ATTACH_THREAD, isolate, vmThreadSize);
+        if (MultiThreaded.getValue() && result == Errors.NO_ERROR) {
+            Safepoint.transitionNativeToJava();
+        }
+        return result;
     }
 
     @Uninterruptible(reason = "Thread state not yet set up.")
@@ -219,7 +223,6 @@ private static int attachThread(Isolate isolate, int vmThreadSize) {
                 PosixVMThreads.IsolateTL.set(thread, isolate);
             }
             writeCurrentVMThread(thread);
-            VMThreads.StatusSupport.setStatusJavaUnguarded(thread);
         }
         return Errors.NO_ERROR;
     }
diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/thread/VMThreads.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/thread/VMThreads.java
index 4513da650569..da6458d0b159 100644
--- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/thread/VMThreads.java
+++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/thread/VMThreads.java
@@ -111,33 +111,20 @@ public static IsolateThread nextThreadFromList(IsolateThread cur) {
      * must be the first method called in every thread.
      */
     @Uninterruptible(reason = "Reason: Thread register not yet set up.")
-    public static void attachThread(IsolateThread vmThread) {
-        assert StatusSupport.isStatusCreated(vmThread) : "Status should be initialized on creation.";
-        // Manipulating the VMThread list requires the lock for
-        // changing the status and for notification.
-        // But the VMThread for the current thread is not set up yet,
-        // so the locking must be without transitions.
-        attachThreadUnderLock(vmThread);
-    }
-
-    /**
-     * The body of {@link #attachThread} that is executed under the VMThreads mutex. With that mutex
-     * held, callees do not need to be annotated with {@link Uninterruptible}.
-     * <p>
-     * Using try-finally rather than a try-with-resources to avoid implicitly calling
-     * {@link Throwable#addSuppressed(Throwable)}, which I can not annotate as uninterruptible.
-     */
-    @Uninterruptible(reason = "Reason: Thread register not yet set up.")
-    public static void attachThreadUnderLock(IsolateThread vmThread) {
-        VMMutex lock = VMThreads.THREAD_MUTEX;
+    public static void attachThread(IsolateThread thread) {
+        assert StatusSupport.isStatusCreated(thread) : "Status should be initialized on creation.";
+        // Manipulating the VMThread list requires the lock, but the IsolateThread is not set up
+        // yet, so the locking must be without transitions. Not using try-with-resources to avoid
+        // implicitly calling addSuppressed(), which is not uninterruptible.
+        VMThreads.THREAD_MUTEX.lockNoTransition();
         try {
-            lock.lockNoTransition();
             VMThreads.THREAD_MUTEX.guaranteeIsLocked("Must hold the VMThreads lock.");
-            nextTL.set(vmThread, head);
-            head = vmThread;
+            nextTL.set(thread, head);
+            head = thread;
+            StatusSupport.setStatusNative(thread);
             VMThreads.THREAD_LIST_CONDITION.broadcast();
         } finally {
-            lock.unlock();
+            VMThreads.THREAD_MUTEX.unlock();
         }
     }
 
@@ -240,6 +227,7 @@ public static void setStatusNative() {
             statusTL.set(STATUS_IN_NATIVE);
         }
 
+        @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
         public static void setStatusNative(IsolateThread vmThread) {
             statusTL.setVolatile(vmThread, STATUS_IN_NATIVE);
         }