[GR-35763] Fix a deadlock when Thread.join is used on the main thread.

PullRequest: graal/10670

Author: christianhaeubl

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/JavaMainWrapper.java

@@ -33,7 +33,9 @@
 import java.util.List;
 
 import org.graalvm.compiler.word.Word;
+import org.graalvm.nativeimage.CurrentIsolate;
 import org.graalvm.nativeimage.ImageSingletons;
+import org.graalvm.nativeimage.Isolate;
 import org.graalvm.nativeimage.Platform;
 import org.graalvm.nativeimage.Platforms;
 import org.graalvm.nativeimage.VMRuntime;
@@ -47,20 +49,24 @@
 import org.graalvm.word.UnsignedWord;
 import org.graalvm.word.WordFactory;
 
-import com.oracle.svm.core.annotate.AlwaysInline;
 import com.oracle.svm.core.annotate.Uninterruptible;
 import com.oracle.svm.core.c.CGlobalData;
 import com.oracle.svm.core.c.CGlobalDataFactory;
 import com.oracle.svm.core.c.function.CEntryPointActions;
 import com.oracle.svm.core.c.function.CEntryPointCreateIsolateParameters;
 import com.oracle.svm.core.c.function.CEntryPointErrors;
 import com.oracle.svm.core.c.function.CEntryPointOptions;
+import com.oracle.svm.core.c.function.CEntryPointOptions.NoEpilogue;
+import com.oracle.svm.core.c.function.CEntryPointOptions.NoPrologue;
+import com.oracle.svm.core.c.function.CEntryPointSetup;
 import com.oracle.svm.core.jdk.InternalVMMethod;
 import com.oracle.svm.core.jdk.RuntimeSupport;
 import com.oracle.svm.core.log.Log;
 import com.oracle.svm.core.thread.JavaThreads;
 import com.oracle.svm.core.thread.PlatformThreads;
+import com.oracle.svm.core.thread.VMThreads;
 import com.oracle.svm.core.util.Counter;
+import com.oracle.svm.core.util.VMError;
 
 import jdk.vm.ci.code.Architecture;
 
@@ -119,17 +125,20 @@ public List<String> getInputArguments() {
         }
     }
 
+    @Uninterruptible(reason = "The caller initialized the thread state, so the callees do not need to be uninterruptible.", calleeMustBe = false)
+    private static int runCore() {
+        return runCore0();
+    }
+
     /**
      * Used by JavaMainWrapper and any user supplied main entry point (from
      * {@link org.graalvm.nativeimage.hosted.Feature.AfterRegistrationAccess}).
      */
-    @AlwaysInline(value = "Single callee from the main entry point.")
-    public static int runCore() {
+    private static int runCore0() {
         Architecture imageArchitecture = ImageSingletons.lookup(SubstrateTargetDescription.class).arch;
         CPUFeatureAccess cpuFeatureAccess = ImageSingletons.lookup(CPUFeatureAccess.class);
         cpuFeatureAccess.verifyHostSupportsArchitecture(imageArchitecture);
 
-        int exitCode;
         try {
             if (SubstrateOptions.ParseRuntimeOptions.getValue()) {
                 /*
@@ -147,10 +156,7 @@ public static int runCore() {
              */
             JavaMainSupport mainSupport = ImageSingletons.lookup(JavaMainSupport.class);
             mainSupport.javaMainHandle.invokeExact(mainSupport.mainArgs);
-
-            /* The application terminated normally. */
-            exitCode = 0;
-
+            return 0;
         } catch (Throwable ex) {
             JavaThreads.dispatchUncaughtException(Thread.currentThread(), ex);
 
@@ -159,30 +165,54 @@ public static int runCore() {
              * if an uncaught exception handler is registered. This behavior is the same on the Java
              * HotSpot VM.
              */
-            exitCode = 1;
-
+            return 1;
         } finally {
-            /*
-             * Shutdown sequence: First wait for all non-daemon threads to exit.
-             */
-            PlatformThreads.singleton().joinAllNonDaemons();
-            /*
-             * Run shutdown hooks (both our own hooks and application-registered hooks. Note that
-             * this can start new non-daemon threads. We are not responsible to wait until they have
-             * exited.
-             */
-            RuntimeSupport.getRuntimeSupport().shutdown();
-
-            Counter.logValues(Log.log());
+            PlatformThreads.exit(Thread.currentThread());
         }
-        return exitCode;
     }
 
+    @Uninterruptible(reason = "The caller initialized the thread state, so the callees do not need to be uninterruptible.", calleeMustBe = false)
+    private static void runShutdown() {
+        runShutdown0();
+    }
+
+    private static void runShutdown0() {
+        PlatformThreads.ensureCurrentAssigned("DestroyJavaVM", null, false);
+
+        // Shutdown sequence: First wait for all non-daemon threads to exit.
+        PlatformThreads.singleton().joinAllNonDaemons();
+
+        /*
+         * Run shutdown hooks (both our own hooks and application-registered hooks. Note that this
+         * can start new non-daemon threads. We are not responsible to wait until they have exited.
+         */
+        RuntimeSupport.getRuntimeSupport().shutdown();
+
+        Counter.logValues(Log.log());
+    }
+
+    @Uninterruptible(reason = "Thread state not set up yet.")
     @CEntryPoint(include = CEntryPoint.NotIncludedAutomatically.class)
-    @CEntryPointOptions(prologue = EnterCreateIsolateWithCArgumentsPrologue.class)
-    @SuppressWarnings("unused")
+    @CEntryPointOptions(prologue = NoPrologue.class, epilogue = NoEpilogue.class)
     public static int run(int argc, CCharPointerPointer argv) {
-        return runCore();
+        try {
+            // Create the isolate and attach the current C thread as the main Java thread.
+            EnterCreateIsolateWithCArgumentsPrologue.enter(argc, argv);
+            assert !VMThreads.wasStartedByCurrentIsolate(CurrentIsolate.getCurrentThread()) : "re-attach would cause issues otherwise";
+
+            Isolate isolate = CurrentIsolate.getIsolate();
+            int exitCode = runCore();
+            CEntryPointSetup.LeaveDetachThreadEpilogue.leave();
+
+            // Re-attach the same C thread as another Java thread.
+            EnterAttachThreadForShutdown.enter(isolate);
+            runShutdown();
+            CEntryPointSetup.LeaveDetachThreadEpilogue.leave();
+
+            return exitCode;
+        } catch (Throwable e) {
+            throw VMError.shouldNotReachHere(e);
+        }
     }
 
     private static boolean isArgumentBlockSupported() {
@@ -283,4 +313,17 @@ public static void enter(int paramArgc, CCharPointerPointer paramArgv) {
             }
         }
     }
+
+    public static class EnterAttachThreadForShutdown implements CEntryPointOptions.Prologue {
+        private static final CGlobalData<CCharPointer> errorMessage = CGlobalDataFactory.createCString(
+                        "Failed to re-attach the main thread for shutting down the main isolate.");
+
+        @Uninterruptible(reason = "prologue")
+        static void enter(Isolate isolate) {
+            int code = CEntryPointActions.enterAttachThread(isolate, false);
+            if (code != CEntryPointErrors.NO_ERROR) {
+                CEntryPointActions.failFatally(code, errorMessage.get());
+            }
+        }
+    }
 }

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/SubstrateSegfaultHandler.java

@@ -51,7 +51,7 @@
 import com.oracle.svm.core.c.function.CEntryPointActions;
 import com.oracle.svm.core.c.function.CEntryPointErrors;
 import com.oracle.svm.core.graal.nodes.WriteCurrentVMThreadNode;
-import com.oracle.svm.core.graal.nodes.WriteHeapBaseNode;
+import com.oracle.svm.core.graal.snippets.CEntryPointSnippets;
 import com.oracle.svm.core.jdk.RuntimeSupport;
 import com.oracle.svm.core.log.Log;
 import com.oracle.svm.core.option.RuntimeOptionKey;
@@ -132,7 +132,7 @@ protected static boolean tryEnterIsolate(RegisterDumper.Context context) {
             // the crash happened in native code that was linked into Native Image.
             if (SubstrateOptions.SpawnIsolates.getValue()) {
                 PointerBase heapBase = RegisterDumper.singleton().getHeapBase(context);
-                WriteHeapBaseNode.writeCurrentVMHeapBase(heapBase);
+                CEntryPointSnippets.setHeapBase(heapBase);
             }
             if (SubstrateOptions.MultiThreaded.getValue()) {
                 PointerBase threadPointer = RegisterDumper.singleton().getThreadPointer(context);

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/c/function/CEntryPointSetup.java

@@ -108,7 +108,7 @@ public static final class LeaveDetachThreadEpilogue implements CEntryPointOption
                         "Failed to leave the current IsolateThread context and to detach the current thread.");
 
         @Uninterruptible(reason = "epilogue")
-        static void leave() {
+        public static void leave() {
             int code = CEntryPointActions.leaveDetachThread();
             if (code != 0) {
                 CEntryPointActions.failFatally(code, errorMessage.get());

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

@@ -599,7 +599,7 @@ private static boolean isApplicationThread(IsolateThread isolateThread) {
     }
 
     @SuppressFBWarnings(value = "NN", justification = "notifyAll is necessary for Java semantics, no shared state needs to be modified beforehand")
-    protected static void exit(Thread thread) {
+    public static void exit(Thread thread) {
         /*
          * First call Thread.exit(). This allows waiters on the thread object to observe that a
          * daemon ThreadGroup is destroyed as well if this thread happens to be the last thread of a

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

@@ -369,7 +369,7 @@ public void detachThread(IsolateThread thread) {
     }
 
     @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
-    private static boolean wasStartedByCurrentIsolate(IsolateThread thread) {
+    public static boolean wasStartedByCurrentIsolate(IsolateThread thread) {
         return StartedByCurrentIsolate.getAddress(thread).readByte(0) != 0;
     }