[GR-47629] Raise native signal after closing the Context for uncaught SignalException

PullRequest: truffleruby/4091

Author: eregon

CHANGELOG.md

@@ -12,6 +12,7 @@ Bug fixes:
 * Fix `Proc#parameters` and return all the numbered parameters lower than the used explicitly ones (@andrykonchin).
 * Fix some C API functions which were failing when called with Ruby values represented as Java primitives (#3352, @eregon).
 * Fix `IO.select([io], nil, [io])` on macOS, it was hanging due to a bug in macOS `poll(2)` (#3346, @eregon, @andrykonchin).
+* Run context cleanup such as showing the output of tools when `SignalException` and `Interrupt` escape (@eregon).
 
 Compatibility:
 

mx.truffleruby/eclipse-settings/org.eclipse.jdt.core.prefs

@@ -1,11 +1,13 @@
 org.eclipse.jdt.core.compiler.problem.invalidJavadoc=warning
+org.eclipse.jdt.core.compiler.problem.invalidJavadocTags=disabled
 org.eclipse.jdt.core.compiler.problem.missingDeprecatedAnnotation=warning
 org.eclipse.jdt.core.compiler.problem.missingOverrideAnnotation=warning
 org.eclipse.jdt.core.compiler.problem.unusedParameter=ignore
 org.eclipse.jdt.core.compiler.problem.reportMethodCanBeStatic=ignore
 org.eclipse.jdt.core.compiler.problem.localVariableHiding=ignore
 org.eclipse.jdt.core.compiler.problem.parameterAssignment=ignore
 org.eclipse.jdt.core.compiler.problem.unusedWarningToken=ignore
+org.eclipse.jdt.core.compiler.problem.overridingPackageDefaultMethod=ignore
 org.eclipse.jdt.core.formatter.insert_space_after_opening_brace_in_array_initializer=insert
 org.eclipse.jdt.core.formatter.insert_space_before_closing_brace_in_array_initializer=insert
 org.eclipse.jdt.core.formatter.never_indent_block_comments_on_first_column=false

mx.truffleruby/suite.py

@@ -207,12 +207,22 @@
             "license": ["EPL-2.0"],
         },
 
-        "org.truffleruby.rubysignal": {
+        "org.truffleruby.signal": {
+            "dir": "src/signal",
+            "sourceDirs": ["java"],
+            "jniHeaders": True,
+            "javaCompliance": "17+",
+            "checkstyle": "org.truffleruby",
+            "workingSets": "TruffleRuby",
+            "license": ["EPL-2.0"],
+        },
+
+        "org.truffleruby.librubysignal": {
             "dir": "src/main/c/rubysignal",
             "native": "shared_lib",
             "deliverable": "rubysignal",
             "buildDependencies": [
-                "org.truffleruby", # for the generated JNI header file
+                "org.truffleruby.signal", # for the generated JNI header file
             ],
             "use_jdk_headers": True, # the generated JNI header includes jni.h
             "cflags": ["-g", "-Wall", "-Werror", "-pthread"],
@@ -245,7 +255,6 @@
         "org.truffleruby": {
             "dir": "src/main",
             "sourceDirs": ["java"],
-            "jniHeaders": True,
             "requires": [
                 "java.logging",
                 "java.management",
@@ -278,6 +287,7 @@
             "checkstyle": "org.truffleruby",
             "workingSets": "TruffleRuby",
             "findbugsIgnoresGenerated": True,
+            "forceJavac": True, # GR-51148 We need to force javac to silence a ECJ warning in generated code
             "license": [
                 "EPL-2.0",          # JRuby (we're choosing EPL out of EPL,GPL,LGPL)
                 "BSD-new",          # Rubinius
@@ -449,11 +459,13 @@
                 "exports": [
                     "org.truffleruby.shared to org.graalvm.ruby, org.graalvm.ruby.launcher",
                     "org.truffleruby.shared.options to org.graalvm.ruby, org.graalvm.ruby.launcher",
+                    "org.truffleruby.signal to org.graalvm.ruby, org.graalvm.ruby.launcher",
                 ],
             },
             "useModulePath": True,
             "dependencies": [
-                "org.truffleruby.shared"
+                "org.truffleruby.shared",
+                "org.truffleruby.signal",
             ],
             "distDependencies": [
                 "truffleruby:TRUFFLERUBY-ANNOTATIONS",
@@ -700,7 +712,7 @@
                     "dependency:org.truffleruby.cext/src/main/c/truffleposix/<lib:truffleposix>",
                     "dependency:org.truffleruby.cext/src/main/c/cext/<lib:truffleruby>",
                     "dependency:org.truffleruby.cext/src/main/c/cext-trampoline/<lib:trufflerubytrampoline>",
-                    "dependency:org.truffleruby.rubysignal",
+                    "dependency:org.truffleruby.librubysignal",
                 ],
                 # The platform-specific files from debug and rbs, see comment above
                 "lib/gems/": "file:lib/gems/extensions",

spec/truffle/signal_exception_spec.rb

@@ -29,15 +29,19 @@
 import org.graalvm.polyglot.PolyglotException;
 import org.graalvm.polyglot.Source;
 import org.graalvm.polyglot.Value;
+import org.truffleruby.shared.ProcessStatus;
+import org.truffleruby.shared.Platform;
 import org.truffleruby.shared.Metrics;
 import org.truffleruby.shared.TruffleRuby;
 import org.truffleruby.shared.options.OptionsCatalog;
+import org.truffleruby.signal.LibRubySignal;
 
 public class RubyLauncher extends AbstractLanguageLauncher {
 
     private CommandLineOptions config;
     private String implementationName = null;
     private boolean helpOptionUsed = false; // Any --help* option
+    private String rubyHome = null;
 
     /** NOTE: not actually used by thin launchers. The first method called with the arguments is
      * {@link #preprocessArguments}. */
@@ -292,30 +296,51 @@ private int runRubyMain(Context.Builder contextBuilder, CommandLineOptions confi
 
         contextBuilder.arguments(TruffleRuby.LANGUAGE_ID, config.getArguments());
 
-        int result = runContext(contextBuilder, config);
+        int processStatus = runContext(contextBuilder, config);
 
         final boolean runTwice = config.getUnknownArguments().contains("--run-twice") ||
                 config.getUnknownArguments().contains("--run-twice=true");
         if (runTwice) {
             final int secondResult = runContext(contextBuilder, config);
-            if (secondResult != 0 && result == 0) {
-                result = secondResult;
+            if (secondResult != 0 && processStatus == 0) {
+                processStatus = secondResult;
             }
         }
 
-        return result;
+        // SignalExeption exit, we need to raise(3) the native signal to set the correct process waitpid(3) status
+        if (ProcessStatus.isSignal(processStatus)) {
+            int signalNumber = ProcessStatus.toSignal(processStatus);
+
+            if (rubyHome != null) {
+                LibRubySignal.loadLibrary(rubyHome, Platform.LIB_SUFFIX);
+                LibRubySignal.restoreSystemHandlerAndRaise(signalNumber);
+                // Some signals are ignored by default, such as SIGWINCH and SIGCHLD, in that exit with 1 like CRuby
+                return 1;
+            } else {
+                System.err.println("The TruffleRuby context ended with signal " + signalNumber +
+                        " but since librubysignal is not found we exit with code " + signalNumber +
+                        " instead of raising the signal");
+                return signalNumber;
+            }
+        }
+
+        return processStatus;
     }
 
     private int runContext(Context.Builder builder, CommandLineOptions config) {
         try (Context context = builder.build()) {
             Metrics.printTime("before-run");
 
+            Value rubyHome = context.eval(source(
+                    // language=ruby
+                    "Truffle::Boot.ruby_home"));
+            if (rubyHome.isString()) {
+                this.rubyHome = rubyHome.asString();
+            }
+
             if (config.executionAction == ExecutionAction.PATH) {
-                final Source source = Source.newBuilder(
-                        TruffleRuby.LANGUAGE_ID,
-                        // language=ruby
-                        "-> name { Truffle::Boot.find_s_file(name) }",
-                        TruffleRuby.BOOT_SOURCE_NAME).internal(true).buildLiteral();
+                final Source source = source(// language=ruby
+                        "-> name { Truffle::Boot.find_s_file(name) }");
 
                 config.executionAction = ExecutionAction.FILE;
                 final Value file = context.eval(source).execute(config.toExecute);
@@ -329,34 +354,35 @@ private int runContext(Context.Builder builder, CommandLineOptions config) {
             }
 
             if (config.logProcessArguments) {
-                Value logInfo = context.eval(
-                        "ruby",
+                Value logInfo = context.eval(source(
                         // language=ruby
-                        "-> message { Truffle::Debug.log_info(message) }");
+                        "-> message { Truffle::Debug.log_info(message) }"));
                 String message = "new process: truffleruby " + String.join(" ", config.initialArguments);
                 logInfo.executeVoid(message);
             }
 
-            final Source source = Source.newBuilder(
-                    TruffleRuby.LANGUAGE_ID,
-                    // language=ruby
-                    "-> argc, argv, kind, to_execute { Truffle::Boot.main(argc, argv, kind, to_execute) }",
-                    TruffleRuby.BOOT_SOURCE_NAME).internal(true).buildLiteral();
+            final Source source = source(// language=ruby
+                    "-> argc, argv, kind, to_execute { Truffle::Boot.main(argc, argv, kind, to_execute) }");
 
             final int argc = getNativeArgc();
             final long argv = getNativeArgv();
             final String kind = config.executionAction.name();
-            final int exitCode = context.eval(source).execute(argc, argv, kind, config.toExecute).asInt();
+            final int processStatus = context.eval(source).execute(argc, argv, kind, config.toExecute).asInt();
             Metrics.printTime("after-run");
-            return exitCode;
+            return processStatus;
         } catch (PolyglotException e) {
             getError().println(
                     "truffleruby: an exception escaped out of the interpreter - this is an implementation bug");
-            e.printStackTrace();
+            e.printStackTrace(System.err);
             return 1;
         }
     }
 
+    private static Source source(String code) {
+        return Source.newBuilder(TruffleRuby.LANGUAGE_ID, code, TruffleRuby.BOOT_SOURCE_NAME).internal(true)
+                .buildLiteral();
+    }
+
     private static List<String> getArgsFromEnvVariable(String name) {
         String value = System.getenv(name);
         if (value != null) {

src/launcher/java/org/truffleruby/launcher/RubyLauncher.java

@@ -37,11 +37,16 @@ JNIEXPORT jint JNICALL Java_org_truffleruby_signal_LibRubySignal_sendSIGVTALRMTo
 }
 
 JNIEXPORT jlong JNICALL Java_org_truffleruby_signal_LibRubySignal_getNativeThreadID(JNIEnv *env, jclass clazz) {
-  #ifdef __APPLE__
-      uint64_t native_id;
-      pthread_threadid_np(NULL, &native_id);
-  #elif defined(__linux__)
-      pid_t native_id = (pid_t) syscall(SYS_gettid);
-  #endif
-      return (jlong) native_id;
+#ifdef __APPLE__
+  uint64_t native_id;
+  pthread_threadid_np(NULL, &native_id);
+#elif defined(__linux__)
+  pid_t native_id = (pid_t) syscall(SYS_gettid);
+#endif
+  return (jlong) native_id;
+}
+
+JNIEXPORT void JNICALL Java_org_truffleruby_signal_LibRubySignal_restoreSystemHandlerAndRaise(JNIEnv *env, jclass clazz, jint signo) {
+  signal(signo, SIG_DFL);
+  raise(signo);
 }

src/main/c/rubysignal/src/rubysignal.c

@@ -116,7 +116,7 @@
 import org.truffleruby.parser.ParsingParameters;
 import org.truffleruby.parser.RubySource;
 import org.truffleruby.parser.TranslatorEnvironment;
-import org.truffleruby.platform.Platform;
+import org.truffleruby.shared.Platform;
 import org.truffleruby.shared.Metrics;
 import org.truffleruby.shared.TruffleRuby;
 import org.truffleruby.shared.options.OptionsCatalog;

src/main/java/org/truffleruby/RubyLanguage.java

@@ -215,12 +215,6 @@ public static final class HandleBlock {
 
         @SuppressWarnings("unused") private Cleanable cleanable;
 
-        private HandleBlock() {
-            base = 0;
-            cleanable = null;
-            wrappers = null;
-        }
-
         public HandleBlock(RubyContext context, RubyLanguage language, ValueWrapperManager manager) {
             HandleBlockAllocator allocator = language.handleBlockAllocator;
             long base = allocator.getFreeBlock();

src/main/java/org/truffleruby/cext/ValueWrapperManager.java

@@ -32,7 +32,7 @@ public Object[] toArray() {
 
     @TruffleBoundary
     public WeakSetIterator<E> iterator() {
-        return new WeakSetIterator<E>(map.keySet().iterator());
+        return new WeakSetIterator<>(map.keySet().iterator());
     }
 
     private static final class WeakSetIterator<E> implements Iterator<E> {

src/main/java/org/truffleruby/collections/ConcurrentWeakSet.java

@@ -69,8 +69,7 @@
 import org.truffleruby.language.RubyGuards;
 import org.truffleruby.language.control.RaiseException;
 import org.truffleruby.language.library.RubyStringLibrary;
-import org.truffleruby.platform.Platform;
-import org.truffleruby.shared.BasicPlatform;
+import org.truffleruby.shared.Platform;
 
 import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
 import com.oracle.truffle.api.TruffleFile;
@@ -214,7 +213,7 @@ public abstract static class HostCPUNode extends CoreMethodNode {
 
         @Specialization
         RubyString hostCPU() {
-            return createString(fromJavaStringNode, BasicPlatform.getArchName(), Encodings.UTF_8);
+            return createString(fromJavaStringNode, Platform.getArchName(), Encodings.UTF_8);
         }
 
     }