oracle / truffleruby

A high performance implementation of the Ruby programming language, built on GraalVM.
https://www.graalvm.org/ruby/
Other
3k stars 183 forks source link

SIGHUP, etc do not have a default signal handler that raises SignalException #2051

Open eregon opened 4 years ago

eregon commented 4 years ago

Originally posted by @eregon in https://github.com/oracle/truffleruby/issues/2045#issuecomment-660539088

MRI 2.6.6:
$ ruby -e 'Thread.new { sleep 1; `kill -HUP #{$$}` }; begin; sleep; rescue Exception => e; p e; end'
#<SignalException: SIGHUP>

TruffleRuby-dev:
$ ruby -e 'Thread.new { sleep 1; `kill -HUP #{$$}` }; begin; sleep; rescue Exception => e; p e; end' 
zsh: hangup     ruby -e 

Similar for e.g., SIGUSR2.

So it seems CRuby has signal handlers for many signals and transforms them in SignalException, and if they bubble up to the main thread, then re-sends the signal (after disabling the custom signal handlers?). TruffleRuby does the second part in https://github.com/oracle/truffleruby/blob/9f03f3c84387257dee80c1fa74b7a52deed2007c/src/main/java/org/truffleruby/language/exceptions/TopLevelRaiseHandler.java#L64 but not the first part, except for SIGINT (which is special, it's an Interupt and not SignalException)

If the signal is not caught, there is no difference:

MRI 2.6.6 and TruffleRuby-dev:
$ ruby -e 'Thread.new { sleep 1; `kill -HUP #{$$}` }; sleep'                                        
zsh: hangup     ruby -e 'Thread.new { sleep 1; `kill -HUP #{$$}` }; sleep'

Capturing all signals seems pretty bad for embedding or polyglot scenarios (Embeddings or other languages might want to use those signals). So that needs some thinking, and whether it's worth it.

Concrete examples are SIGQUIT and SIGALRM which are both used for printing backtraces/stacktraces on TruffleRuby.

I think explicit handling via trap() as done in https://github.com/socketry/async-container/commit/74cc92b5fc64019b6e6b0ab547caf8f980d760ec is better and de facto more portable (JRuby seems to also not have a handler by default for such signals).

cc @ioquatix

eregon commented 4 years ago

Of the list in https://github.com/ruby/ruby/blob/2eaa53e9a38b46442508878ac1795d91688701e3/signal.c#L1553-L1571:

ioquatix commented 4 years ago

I don't know what the answer is here. I'm happy to install handlers in my case, it's not a big overhead to get the required consistency.

My feeling is signals need to be left for user level. Things like SIGQUIT as outlined above are wrong IMHO. If you want to install debugging signals, it should be a specific flag, e.g. ruby --debug-signals or something like that. Maybe expose user level interface, they can write their own signal handlers to install the TruffleRuby specific debugging behaviour (which could be really useful - not arguing against that).

cc @headius

headius commented 4 years ago

JRuby leaves all signals alone, since some are used internally by JVMs (USR1 at least) or are already hooked up to features JVM users will expect to see (QUIT dumping thread stacks, INT performing a hard shutdown, etc). As for the rest, it seems inappropriate to add handlers for signals the user has not explicitly chosen to trap.

Trapping most signals at a user level will work fine, with caveats as listed here: https://github.com/jruby/jruby/wiki/Signal-Handling

ioquatix commented 4 years ago

If some signals are not available on a platform - platform basis (e.g. USR1 which is exceedingly common for use), it would make sense to:

  1. Ensure it's clearly documented for all platforms (above linked docs are a good start!), and
  2. Provide some interface to identify signals which can't be used (e.g. Signal.available), and/or
  3. Maybe try to provide a guarantee that some signals will always be available (even if it breaks default behaviour like thread backtraces in JVM), e.g. SIGINT, SIGQUIT, SIGHUP.

Regarding the above API, we should probably consider the needs of server authors, e.g. how is one supposed to know to use USR1 or USR2 or something else?

chrisseaton commented 4 years ago

Ensure it's clearly documented for all platforms

We've done our best - in https://github.com/oracle/truffleruby/blob/9f03f3c84387257dee80c1fa74b7a52deed2007c/doc/user/compatibility.md#signals.

Provide some interface to identify signals which can't be used

I proposed an API ages ago for a Signal.trappable? but it didn't get a lot of support. Maybe we should go back to that?

Maybe try to provide a guarantee that some signals will always be available (even if it breaks default behaviour like thread backtraces in JVM)

We can't trap USR1 or QUIT on the JVM because the JVM doesn't let us. It's not a case of doing it breaks the JVM, I just don't believe it's possible to do in the first place. You could ship a modified JVM... which is sort of what we do for native image, partly for this kind of reason.

ioquatix commented 4 years ago

Platform specific limits need to be available through a programmatic interface otherwise we are going to have a ton of if platform_x? ... elsif platform_y? ...

eregon commented 4 years ago

One way to check if a signal can be trap-ed is to just trap(signal) and see if that raises an error. This also applies to CRuby BTW, on Windows only a couple signals are available. And I would guess it also varies by OS. I don't think there is any API at any level to list available signals unfortunately, it's always a "trap-or-fail" approach.

eregon commented 4 years ago

If you want to install debugging signals, it should be a specific flag, e.g. ruby --debug-signals or something like that. Maybe expose user level interface, they can write their own signal handlers to install the TruffleRuby specific debugging behaviour (which could be really useful - not arguing against that).

This kind of debugging is mostly useful as in enabled-by-default, not as second thought (e.g., useful to debug hangs). So enabled by default is a must here.

eregon commented 4 years ago

IMHO this issue is about whether SIGHUP, SIGTERM, SIGUSR1, SIGUSR2 should raise SignalException on TruffleRuby for compatibility. This would only applies when run from the truffleruby launcher (not when embedded), and maybe disabled if --polyglot is passed.

There is already explicit documentation listing the signals and there are external constraints that limit available signals that already exist even on CRuby: it depends on the "platform" and "platform" includes whether it's JVM or not (and which JVM on JRuby).