[GR-18163] Fix handling of signals with --single-threaded (#2265)

PullRequest: truffleruby/2433

Author: eregon

CHANGELOG.md

@@ -13,6 +13,7 @@ Bug fixes:
 * Fix `Thread.handle_interrupt` to defer non-pure interrupts until the end of the `handle_interrupt` block (#2219).
 * Clear and restore errinfo on entry and normal return from methods in C extensions (#2227).
 * Fix extra whitespace in squiggly heredoc with escaped newline (#2238, @wildmaples and @norswap).
+* Fix handling of signals with `--single-threaded` (#2265).
 
 Compatibility:
 

src/main/java/org/truffleruby/RubyContext.java

@@ -10,6 +10,9 @@
 package org.truffleruby;
 
 import java.io.File;
+import java.io.OutputStream;
+import java.io.PrintStream;
+import java.io.UnsupportedEncodingException;
 import java.nio.ByteBuffer;
 import java.security.NoSuchAlgorithmException;
 import java.security.SecureRandom;
@@ -96,6 +99,8 @@ public class RubyContext {
 
     private final RubyLanguage language;
     @CompilationFinal private TruffleLanguage.Env env;
+    @CompilationFinal private PrintStream outStream;
+    @CompilationFinal private PrintStream errStream;
     @CompilationFinal private boolean hasOtherPublicLanguages;
 
     @CompilationFinal private Options options;
@@ -154,15 +159,12 @@ public class RubyContext {
     public RubyContext(RubyLanguage language, TruffleLanguage.Env env) {
         Metrics.printTime("before-context-constructor");
 
-        this.preInitializing = env.isPreInitialization();
+        this.language = language;
+        setEnv(env);
         this.preInitialized = preInitializing;
 
         preInitializationManager = preInitializing ? new PreInitializationManager() : null;
 
-        this.language = language;
-        this.env = env;
-        this.hasOtherPublicLanguages = computeHasOtherPublicLanguages(env);
-
         options = createOptions(env, language.options);
 
         warningCategoryDeprecated = new AssumedValue<>(options.WARN_DEPRECATED);
@@ -266,9 +268,7 @@ public void initialize() {
      * initialization which needs to be performed to adapt to the new process and external environment. Calls are kept
      * in the same order as during normal initialization. */
     protected boolean patch(Env newEnv) {
-        this.env = newEnv;
-        this.hasOtherPublicLanguages = computeHasOtherPublicLanguages(newEnv);
-        this.preInitializing = newEnv.isPreInitialization();
+        setEnv(newEnv);
         if (preInitializing) {
             throw CompilerDirectives.shouldNotReachHere("Expected patch Env#isPreInitialization() to be false");
         }
@@ -389,6 +389,22 @@ private Options createOptions(TruffleLanguage.Env env, LanguageOptions languageO
         return options;
     }
 
+    private void setEnv(Env env) {
+        this.env = env;
+        this.outStream = printStreamFor(env.out());
+        this.errStream = printStreamFor(env.err());
+        this.hasOtherPublicLanguages = computeHasOtherPublicLanguages(env);
+        this.preInitializing = env.isPreInitialization();
+    }
+
+    private static PrintStream printStreamFor(OutputStream outputStream) {
+        try {
+            return new PrintStream(outputStream, true, "UTF-8");
+        } catch (UnsupportedEncodingException e) {
+            throw CompilerDirectives.shouldNotReachHere(e);
+        }
+    }
+
     private static boolean computeHasOtherPublicLanguages(Env env) {
         for (String language : env.getPublicLanguages().keySet()) {
             if (!language.equals(TruffleRuby.LANGUAGE_ID)) {
@@ -832,6 +848,14 @@ public AssumedValue<Boolean> getWarningCategoryExperimental() {
         return warningCategoryExperimental;
     }
 
+    public PrintStream getEnvOutStream() {
+        return outStream;
+    }
+
+    public PrintStream getEnvErrStream() {
+        return errStream;
+    }
+
     @TruffleBoundary
     public static String fileLine(SourceSection section) {
         if (section == null) {

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

@@ -37,7 +37,6 @@
  */
 package org.truffleruby.core;
 
-import java.io.PrintStream;
 import java.util.Map.Entry;
 
 import com.oracle.truffle.api.TruffleStackTrace;
@@ -67,7 +66,6 @@
 import org.truffleruby.language.RubyDynamicObject;
 import org.truffleruby.language.SafepointPredicate;
 import org.truffleruby.language.backtrace.Backtrace;
-import org.truffleruby.language.backtrace.BacktraceFormatter;
 import org.truffleruby.language.control.ExitException;
 import org.truffleruby.language.control.RaiseException;
 import org.truffleruby.language.control.ThrowException;
@@ -254,18 +252,17 @@ protected boolean restoreDefault(Object signalString, Object action,
         @Specialization(guards = "libSignalString.isRubyString(signalString)")
         protected boolean watchSignalProc(Object signalString, RubyProc action,
                 @CachedLibrary(limit = "2") RubyStringLibrary libSignalString) {
-            if (getContext().getThreadManager().getCurrentThread() != getContext().getThreadManager().getRootThread()) {
+            final RubyContext context = getContext();
+
+            if (context.getThreadManager().getCurrentThread() != context.getThreadManager().getRootThread()) {
                 // The proc will be executed on the main thread
                 SharedObjects.writeBarrier(getLanguage(), action);
             }
 
-            final RubyContext context = getContext();
-
-            String signalName = libSignalString.getJavaString(signalString);
+            final String signalName = libSignalString.getJavaString(signalString);
             return registerHandler(signalName, signal -> {
                 if (context.getOptions().SINGLE_THREADED) {
-                    RubyLanguage.LOGGER.severe(
-                            "signal " + signal + " caught but can't create a thread to handle it so ignoring");
+                    warnRestoreAndRaise(context, signalName, signal, "create a thread");
                     return;
                 }
 
@@ -278,17 +275,7 @@ protected boolean watchSignalProc(Object signalString, RubyProc action,
                 try {
                     prev = truffleContext.enter(this);
                 } catch (IllegalStateException e) { // Multi threaded access denied from Truffle
-                    // Not in a context, so we cannot use TruffleLogger
-                    final PrintStream printStream = BacktraceFormatter.printStreamFor(context.getEnv().err());
-                    printStream.println(
-                            "[ruby] SEVERE: signal " + signal +
-                                    " caught but can't attach a thread 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(printStream);
-                    }
+                    warnRestoreAndRaise(context, signalName, signal, "attach a thread");
                     return;
                 }
                 try {
@@ -302,6 +289,19 @@ protected boolean watchSignalProc(Object signalString, RubyProc 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/exception/CoreExceptions.java

@@ -12,7 +12,6 @@
 import static org.truffleruby.core.array.ArrayHelpers.createArray;
 
 import java.io.IOException;
-import java.io.PrintStream;
 import java.util.EnumSet;
 
 import com.oracle.truffle.api.interop.InvalidBufferOffsetException;
@@ -99,8 +98,7 @@ public void showExceptionIfDebug(RubyClass rubyClass, Object message, Backtrace
                 Object stderr = context.getCoreLibrary().getStderr();
                 RubyContext.send(stderr, "write", outputString);
             } else {
-                final PrintStream printStream = BacktraceFormatter.printStreamFor(context.getEnv().err());
-                printStream.println(output);
+                context.getEnvErrStream().println(output);
             }
         }
     }

src/main/java/org/truffleruby/core/kernel/KernelNodes.java

@@ -13,7 +13,6 @@
 import static org.truffleruby.language.dispatch.DispatchConfiguration.PUBLIC;
 
 import java.io.File;
-import java.io.PrintStream;
 import java.nio.file.Paths;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -26,7 +25,6 @@
 import org.jcodings.specific.UTF8Encoding;
 import org.truffleruby.RubyContext;
 import org.truffleruby.RubyLanguage;
-import org.truffleruby.SuppressFBWarnings;
 import org.truffleruby.builtins.CoreMethod;
 import org.truffleruby.builtins.CoreMethodArrayArgumentsNode;
 import org.truffleruby.builtins.CoreMethodNode;
@@ -1361,11 +1359,9 @@ protected Object p(VirtualFrame frame, Object value) {
             return value;
         }
 
-        @SuppressFBWarnings("OS")
         @TruffleBoundary
         private void print(Object inspected) {
-            final PrintStream stream = BacktraceFormatter.printStreamFor(getContext().getEnv().out());
-            stream.println(inspected.toString());
+            getContext().getEnvOutStream().println(inspected.toString());
         }
     }
 

src/main/java/org/truffleruby/language/backtrace/BacktraceFormatter.java

@@ -9,7 +9,6 @@
  */
 package org.truffleruby.language.backtrace;
 
-import com.oracle.truffle.api.CompilerDirectives;
 import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
 import com.oracle.truffle.api.TruffleStackTrace;
 import com.oracle.truffle.api.TruffleStackTraceElement;
@@ -34,9 +33,7 @@
 import org.truffleruby.language.methods.TranslateExceptionNode;
 import org.truffleruby.parser.RubySource;
 
-import java.io.OutputStream;
 import java.io.PrintStream;
-import java.io.UnsupportedEncodingException;
 import java.util.ArrayList;
 import java.util.EnumSet;
 import java.util.List;
@@ -113,7 +110,7 @@ public void printTopLevelRubyExceptionOnEnvStderr(RubyException rubyException) {
     @SuppressFBWarnings("OS")
     @TruffleBoundary
     public void printRubyExceptionOnEnvStderr(String info, RubyException rubyException) {
-        final PrintStream printer = printStreamFor(context.getEnv().err());
+        final PrintStream printer = context.getEnvErrStream();
         if (!info.isEmpty()) {
             printer.print(info);
         }
@@ -149,16 +146,7 @@ public void printRubyExceptionOnEnvStderr(String info, RubyException rubyExcepti
     @TruffleBoundary
     public void printBacktraceOnEnvStderr(Node currentNode) {
         final Backtrace backtrace = context.getCallStack().getBacktrace(currentNode);
-        final PrintStream printer = printStreamFor(context.getEnv().err());
-        printer.println(formatBacktrace(null, backtrace));
-    }
-
-    public static PrintStream printStreamFor(OutputStream outputStream) {
-        try {
-            return new PrintStream(outputStream, true, "UTF-8");
-        } catch (UnsupportedEncodingException e) {
-            throw CompilerDirectives.shouldNotReachHere(e);
-        }
+        context.getEnvErrStream().println(formatBacktrace(null, backtrace));
     }
 
     /** Format the backtrace as a String with \n between each line, but no trailing \n. */
@@ -396,7 +384,7 @@ public static void printInternalError(RubyContext context, Throwable throwable,
             throw ExceptionOperations.rethrow(throwable);
         }
 
-        final PrintStream stream = BacktraceFormatter.printStreamFor(context.getEnv().err());
+        final PrintStream stream = context.getEnvErrStream();
         final BacktraceFormatter formatter = context.getDefaultBacktraceFormatter();
         stream.println();
         stream.println("truffleruby: " + from + ",");