Crystal: at_exit is not triggered by SIGINT and SIGTERM

Created on 14 Jan 2020  路  17Comments  路  Source: crystal-lang/crystal

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

feature topicruntime

Most helpful comment

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.

All 17 comments

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.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

pbrusco picture pbrusco  路  3Comments

costajob picture costajob  路  3Comments

Sija picture Sija  路  3Comments

asterite picture asterite  路  3Comments

oprypin picture oprypin  路  3Comments