This code will not print on CTRL-C, which might be [unexpected]:
at_exit { print "Exiting" }
sleep
Currently, it can be handled, but feels awkward:
at_exit { print "Exiting" }
Signal::INT.trap { exit }
sleep
Crystal 0.32.1 [41bd18fbe] (2019-12-18)
LLVM: 8.0.0
Default target: x86_64-unknown-linux-gnu
Same behavior in Ruby... so probably not a bug?
@asterite
It prints for me with Ruby 2.6.
^CExitingTraceback (most recent call last):
1: from ./t.rb:5:in `<main>'
./t.rb:5:in `sleep': Interrupt
Oh, you are right, I missed it because it was mixed with the other message.
I think it might be fine adding exit
as default handler for Signal::INT
.
I guess Ruby has a default SIGINT
I've just noticed, Ruby triggers at_exit
on SIGTERM too.
Would be nice to have it that way with Crystal.
馃憤 on my side. As long as at_exit has a way opt-out the sigint and/or sigterm registration.
I've been overtaken by some concerns about this issue.
What if one wants to distinguish between normal program exiting and exiting because of receiving SIGTERM or SIGINT? Currently, it can easily be handled:
at_exit { do_normal_exit_routine }
Signal::INT { do_siging_routine }
Signal::TERM { do_sigterm_routine }
But if at_exit
is going to catch all of those, one'd have to use flags?
enum TrackSignal
SIGINT
SIGTERM
end
module Exit
class_property signal : TrackSignal
end
at_exit do
case Exit.signal
when TrackSignal::SIGINT then do_sigint_routine
when TrackSignal::SIGTERM then do_sigterm_routine
else do_normal_exit_routine
end
end
Signal::INT.trap { Exit.signal = TrackSignal::SIGINT }
Signal::TERM.trap { Exit.signal = TrackSignal::SIGTERM }
So, despite being more verbose sometimes, the current implementation is more explicit and clear. Sorry for wasting your time.
If you want to distinguish those cases you can handle sigint. If not, I think it's fine to run at_exit handlers as expected.
@asterite please, consider this:
def should_run_only_at_normal_exit
# ...
end
def should_run_only_at_sigint
# ...
end
at_exit { should_run_only_at_normal_exit }
Signal::INT.trap { should_run_only_at_sigint }
How to prevent the should_run_only_at_normal_exit
method from being triggered by SIGINT
then?
I was suggesting something like the following to have the flexibility
at_exit(sig_int: false, sig_term: false) { run_only_at_normal_exit }
at_exit { run_on_normal_int_or_term }
at_exit(sig_int: true, sig_term: true) { run_on_normal_int_or_term } # same as above
Signal::INT.trap { run_only_at_int }
Signal::TERM.trap { run_only_at_term }
at_exit(sig_term: true) { run_on_normal_or_term }
at_exit(sig_int: true) { run_on_normal_or_int }
I'd lay out the relationship in the opposite direction: Call exit(run_at_exit_handlers: false)
from signal handlers that are not supposed to execute at_exit
handlers. This could have other use cases as well.
However, I'm not entirely sure if we really need an overload for exit
or just let the application provide and use appropriate methods instead of calling top-level exit
directly.
The motivation is that users can use at_exit
, not mention signals at all and the block will be executed even if the app exits due to a ctrl+c. It seems a good _default_ for me.
If Ruby doesn't allow doing what you want, why do we need to do something different?
Nobody seems to have problems with how it works in Ruby. at_exit
should run at exit regardless of how the program exits.
I think doing the same thing as Ruby is fine.
If Ruby doesn't allow doing what you want, why do we need to do something different?
Ruby is great, but not ideal. Crystal is already different (and better) at some areas, so why not keeping it that way?
IMO having default SIGINT and SIGTERM traps that merely call exit
is a good behavior. That would allow to gracefully terminate the program, running all at_exit
callbacks.
I think adding configuration to run an at_exit
callback is unrequited complexity and mixes unrelated concepts together: signals may lead to exit, but exit itself has nothing to do with signals.
I agree with @straight-shoota that we could instead have a mean to immediately exit, without calling any at_exit
callbacks at all.
If you need to do something special for SIGINT, then trap SIGINT and call exit
(or not). Same for SIGTERM. Either with an run_at_exit_handlers
argument or a new exit!
method.
Here is an example to gracefully terminate on normal exit or SIGINT + immediate termination on a second SIGINT (Ctrl+C):
at_exit { do_some_cleanup() }
sigint = Atomic::Flag.new
Signal::INT.trap do
if sigint.test_and_set
puts "Type Ctrl+C again to force termination"
exit
else
exit run_at_exit_handlers: false
end
end
I'm in total agreement with @ysbaddaden: let's add default SIGINT
and SIGTERM
traps, and then add exit!
to allow exiting without running at_exit
hooks.
Most helpful comment
I'm in total agreement with @ysbaddaden: let's add default
SIGINT
andSIGTERM
traps, and then addexit!
to allow exiting without runningat_exit
hooks.