Reference handler and VM operation thread fixes.

Author: christianhaeubl

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/snippets/CEntryPointSnippets.java

@@ -678,7 +678,12 @@ private static int tearDownIsolate() {
             /* After threadExit(), only uninterruptible code may be executed. */
             RecurringCallbackSupport.suspendCallbackTimer("Execution of arbitrary code is prohibited during the last teardown steps.");
 
-            /* Shut down VM thread. */
+            /* Wait until the reference handler thread detaches (it was already stopped earlier). */
+            if (ReferenceHandler.useDedicatedThread()) {
+                ReferenceHandlerThread.waitUntilDetached();
+            }
+
+            /* Shut down VM operation thread. */
             if (VMOperationControl.useDedicatedVMOperationThread()) {
                 VMOperationControl.shutdownAndDetachVMOperationThread();
             }
@@ -724,7 +729,16 @@ private static boolean initiateTearDownIsolateInterruptibly() {
             return false;
         }
 
+        /*
+         * At this point, only the current thread, the VM operation thread, and the reference
+         * handler thread are still running.
+         */
+        if (ReferenceHandler.useDedicatedThread()) {
+            ReferenceHandlerThread.initiateShutdown();
+        }
+
         VMThreads.singleton().threadExit();
+        /* After threadExit(), only uninterruptible code may be executed. */
         return true;
     }
 
@@ -802,7 +816,7 @@ static boolean runtimeAssertionsEnabled() {
     @SubstrateForeignCallTarget(stubCallingConvention = false)
     private static int verifyIsolateThread(IsolateThread thread) {
         VMError.guarantee(CurrentIsolate.getCurrentThread() == thread, "Threads must match for the call below");
-        if (!VMThreads.singleton().verifyIsCurrentThread(thread) || !VMThreads.singleton().verifyThreadIsAttached(thread)) {
+        if (!VMThreads.singleton().verifyIsCurrentThread(thread) || !VMThreads.isAttached(thread)) {
             throw VMError.shouldNotReachHere("A call from native code to Java code provided the wrong JNI environment or the wrong IsolateThread. " +
                             "The JNI environment / IsolateThread is a thread-local data structure and must not be shared between threads.");
 

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/ReferenceHandler.java

@@ -24,18 +24,22 @@
  */
 package com.oracle.svm.core.heap;
 
+import static com.oracle.svm.core.Uninterruptible.CALLED_FROM_UNINTERRUPTIBLE_CODE;
+
 import java.lang.ref.Reference;
 
 import com.oracle.svm.core.IsolateArgumentParser;
 import com.oracle.svm.core.NeverInline;
 import com.oracle.svm.core.SubstrateOptions;
 import com.oracle.svm.core.SubstrateUtil;
+import com.oracle.svm.core.Uninterruptible;
 import com.oracle.svm.core.stack.StackOverflowCheck;
 import com.oracle.svm.core.util.VMError;
 
 import jdk.internal.ref.CleanerFactory;
 
 public final class ReferenceHandler {
+    @Uninterruptible(reason = CALLED_FROM_UNINTERRUPTIBLE_CODE, mayBeInlined = true)
     public static boolean useDedicatedThread() {
         int automaticReferenceHandling = IsolateArgumentParser.getOptionIndex(SubstrateOptions.ConcealedOptions.AutomaticReferenceHandling);
         return ReferenceHandlerThread.isSupported() && IsolateArgumentParser.singleton().getBooleanOptionValue(automaticReferenceHandling);

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/ReferenceHandlerThread.java

@@ -41,10 +41,12 @@
 import com.oracle.svm.core.util.VMError;
 
 import jdk.graal.compiler.api.replacements.Fold;
+import jdk.graal.compiler.word.Word;
 
 public final class ReferenceHandlerThread implements Runnable {
     private final Thread thread;
     private volatile IsolateThread isolateThread;
+    private volatile boolean stopped;
 
     @Platforms(Platform.HOSTED_ONLY.class)
     ReferenceHandlerThread() {
@@ -54,24 +56,48 @@ public final class ReferenceHandlerThread implements Runnable {
     }
 
     public static void start() {
-        if (isSupported()) {
-            singleton().thread.start();
+        if (!isSupported()) {
+            return;
         }
+
+        singleton().thread.start();
+        /* Wait until the isolateThread field is initialized. */
+        while (singleton().isolateThread.isNull()) {
+            Thread.yield();
+        }
+    }
+
+    public static void initiateShutdown() {
+        if (!isSupported()) {
+            return;
+        }
+
+        singleton().stopped = true;
+        Heap.getHeap().wakeUpReferencePendingListWaiters();
+    }
+
+    @Uninterruptible(reason = "Executed during teardown after VMThreads#threadExit")
+    public static void waitUntilDetached() {
+        if (!isSupported()) {
+            return;
+        }
+
+        VMThreads.waitInNativeUntilDetached(singleton().isolateThread);
+        singleton().isolateThread = Word.nullPointer();
     }
 
     @Uninterruptible(reason = CALLED_FROM_UNINTERRUPTIBLE_CODE, mayBeInlined = true)
     public static boolean isReferenceHandlerThread() {
-        if (isSupported()) {
-            return CurrentIsolate.getCurrentThread() == singleton().isolateThread;
-        }
-        return false;
+        return isReferenceHandlerThread(CurrentIsolate.getCurrentThread());
+    }
+
+    @Uninterruptible(reason = CALLED_FROM_UNINTERRUPTIBLE_CODE, mayBeInlined = true)
+    public static boolean isReferenceHandlerThread(IsolateThread other) {
+        return isSupported() && other == singleton().isolateThread;
     }
 
     public static boolean isReferenceHandlerThread(Thread other) {
-        if (isSupported()) {
-            return other == singleton().thread;
-        }
-        return false;
+        return isSupported() && other == singleton().thread;
     }
 
     @Override
@@ -80,19 +106,13 @@ public void run() {
 
         this.isolateThread = CurrentIsolate.getCurrentThread();
         try {
-            while (true) {
+            while (!stopped) {
                 ReferenceInternals.waitForPendingReferences();
                 ReferenceInternals.processPendingReferences();
                 ReferenceHandler.processCleaners();
             }
-        } catch (InterruptedException e) {
-            VMError.guarantee(VMThreads.isTearingDown(), "Reference Handler should only be interrupted during tear-down");
         } catch (Throwable t) {
-            if (t instanceof OutOfMemoryError && VMThreads.isTearingDown()) {
-                // Likely failed to allocate the InterruptedException, ignore either way.
-            } else {
-                throw VMError.shouldNotReachHere("Reference processing and cleaners must handle all potential exceptions", t);
-            }
+            throw VMError.shouldNotReachHere("Reference processing and cleaners must handle all potential exceptions", t);
         }
     }
 

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

@@ -47,7 +47,6 @@
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
 
-import jdk.graal.compiler.word.Word;
 import org.graalvm.nativeimage.CurrentIsolate;
 import org.graalvm.nativeimage.ImageInfo;
 import org.graalvm.nativeimage.ImageSingletons;
@@ -89,6 +88,7 @@
 import com.oracle.svm.core.heap.VMOperationInfos;
 import com.oracle.svm.core.jdk.StackTraceUtils;
 import com.oracle.svm.core.jdk.UninterruptibleUtils;
+import com.oracle.svm.core.locks.VMCondition;
 import com.oracle.svm.core.locks.VMMutex;
 import com.oracle.svm.core.log.Log;
 import com.oracle.svm.core.memory.NativeMemory;
@@ -109,6 +109,7 @@
 
 import jdk.graal.compiler.api.replacements.Fold;
 import jdk.graal.compiler.core.common.SuppressFBWarnings;
+import jdk.graal.compiler.word.Word;
 import jdk.internal.misc.Unsafe;
 
 /**
@@ -544,7 +545,11 @@ public void closeOSThreadHandle(OSThreadHandle threadHandle) {
         FORK_JOIN_POOL_TRY_TERMINATE_METHOD = ReflectionUtil.lookupMethod(ForkJoinPool.class, "tryTerminate", boolean.class, boolean.class);
     }
 
-    /** Have each thread, except this one, tear itself down. */
+    /**
+     * Interrupts all threads except for the current thread and any threads that require custom
+     * teardown logic (see {@link #isVMInternalThread(IsolateThread)}). Waits until the interrupted
+     * threads detach.
+     */
     public static boolean tearDownOtherThreads() {
         final Log trace = Log.noopLog().string("[PlatformThreads.tearDownPlatformThreads:").newline().flush();
 
@@ -557,9 +562,9 @@ public static boolean tearDownOtherThreads() {
          */
         VMThreads.setTearingDown();
 
-        /* Fetch all running application threads and interrupt them. */
+        /* Fetch threads and interrupt them. */
         ArrayList<Thread> threads = new ArrayList<>();
-        FetchApplicationThreadsOperation operation = new FetchApplicationThreadsOperation(threads);
+        FetchThreadsForTeardownOperation operation = new FetchThreadsForTeardownOperation(threads);
         operation.enqueue();
 
         Set<ExecutorService> pools = Collections.newSetFromMap(new IdentityHashMap<>());
@@ -626,9 +631,9 @@ public static boolean tearDownOtherThreads() {
         return result;
     }
 
-    /** Wait (im)patiently for the VMThreads list to drain. */
+    /** Wait (im)patiently for the thread list to drain. */
     private static boolean waitForTearDown() {
-        assert isApplicationThread(CurrentIsolate.getCurrentThread()) : "we count the application threads until only the current one remains";
+        assert !isVMInternalThread(CurrentIsolate.getCurrentThread()) : "we count the threads until only the current one remains";
 
         final Log trace = Log.noopLog().string("[PlatformThreads.waitForTearDown:").newline();
         final long warningNanos = SubstrateOptions.getTearDownWarningNanos();
@@ -666,8 +671,8 @@ private static boolean waitForTearDown() {
         }
     }
 
-    private static boolean isApplicationThread(IsolateThread isolateThread) {
-        return !VMOperationControl.isDedicatedVMOperationThread(isolateThread);
+    public static boolean isVMInternalThread(IsolateThread thread) {
+        return VMOperationControl.isDedicatedVMOperationThread(thread) || ReferenceHandlerThread.isReferenceHandlerThread(thread);
     }
 
     @SuppressFBWarnings(value = "NN", justification = "notifyAll is necessary for Java semantics, no shared state needs to be modified beforehand")
@@ -762,7 +767,10 @@ static void incrementNonDaemonThreads() {
         assert numThreads > 0;
     }
 
-    /** A caller must call THREAD_LIST_CONDITION.broadcast() manually. */
+    /**
+     * Callers must manually invoke {@link VMCondition#broadcast()} on
+     * {@link VMThreads#THREAD_LIST_CONDITION} to notify any threads waiting for changes.
+     */
     @Uninterruptible(reason = CALLED_FROM_UNINTERRUPTIBLE_CODE, mayBeInlined = true)
     private static void decrementNonDaemonThreads() {
         int numThreads = nonDaemonThreads.decrementAndGet();
@@ -1127,14 +1135,15 @@ protected void operate() {
     }
 
     /**
-     * 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}
+     * Builds a list of all threads that don't need any custom teardown logic. 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}.
      */
-    private static class FetchApplicationThreadsOperation extends JavaVMOperation {
+    private static class FetchThreadsForTeardownOperation extends JavaVMOperation {
         private final List<Thread> list;
 
-        FetchApplicationThreadsOperation(List<Thread> list) {
-            super(VMOperationInfos.get(FetchApplicationThreadsOperation.class, "Fetch application threads", SystemEffect.NONE));
+        FetchThreadsForTeardownOperation(List<Thread> list) {
+            super(VMOperationInfos.get(FetchThreadsForTeardownOperation.class, "Fetch threads for teardown", SystemEffect.NONE));
             this.list = list;
         }
 
@@ -1144,11 +1153,13 @@ public void operate() {
             VMMutex lock = VMThreads.THREAD_MUTEX.lock();
             try {
                 for (IsolateThread isolateThread = VMThreads.firstThread(); isolateThread.isNonNull(); isolateThread = VMThreads.nextThread(isolateThread)) {
-                    if (isApplicationThread(isolateThread)) {
-                        final Thread thread = PlatformThreads.fromVMThread(isolateThread);
-                        if (thread != null) {
-                            list.add(thread);
-                        }
+                    if (isVMInternalThread(isolateThread)) {
+                        continue;
+                    }
+
+                    Thread thread = PlatformThreads.fromVMThread(isolateThread);
+                    if (thread != null) {
+                        list.add(thread);
                     }
                 }
             } finally {
@@ -1185,24 +1196,26 @@ public void operate() {
             VMMutex lock = VMThreads.THREAD_MUTEX.lock();
             try {
                 for (IsolateThread isolateThread = VMThreads.firstThread(); isolateThread.isNonNull(); isolateThread = VMThreads.nextThread(isolateThread)) {
-                    if (isApplicationThread(isolateThread)) {
-                        attachedCount++;
-                        if (printLaggards.get() && trace.isEnabled() && isolateThread != queuingThread) {
-                            trace.string("  laggard isolateThread: ").hex(isolateThread);
-                            final Thread thread = PlatformThreads.fromVMThread(isolateThread);
-                            if (thread != null) {
-                                final String name = thread.getName();
-                                final Thread.State status = thread.getState();
-                                final boolean interruptedStatus = JavaThreads.isInterrupted(thread);
-                                trace.string("  thread.getName(): ").string(name)
-                                                .string("  interruptedStatus: ").bool(interruptedStatus)
-                                                .string("  getState(): ").string(status.name()).newline();
-                                for (StackTraceElement e : thread.getStackTrace()) {
-                                    trace.string(e.toString()).newline();
-                                }
+                    if (isVMInternalThread(isolateThread)) {
+                        continue;
+                    }
+
+                    attachedCount++;
+                    if (printLaggards.get() && trace.isEnabled() && isolateThread != queuingThread) {
+                        trace.string("  laggard isolateThread: ").hex(isolateThread);
+                        final Thread thread = PlatformThreads.fromVMThread(isolateThread);
+                        if (thread != null) {
+                            final String name = thread.getName();
+                            final Thread.State status = thread.getState();
+                            final boolean interruptedStatus = JavaThreads.isInterrupted(thread);
+                            trace.string("  thread.getName(): ").string(name)
+                                            .string("  interruptedStatus: ").bool(interruptedStatus)
+                                            .string("  getState(): ").string(status.name()).newline();
+                            for (StackTraceElement e : thread.getStackTrace()) {
+                                trace.string(e.toString()).newline();
                             }
-                            trace.newline().flush();
                         }
+                        trace.newline().flush();
                     }
                 }