Skip to content

Commit 3b48762

Browse files
eregoneregon
committed
[GR-47629] Raise native signal after closing the Context for uncaught SignalException
* Otherwise hooks on Context#close are not done, e.g., --cpusampler output. * Rename BasicPlatform to Platform and remove redundant org.truffleruby.platform.Platform.
1 parent 7c5bef1 commit 3b48762

File tree

20 files changed

+155
-142
lines changed

20 files changed

+155
-142
lines changed

‎CHANGELOG.md

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

1617
Compatibility:
1718

‎mx.truffleruby/suite.py

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -207,12 +207,22 @@
207207
"license": ["EPL-2.0"],
208208
},
209209

210-
"org.truffleruby.rubysignal": {
210+
"org.truffleruby.signal": {
211+
"dir": "src/signal",
212+
"sourceDirs": ["java"],
213+
"jniHeaders": True,
214+
"javaCompliance": "17+",
215+
"checkstyle": "org.truffleruby",
216+
"workingSets": "TruffleRuby",
217+
"license": ["EPL-2.0"],
218+
},
219+
220+
"org.truffleruby.librubysignal": {
211221
"dir": "src/main/c/rubysignal",
212222
"native": "shared_lib",
213223
"deliverable": "rubysignal",
214224
"buildDependencies": [
215-
"org.truffleruby", # for the generated JNI header file
225+
"org.truffleruby.signal", # for the generated JNI header file
216226
],
217227
"use_jdk_headers": True, # the generated JNI header includes jni.h
218228
"cflags": ["-g", "-Wall", "-Werror", "-pthread"],
@@ -245,7 +255,6 @@
245255
"org.truffleruby": {
246256
"dir": "src/main",
247257
"sourceDirs": ["java"],
248-
"jniHeaders": True,
249258
"requires": [
250259
"java.logging",
251260
"java.management",
@@ -449,11 +458,13 @@
449458
"exports": [
450459
"org.truffleruby.shared to org.graalvm.ruby, org.graalvm.ruby.launcher",
451460
"org.truffleruby.shared.options to org.graalvm.ruby, org.graalvm.ruby.launcher",
461+
"org.truffleruby.signal to org.graalvm.ruby, org.graalvm.ruby.launcher",
452462
],
453463
},
454464
"useModulePath": True,
455465
"dependencies": [
456-
"org.truffleruby.shared"
466+
"org.truffleruby.shared",
467+
"org.truffleruby.signal",
457468
],
458469
"distDependencies": [
459470
"truffleruby:TRUFFLERUBY-ANNOTATIONS",
@@ -700,7 +711,7 @@
700711
"dependency:org.truffleruby.cext/src/main/c/truffleposix/<lib:truffleposix>",
701712
"dependency:org.truffleruby.cext/src/main/c/cext/<lib:truffleruby>",
702713
"dependency:org.truffleruby.cext/src/main/c/cext-trampoline/<lib:trufflerubytrampoline>",
703-
"dependency:org.truffleruby.rubysignal",
714+
"dependency:org.truffleruby.librubysignal",
704715
],
705716
# The platform-specific files from debug and rbs, see comment above
706717
"lib/gems/": "file:lib/gems/extensions",

‎spec/truffle/signal_exception_spec.rb

Lines changed: 0 additions & 20 deletions
This file was deleted.

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

Lines changed: 46 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -29,15 +29,19 @@
2929
import org.graalvm.polyglot.PolyglotException;
3030
import org.graalvm.polyglot.Source;
3131
import org.graalvm.polyglot.Value;
32+
import org.truffleruby.shared.ProcessStatus;
33+
import org.truffleruby.shared.Platform;
3234
import org.truffleruby.shared.Metrics;
3335
import org.truffleruby.shared.TruffleRuby;
3436
import org.truffleruby.shared.options.OptionsCatalog;
37+
import org.truffleruby.signal.LibRubySignal;
3538

3639
public class RubyLauncher extends AbstractLanguageLauncher {
3740

3841
private CommandLineOptions config;
3942
private String implementationName = null;
4043
private boolean helpOptionUsed = false; // Any --help* option
44+
private String rubyHome = null;
4145

4246
/** NOTE: not actually used by thin launchers. The first method called with the arguments is
4347
* {@link #preprocessArguments}. */
@@ -292,30 +296,51 @@ private int runRubyMain(Context.Builder contextBuilder, CommandLineOptions confi
292296

293297
contextBuilder.arguments(TruffleRuby.LANGUAGE_ID, config.getArguments());
294298

295-
int result = runContext(contextBuilder, config);
299+
int processStatus = runContext(contextBuilder, config);
296300

297301
final boolean runTwice = config.getUnknownArguments().contains("--run-twice") ||
298302
config.getUnknownArguments().contains("--run-twice=true");
299303
if (runTwice) {
300304
final int secondResult = runContext(contextBuilder, config);
301-
if (secondResult != 0 && result == 0) {
302-
result = secondResult;
305+
if (secondResult != 0 && processStatus == 0) {
306+
processStatus = secondResult;
303307
}
304308
}
305309

306-
return result;
310+
// SignalExeption exit, we need to raise(3) the native signal to set the correct process waitpid(3) status
311+
if (ProcessStatus.isSignal(processStatus)) {
312+
int signalNumber = ProcessStatus.toSignal(processStatus);
313+
314+
if (rubyHome != null) {
315+
LibRubySignal.loadLibrary(rubyHome, Platform.LIB_SUFFIX);
316+
LibRubySignal.restoreSystemHandlerAndRaise(signalNumber);
317+
// Some signals are ignored by default, such as SIGWINCH and SIGCHLD, in that exit with 1 like CRuby
318+
return 1;
319+
} else {
320+
System.err.println("The TruffleRuby context ended with signal " + signalNumber +
321+
" but since librubysignal is not found we exit with code " + signalNumber +
322+
" instead of raising the signal");
323+
return signalNumber;
324+
}
325+
}
326+
327+
return processStatus;
307328
}
308329

309330
private int runContext(Context.Builder builder, CommandLineOptions config) {
310331
try (Context context = builder.build()) {
311332
Metrics.printTime("before-run");
312333

334+
Value rubyHome = context.eval(source(
335+
// language=ruby
336+
"Truffle::Boot.ruby_home"));
337+
if (rubyHome.isString()) {
338+
this.rubyHome = rubyHome.asString();
339+
}
340+
313341
if (config.executionAction == ExecutionAction.PATH) {
314-
final Source source = Source.newBuilder(
315-
TruffleRuby.LANGUAGE_ID,
316-
// language=ruby
317-
"-> name { Truffle::Boot.find_s_file(name) }",
318-
TruffleRuby.BOOT_SOURCE_NAME).internal(true).buildLiteral();
342+
final Source source = source(// language=ruby
343+
"-> name { Truffle::Boot.find_s_file(name) }");
319344

320345
config.executionAction = ExecutionAction.FILE;
321346
final Value file = context.eval(source).execute(config.toExecute);
@@ -329,34 +354,35 @@ private int runContext(Context.Builder builder, CommandLineOptions config) {
329354
}
330355

331356
if (config.logProcessArguments) {
332-
Value logInfo = context.eval(
333-
"ruby",
357+
Value logInfo = context.eval(source(
334358
// language=ruby
335-
"-> message { Truffle::Debug.log_info(message) }");
359+
"-> message { Truffle::Debug.log_info(message) }"));
336360
String message = "new process: truffleruby " + String.join(" ", config.initialArguments);
337361
logInfo.executeVoid(message);
338362
}
339363

340-
final Source source = Source.newBuilder(
341-
TruffleRuby.LANGUAGE_ID,
342-
// language=ruby
343-
"-> argc, argv, kind, to_execute { Truffle::Boot.main(argc, argv, kind, to_execute) }",
344-
TruffleRuby.BOOT_SOURCE_NAME).internal(true).buildLiteral();
364+
final Source source = source(// language=ruby
365+
"-> argc, argv, kind, to_execute { Truffle::Boot.main(argc, argv, kind, to_execute) }");
345366

346367
final int argc = getNativeArgc();
347368
final long argv = getNativeArgv();
348369
final String kind = config.executionAction.name();
349-
final int exitCode = context.eval(source).execute(argc, argv, kind, config.toExecute).asInt();
370+
final int processStatus = context.eval(source).execute(argc, argv, kind, config.toExecute).asInt();
350371
Metrics.printTime("after-run");
351-
return exitCode;
372+
return processStatus;
352373
} catch (PolyglotException e) {
353374
getError().println(
354375
"truffleruby: an exception escaped out of the interpreter - this is an implementation bug");
355-
e.printStackTrace();
376+
e.printStackTrace(System.err);
356377
return 1;
357378
}
358379
}
359380

381+
private static Source source(String code) {
382+
return Source.newBuilder(TruffleRuby.LANGUAGE_ID, code, TruffleRuby.BOOT_SOURCE_NAME).internal(true)
383+
.buildLiteral();
384+
}
385+
360386
private static List<String> getArgsFromEnvVariable(String name) {
361387
String value = System.getenv(name);
362388
if (value != null) {

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

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,16 @@ JNIEXPORT jint JNICALL Java_org_truffleruby_signal_LibRubySignal_sendSIGVTALRMTo
3737
}
3838

3939
JNIEXPORT jlong JNICALL Java_org_truffleruby_signal_LibRubySignal_getNativeThreadID(JNIEnv *env, jclass clazz) {
40-
#ifdef __APPLE__
41-
uint64_t native_id;
42-
pthread_threadid_np(NULL, &native_id);
43-
#elif defined(__linux__)
44-
pid_t native_id = (pid_t) syscall(SYS_gettid);
45-
#endif
46-
return (jlong) native_id;
40+
#ifdef __APPLE__
41+
uint64_t native_id;
42+
pthread_threadid_np(NULL, &native_id);
43+
#elif defined(__linux__)
44+
pid_t native_id = (pid_t) syscall(SYS_gettid);
45+
#endif
46+
return (jlong) native_id;
47+
}
48+
49+
JNIEXPORT void JNICALL Java_org_truffleruby_signal_LibRubySignal_restoreSystemHandlerAndRaise(JNIEnv *env, jclass clazz, jint signo) {
50+
signal(signo, SIG_DFL);
51+
raise(signo);
4752
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@
116116
import org.truffleruby.parser.ParsingParameters;
117117
import org.truffleruby.parser.RubySource;
118118
import org.truffleruby.parser.TranslatorEnvironment;
119-
import org.truffleruby.platform.Platform;
119+
import org.truffleruby.shared.Platform;
120120
import org.truffleruby.shared.Metrics;
121121
import org.truffleruby.shared.TruffleRuby;
122122
import org.truffleruby.shared.options.OptionsCatalog;

‎src/main/java/org/truffleruby/core/TruffleSystemNodes.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,7 @@
6969
import org.truffleruby.language.RubyGuards;
7070
import org.truffleruby.language.control.RaiseException;
7171
import org.truffleruby.language.library.RubyStringLibrary;
72-
import org.truffleruby.platform.Platform;
73-
import org.truffleruby.shared.BasicPlatform;
72+
import org.truffleruby.shared.Platform;
7473

7574
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
7675
import com.oracle.truffle.api.TruffleFile;
@@ -214,7 +213,7 @@ public abstract static class HostCPUNode extends CoreMethodNode {
214213

215214
@Specialization
216215
RubyString hostCPU() {
217-
return createString(fromJavaStringNode, BasicPlatform.getArchName(), Encodings.UTF_8);
216+
return createString(fromJavaStringNode, Platform.getArchName(), Encodings.UTF_8);
218217
}
219218

220219
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@
6565
import com.oracle.truffle.api.TruffleStackTrace;
6666
import com.oracle.truffle.api.nodes.Node;
6767
import com.oracle.truffle.api.object.Shape;
68+
import org.truffleruby.shared.Platform;
6869
import org.truffleruby.signal.LibRubySignal;
6970

7071
public final class ThreadManager {
@@ -102,7 +103,7 @@ public void initialize() {
102103
language.getRubyHome() != null;
103104
nativeInterrupt = context.getOptions().NATIVE_INTERRUPT && useLibRubySignal;
104105
if (useLibRubySignal) {
105-
LibRubySignal.loadLibrary(language.getRubyHome());
106+
LibRubySignal.loadLibrary(language.getRubyHome(), Platform.LIB_SUFFIX);
106107
}
107108
if (nativeInterrupt) {
108109
LibRubySignal.setupSIGVTALRMEmptySignalHandler();

‎src/main/java/org/truffleruby/debug/TruffleDebugNodes.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,9 +114,9 @@
114114
import org.truffleruby.parser.YARPTranslatorDriver;
115115
import org.truffleruby.parser.parser.ParserConfiguration;
116116
import org.truffleruby.parser.scope.StaticScope;
117-
import org.truffleruby.platform.Platform;
118117
import org.prism.Loader;
119118
import org.prism.Parser;
119+
import org.truffleruby.shared.Platform;
120120

121121
@CoreModule("Truffle::Debug")
122122
public abstract class TruffleDebugNodes {

‎src/main/java/org/truffleruby/language/exceptions/TopLevelRaiseHandler.java

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,11 @@
2626
import org.truffleruby.language.control.RaiseException;
2727
import org.truffleruby.language.dispatch.DispatchNode;
2828
import org.truffleruby.language.objects.IsANode;
29+
import org.truffleruby.shared.ProcessStatus;
2930

3031
public final class TopLevelRaiseHandler extends RubyBaseNode {
3132

33+
// Must use ProcessStatus.exitCode() or ProcessStatus.signal() for every return
3234
@TruffleBoundary
3335
public int execute(Runnable body) {
3436
int exitCode = 0;
@@ -39,7 +41,7 @@ public int execute(Runnable body) {
3941
body.run();
4042
} catch (ExitException e) {
4143
// hard #exit!, return immediately, skip at_exit hooks
42-
return e.getCode();
44+
return ProcessStatus.exitCode(e.getCode());
4345
} catch (AbstractTruffleException e) {
4446
// No KillException, it's a SystemExit instead for the main thread
4547
assert !(e instanceof KillException) : e;
@@ -58,7 +60,7 @@ public int execute(Runnable body) {
5860

5961
BacktraceFormatter.printInternalError(getContext(), e,
6062
"an internal exception escaped out of the interpreter");
61-
return 1;
63+
return ProcessStatus.exitCode(1);
6264
}
6365

6466
// Execute at_exit hooks (except if hard #exit!)
@@ -77,7 +79,10 @@ public int execute(Runnable body) {
7779
getContext().getDefaultBacktraceFormatter().printTopLevelRubyExceptionOnEnvStderr(caughtException);
7880
}
7981

80-
handleSignalException(caughtException);
82+
int signo = handleSignalException(caughtException);
83+
if (signo != 0) {
84+
return ProcessStatus.signal(signo);
85+
}
8186
}
8287
} catch (ExitException e) {
8388
// hard #exit! during at_exit: ignore the main script exception
@@ -86,10 +91,10 @@ public int execute(Runnable body) {
8691
throw e;
8792
} catch (RuntimeException | Error e) { // Internal error
8893
BacktraceFormatter.printInternalError(getContext(), e, step);
89-
return 1;
94+
return ProcessStatus.exitCode(1);
9095
}
9196

92-
return exitCode;
97+
return ProcessStatus.exitCode(exitCode);
9398
}
9499

95100
private int statusFromException(AbstractTruffleException exception) {
@@ -105,16 +110,17 @@ private int statusFromException(AbstractTruffleException exception) {
105110
}
106111
}
107112

108-
/** See rb_ec_cleanup() in CRuby, which calls ruby_default_signal(), which uses raise(3). */
109-
private void handleSignalException(AbstractTruffleException exception) {
113+
/** See rb_ec_cleanup() in CRuby, which calls ruby_default_signal(), which uses raise(3). We need to raise(3) after
114+
* Context#close for e.g. letting tools print their output, so just return the signal number here. */
115+
private int handleSignalException(AbstractTruffleException exception) {
110116
if (exception instanceof RaiseException) {
111117
RubyException rubyException = ((RaiseException) exception).getException();
112118

113119
if (IsANode.getUncached().executeIsA(rubyException, coreLibrary().signalExceptionClass)) {
114-
// Calls raise(3) or no-op
115-
DispatchNode.getUncached().call(rubyException, "reached_top_level");
120+
return (int) DispatchNode.getUncached().call(rubyException, "signo");
116121
}
117122
}
123+
return 0;
118124
}
119125

120126
}

0 commit comments

Comments
 (0)