[GR-7405] We no longer need to enter the TruffleContext to interrupt other threads, except for rb_thread_call_without_gvl()

* All other UnblockingAction are fine to execute without being inside the context.

Author: eregon

CHANGELOG.md

@@ -4,6 +4,7 @@ New features:
 
 * Access to local variables of the interactive Binding via language bindings is now supported: `context.getBindings("ruby").putMember("my_var", 42);` (#2030).
 * `VALUE`s in C extensions now expose the Ruby object when viewed in the debugger, as long as they have not been converted to native values.
+* Signal handlers can now be run without triggering multi-threading.
 
 Bug fixes:
 

src/main/java/org/truffleruby/core/VMPrimitiveNodes.java

@@ -80,14 +80,12 @@
 
 import com.oracle.truffle.api.CompilerDirectives;
 import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
-import com.oracle.truffle.api.TruffleContext;
 import com.oracle.truffle.api.dsl.Cached;
 import com.oracle.truffle.api.dsl.Specialization;
 import com.oracle.truffle.api.frame.VirtualFrame;
 import com.oracle.truffle.api.profiles.BranchProfile;
 import com.oracle.truffle.api.profiles.ConditionProfile;
 
-import sun.misc.Signal;
 import sun.misc.SignalHandler;
 
 @CoreModule(value = "VMPrimitives", isClass = true)
@@ -260,48 +258,16 @@ protected boolean watchSignalProc(Object signalString, RubyProc action,
             }
 
             final String signalName = libSignalString.getJavaString(signalString);
-            return registerHandler(signalName, signal -> {
-                if (context.getOptions().SINGLE_THREADED) {
-                    warnRestoreAndRaise(context, signalName, signal, "create a thread");
-                    return;
-                }
 
+            return registerHandler(signalName, signal -> {
                 final RubyThread rootThread = context.getThreadManager().getRootThread();
-
-                // Workaround: we need to register with Truffle (which means going multithreaded),
-                // so that NFI can get its context to call pthread_kill() (GR-7405).
-                final TruffleContext truffleContext = context.getEnv().getContext();
-                final Object prev;
-                try {
-                    prev = truffleContext.enter(this);
-                } catch (IllegalStateException e) { // Multi threaded access denied from Truffle
-                    warnRestoreAndRaise(context, signalName, signal, "attach a thread");
-                    return;
-                }
-                try {
-                    context.getSafepointManager().pauseAllThreadsAndExecuteFromNonRubyThread(
-                            "Handling of signal " + signal,
-                            SafepointPredicate.currentFiberOfThread(context, rootThread),
-                            (rubyThread, currentNode) -> ProcOperations.rootCall(action));
-                } finally {
-                    truffleContext.leave(this, prev);
-                }
+                context.getSafepointManager().pauseAllThreadsAndExecuteFromNonRubyThread(
+                        "Handling of signal " + signal,
+                        SafepointPredicate.currentFiberOfThread(context, rootThread),
+                        (rubyThread, currentNode) -> ProcOperations.rootCall(action));
             });
         }
 
-        private static void warnRestoreAndRaise(RubyContext context, String signalName, Signal signal, String failure) {
-            // Not in a context, so we cannot use TruffleLogger
-            context.getEnvErrStream().println(
-                    "[ruby] SEVERE: signal " + signal + " caught but can't " + failure +
-                            " to handle it so restoring the default handler and re-raising the signal");
-            Signals.restoreDefaultHandler(signalName);
-            try {
-                Signal.raise(signal);
-            } catch (IllegalArgumentException illegalArgumentException) {
-                illegalArgumentException.printStackTrace(context.getEnvErrStream());
-            }
-        }
-
         @TruffleBoundary
         private boolean restoreDefaultHandler(String signalName) {
             if (getContext().getOptions().EMBEDDED) {

src/main/java/org/truffleruby/core/thread/ThreadManager.java

@@ -536,6 +536,11 @@ public RubyThread getCurrentThread() {
         return rubyThread;
     }
 
+    @TruffleBoundary
+    public RubyThread getCurrentThreadOrNull() {
+        return currentThread.get();
+    }
+
     @TruffleBoundary
     public RubyFiber getRubyFiberFromCurrentJavaThread() {
         return rubyFiber.get();

src/main/java/org/truffleruby/core/thread/ThreadNodes.java

@@ -43,6 +43,7 @@
 import java.util.Queue;
 import java.util.concurrent.TimeUnit;
 
+import com.oracle.truffle.api.TruffleContext;
 import com.oracle.truffle.api.profiles.ConditionProfile;
 import org.jcodings.specific.USASCIIEncoding;
 import org.jcodings.specific.UTF8Encoding;
@@ -558,7 +559,7 @@ protected Object call(RubyThread thread, Object function, Object arg, Object unb
             if (unblocker == nil) {
                 unblockingAction = threadManager.getNativeCallUnblockingAction();
             } else {
-                unblockingAction = makeUnblockingAction(unblocker, unblockerArg);
+                unblockingAction = makeUnblockingAction(getContext(), unblocker, unblockerArg);
             }
 
             final UnblockingActionHolder actionHolder = threadManager.getActionHolder(Thread.currentThread());
@@ -574,13 +575,37 @@ protected Object call(RubyThread thread, Object function, Object arg, Object unb
         }
 
         @TruffleBoundary
-        private UnblockingAction makeUnblockingAction(Object function, Object argument) {
+        private static UnblockingAction makeUnblockingAction(RubyContext context, Object function, Object argument) {
             assert InteropLibrary.getUncached().isExecutable(function);
+
             return () -> {
+                final TruffleContext truffleContext = context.getEnv().getContext();
+                final boolean alreadyEntered = truffleContext.isEntered();
+                Object prev = null;
+                if (!alreadyEntered) {
+                    // We need to enter the context to execute this unblocking action, as it runs on Sulong.
+                    try {
+                        if (context.getOptions().SINGLE_THREADED) {
+                            throw new IllegalStateException("--single-threaded was passed");
+                        }
+                        prev = truffleContext.enter(null);
+                    } catch (IllegalStateException e) { // Multi threaded access denied from Truffle
+                        // Not in a context, so we cannot use TruffleLogger
+                        context.getEnvErrStream().println(
+                                "[ruby] SEVERE: could not unblock thread inside blocking call in C extension because " +
+                                        "the context does not allow multithreading (" + e.getMessage() + ")");
+                        return;
+                    }
+                }
+
                 try {
                     InteropLibrary.getUncached().execute(function, argument);
                 } catch (InteropException e) {
                     throw CompilerDirectives.shouldNotReachHere(e);
+                } finally {
+                    if (!alreadyEntered) {
+                        truffleContext.leave(null, prev);
+                    }
                 }
             };
         }

src/main/java/org/truffleruby/language/SafepointManager.java

@@ -144,8 +144,6 @@ private void assumptionInvalidated(Node currentNode, boolean fromBlockingCall) {
     private SafepointAction step(Node currentNode, boolean isDrivingThread, String reason) {
         assert isDrivingThread == lock.isHeldByCurrentThread();
 
-        final RubyThread thread = context.getThreadManager().getCurrentThread();
-
         // Wait for other threads to reach their safepoint
         if (isDrivingThread) {
             driveArrivalAtPhaser(reason);
@@ -162,7 +160,9 @@ private SafepointAction step(Node currentNode, boolean isDrivingThread, String r
         SafepointAction deferredAction = null;
 
         try {
-            if (filter.test(thread)) {
+            final RubyThread thread = context.getThreadManager().getCurrentThreadOrNull();
+
+            if (thread != null && filter.test(thread)) {
                 if (SafepointAction.isDeferred(action)) {
                     deferredAction = action;
                 } else {
@@ -196,8 +196,9 @@ private void driveArrivalAtPhaser(String reason) {
                 // retry
             } catch (TimeoutException e) {
                 if (System.nanoTime() >= max) {
-                    RubyLanguage.LOGGER.severe(String.format(
-                            "waited %d seconds in the SafepointManager but %d of %d threads did not arrive - a thread is likely making a blocking call - reason for the safepoint: %s",
+                    // Possibly not in a context, so we cannot use TruffleLogger
+                    context.getEnvErrStream().println(String.format(
+                            "[ruby] SEVERE: waited %d seconds in the SafepointManager but %d of %d threads did not arrive - a thread is likely making a blocking call - reason for the safepoint: %s",
                             waits * WAIT_TIME_IN_SECONDS,
                             phaser.getUnarrivedParties(),
                             phaser.getRegisteredParties(),
@@ -207,8 +208,8 @@ private void driveArrivalAtPhaser(String reason) {
                         restoreDefaultInterruptHandler();
                     }
                     if (max >= exitTime) {
-                        RubyLanguage.LOGGER.severe(
-                                "waited " + MAX_WAIT_TIME_IN_SECONDS +
+                        context.getEnvErrStream().println(
+                                "[ruby] SEVERE: waited " + MAX_WAIT_TIME_IN_SECONDS +
                                         " seconds in the SafepointManager, terminating the process as it is unlikely to get unstuck");
                         System.exit(1);
                     }
@@ -253,12 +254,12 @@ private void printStacktracesOfBlockedThreads() {
     }
 
     private void restoreDefaultInterruptHandler() {
-        RubyLanguage.LOGGER.warning("restoring default interrupt handler");
-
+        // Possibly not in a context, so we cannot use TruffleLogger
+        context.getEnvErrStream().println("[ruby] WARNING: restoring default interrupt handler");
         try {
             Signals.restoreDefaultHandler("INT");
         } catch (Throwable t) {
-            RubyLanguage.LOGGER.warning("failed to restore default interrupt handler\n" + t);
+            context.getEnvErrStream().println("[ruby] WARNING: failed to restore default interrupt handler\n" + t);
         }
     }