Skip to content

Commit

Permalink
Refactor signal handlers (#5730)
Browse files Browse the repository at this point in the history
Breaking change:
- Harness the SIGCHLD handling, which is required by Process#wait.
  Now we always handle SIGCHLD using SignalChildHandler. Trying to
  reset or ignore SIGCHLD will actually set the default handler,
  trying to trap SIGCHLD will wrap the custom handler instead.

Fixes:
- Synchronize some accesses using a Mutex and an Atomic to further
  enhance potential concurrency issues —probably impossible until
  parallelism is implemented.
- No longer closes the file descriptor at exit, which prevents an
  unhandled exception when receiving a signal while the program is
  exiting.
- Restore STDIN/OUT/ERR blocking state on exit.

Simplify implementation:
- Move private types (SignalHandler, SignalChildHandler) to the
  private Crystal namespace.
- Rename SignalHandler to Crystal::Signal.
- No more singleton classes.
- Using a Channel directly instead of a Concurrent::Future.
- Using macros under enum (it wasn't possible before).
- Introduce LibC::SIG_DFL and LibC::SIG_IGN definitions.
  • Loading branch information
ysbaddaden authored and RX14 committed Feb 21, 2018
1 parent 5911da0 commit 5a0e21f
Show file tree
Hide file tree
Showing 17 changed files with 304 additions and 285 deletions.
26 changes: 26 additions & 0 deletions spec/std/signal_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,31 @@ describe "Signal" do
Process.kill Signal::USR2, Process.pid
end

it "CHLD.reset sets default Crystal child handler" do
Signal::CHLD.reset
child = Process.new("true", shell: true)
child.wait # doesn't block forever
end

it "CHLD.ignore sets default Crystal child handler" do
Signal::CHLD.ignore
child = Process.new("true", shell: true)
child.wait # doesn't block forever
end

it "CHLD.trap is called after default Crystal child handler" do
called = false
child = nil

Signal::CHLD.trap do
called = true
Process.exists?(child.not_nil!.pid).should be_false
end

child = Process.new("true", shell: true)
child.not_nil!.wait # doesn't block forever
called.should be_true
end

# TODO: test Signal::X.reset
end
8 changes: 4 additions & 4 deletions src/compiler/crystal/command.cr
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ class Crystal::Command
Process.run(output_filename, args: run_args, input: Process::Redirect::Inherit, output: Process::Redirect::Inherit, error: Process::Redirect::Inherit) do |process|
# Ignore the signal so we don't exit the running process
# (the running process can still handle this signal)
Signal::INT.ignore # do
::Signal::INT.ignore # do
end
end
{$?, elapsed}
Expand All @@ -235,11 +235,11 @@ class Crystal::Command
exit status.exit_code
else
case status.exit_signal
when Signal::KILL
when ::Signal::KILL
STDERR.puts "Program was killed"
when Signal::SEGV
when ::Signal::SEGV
STDERR.puts "Program exited because of a segmentation fault (11)"
when Signal::INT
when ::Signal::INT
# OK, bubbled from the sub-program
else
STDERR.puts "Program received and didn't handle signal #{status.exit_signal} (#{status.exit_signal.value})"
Expand Down
68 changes: 0 additions & 68 deletions src/event/signal_child_handler.cr

This file was deleted.

95 changes: 0 additions & 95 deletions src/event/signal_handler.cr

This file was deleted.

2 changes: 1 addition & 1 deletion src/io.cr
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ abstract class IO
# reader.gets # => "hello"
# reader.gets # => "world"
# ```
def self.pipe(read_blocking = false, write_blocking = false)
def self.pipe(read_blocking = false, write_blocking = false) : {IO::FileDescriptor, IO::FileDescriptor}
pipe_fds = uninitialized StaticArray(LibC::Int, 2)
if LibC.pipe(pipe_fds) != 0
raise Errno.new("Could not create pipe")
Expand Down
11 changes: 5 additions & 6 deletions src/kernel.cr
Original file line number Diff line number Diff line change
Expand Up @@ -189,23 +189,22 @@ class Process
def self.after_fork_child_callbacks
@@after_fork_child_callbacks ||= [
->Scheduler.after_fork,
->Event::SignalHandler.after_fork,
->{ Event::SignalChildHandler.instance.after_fork },
->Crystal::Signal.after_fork,
->Crystal::SignalChildHandler.after_fork,
->Random::DEFAULT.new_seed,
] of -> Nil
end
end

{% unless flag?(:win32) %}
Signal.setup_default_handlers

at_exit { Event::SignalHandler.close }

# Background loop to cleanup unused fiber stacks.
spawn do
loop do
sleep 5
Fiber.stack_pool_collect
end
end

Signal.setup_default_handlers
LibExt.setup_sigfault_handler
{% end %}
4 changes: 4 additions & 0 deletions src/lib_c/aarch64-linux-gnu/c/signal.cr
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ lib LibC
SIGSTKFLT = 16
SIGUNUSED = 31

alias SighandlerT = Int ->
SIG_DFL = SighandlerT.new(Pointer(Void).new(0_u64), Pointer(Void).null)
SIG_IGN = SighandlerT.new(Pointer(Void).new(1_u64), Pointer(Void).null)

fun kill(pid : PidT, sig : Int) : Int
fun signal(sig : Int, handler : Int -> Void) : Int -> Void
end
4 changes: 4 additions & 0 deletions src/lib_c/amd64-unknown-openbsd/c/signal.cr
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ lib LibC
SIGINFO = 29
SIGWINCH = 28

alias SighandlerT = Int ->
SIG_DFL = SighandlerT.new(Pointer(Void).new(0_u64), Pointer(Void).null)
SIG_IGN = SighandlerT.new(Pointer(Void).new(1_u64), Pointer(Void).null)

fun kill(x0 : PidT, x1 : Int) : Int
fun signal(x0 : Int, x1 : Int -> Void) : Int -> Void
end
4 changes: 4 additions & 0 deletions src/lib_c/arm-linux-gnueabihf/c/signal.cr
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ lib LibC
SIGSTKFLT = 16
SIGUNUSED = 31

alias SighandlerT = Int ->
SIG_DFL = SighandlerT.new(Pointer(Void).new(0_u64), Pointer(Void).null)
SIG_IGN = SighandlerT.new(Pointer(Void).new(1_u64), Pointer(Void).null)

fun kill(pid : PidT, sig : Int) : Int
fun signal(sig : Int, handler : Int -> Void) : Int -> Void
end
4 changes: 4 additions & 0 deletions src/lib_c/i686-linux-gnu/c/signal.cr
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ lib LibC
SIGSTKFLT = 16
SIGUNUSED = 31

alias SighandlerT = Int ->
SIG_DFL = SighandlerT.new(Pointer(Void).new(0_u64), Pointer(Void).null)
SIG_IGN = SighandlerT.new(Pointer(Void).new(1_u64), Pointer(Void).null)

fun kill(pid : PidT, sig : Int) : Int
fun signal(sig : Int, handler : Int -> Void) : Int -> Void
end
4 changes: 4 additions & 0 deletions src/lib_c/i686-linux-musl/c/signal.cr
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ lib LibC
SIGSTKFLT = 16
SIGUNUSED = LibC::SIGSYS

alias SighandlerT = Int ->
SIG_DFL = SighandlerT.new(Pointer(Void).new(0_u64), Pointer(Void).null)
SIG_IGN = SighandlerT.new(Pointer(Void).new(1_u64), Pointer(Void).null)

fun kill(x0 : PidT, x1 : Int) : Int
fun signal(x0 : Int, x1 : Int -> Void) : Int -> Void
end
4 changes: 4 additions & 0 deletions src/lib_c/x86_64-linux-gnu/c/signal.cr
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ lib LibC
SIGSTKFLT = 16
SIGUNUSED = 31

alias SighandlerT = Int ->
SIG_DFL = SighandlerT.new(Pointer(Void).new(0_u64), Pointer(Void).null)
SIG_IGN = SighandlerT.new(Pointer(Void).new(1_u64), Pointer(Void).null)

fun kill(pid : PidT, sig : Int) : Int
fun signal(sig : Int, handler : Int -> Void) : Int -> Void
end
4 changes: 4 additions & 0 deletions src/lib_c/x86_64-linux-musl/c/signal.cr
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ lib LibC
SIGSTKFLT = 16
SIGUNUSED = LibC::SIGSYS

alias SighandlerT = Int ->
SIG_DFL = SighandlerT.new(Pointer(Void).new(0_u64), Pointer(Void).null)
SIG_IGN = SighandlerT.new(Pointer(Void).new(1_u64), Pointer(Void).null)

fun kill(x0 : PidT, x1 : Int) : Int
fun signal(x0 : Int, x1 : Int -> Void) : Int -> Void
end
4 changes: 4 additions & 0 deletions src/lib_c/x86_64-macosx-darwin/c/signal.cr
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ lib LibC
SIGINFO = 29
SIGWINCH = 28

alias SighandlerT = Int ->
SIG_DFL = SighandlerT.new(Pointer(Void).new(0_u64), Pointer(Void).null)
SIG_IGN = SighandlerT.new(Pointer(Void).new(1_u64), Pointer(Void).null)

fun kill(x0 : PidT, x1 : Int) : Int
fun signal(x0 : Int, x1 : Int -> Void) : Int -> Void
end
4 changes: 4 additions & 0 deletions src/lib_c/x86_64-portbld-freebsd/c/signal.cr
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ lib LibC
SIGINFO = 29
SIGWINCH = 28

alias SighandlerT = Int ->
SIG_DFL = SighandlerT.new(Pointer(Void).new(0_u64), Pointer(Void).null)
SIG_IGN = SighandlerT.new(Pointer(Void).new(1_u64), Pointer(Void).null)

fun kill(x0 : PidT, x1 : Int) : Int
fun signal(x0 : Int, x1 : Int -> Void) : Int -> Void
end
Loading

0 comments on commit 5a0e21f

Please sign in to comment.