diff --git a/README.md b/README.md index 31ed956..c38850e 100644 --- a/README.md +++ b/README.md @@ -116,11 +116,20 @@ different stop signal in order to do graceful cleanup. For example, to rewrite the signal SIGTERM (number 15) to SIGQUIT (number 3), just add `--rewrite 15:3` on the command line. -One caveat with this feature: for job control signals (SIGTSTP, SIGTTIN, -SIGTTOU), dumb-init will always suspend itself after receiving the signal, even -if you rewrite it to something else. Additionally, if in setsid mode, dumb-init -will always forward SIGSTOP instead, since the original signals have no effect -unless the child has handlers for them. + +#### Signal rewriting special case + +When running in setsid mode, it is not sufficient to forward +`SIGTSTP`/`SIGTTIN`/`SIGTTOU` in most cases, since if the process has not added +a custom signal handler for these signals, then the kernel will not apply +default signal handling behavior (which would be suspending the process) since +it is a member of an orphaned process group. For this reason, we set default +rewrites to `SIGSTOP` from those three signals. You can opt out of this +behavior by rewriting the signals back to their original values, if desired. + +One caveat with this feature: for job control signals (`SIGTSTP`, `SIGTTIN`, +`SIGTTOU`), dumb-init will always suspend itself after receiving the signal, +even if you rewrite it to something else. ## Installing inside Docker containers diff --git a/dumb-init.c b/dumb-init.c index b4a3685..f0069e7 100644 --- a/dumb-init.c +++ b/dumb-init.c @@ -75,19 +75,6 @@ void forward_signal(int signum) { * The libc manual is useful: * https://www.gnu.org/software/libc/manual/html_node/Job-Control-Signals.html * - * When running in setsid mode, however, it is not sufficient to forward - * SIGTSTP/SIGTTIN/SIGTTOU in most cases. If the process has not added a custom - * signal handler for these signals, then the kernel will not apply default - * signal handling behavior (which would be suspending the process) since it is - * a member of an orphaned process group. - * - * Sadly this doesn't appear to be well documented except in the kernel itself: - * https://github.com/torvalds/linux/blob/v4.2/kernel/signal.c#L2296-L2299 - * - * Forwarding SIGSTOP instead is effective, though not ideal; unlike SIGTSTP, - * SIGSTOP cannot be caught, and so it doesn't allow processes a change to - * clean up before suspending. In non-setsid mode, we proxy the original signal - * instead of SIGSTOP for this reason. */ void handle_signal(int signum) { DEBUG("Received signal %d.\n", signum); @@ -110,23 +97,12 @@ void handle_signal(int signum) { exit(exit_status); } } - } else if ( - signum == SIGTSTP || // tty: background yourself - signum == SIGTTIN || // tty: stop reading - signum == SIGTTOU // tty: stop writing - ) { - if (use_setsid) { - DEBUG("Running in setsid mode, so forwarding SIGSTOP instead.\n"); - forward_signal(SIGSTOP); - } else { - DEBUG("Not running in setsid mode, so forwarding the original signal (%d).\n", signum); - forward_signal(signum); - } - - DEBUG("Suspending self due to TTY signal.\n"); - kill(getpid(), SIGSTOP); } else { forward_signal(signum); + if (signum == SIGTSTP || signum == SIGTTOU || signum == SIGTTIN) { + DEBUG("Suspending self due to TTY signal.\n"); + kill(getpid(), SIGSTOP); + } } } @@ -178,6 +154,11 @@ void parse_rewrite_signum(char *arg) { } } +void set_rewrite_to_sigstop_if_not_defined(int signum) { + if (signal_rewrite[signum] == 0) + signal_rewrite[signum] = SIGSTOP; +} + char **parse_command(int argc, char *argv[]) { int opt; struct option long_options[] = { @@ -231,6 +212,12 @@ char **parse_command(int argc, char *argv[]) { DEBUG("Not running in setsid mode.\n"); } + if (use_setsid) { + set_rewrite_to_sigstop_if_not_defined(SIGTSTP); + set_rewrite_to_sigstop_if_not_defined(SIGTTOU); + set_rewrite_to_sigstop_if_not_defined(SIGTTIN); + } + return &argv[optind]; } diff --git a/tests/proxies_signals_test.py b/tests/proxies_signals_test.py index 57955d2..79c691b 100644 --- a/tests/proxies_signals_test.py +++ b/tests/proxies_signals_test.py @@ -11,6 +11,7 @@ from tests.lib.testing import NORMAL_SIGNALS from tests.lib.testing import pid_tree +from tests.lib.testing import process_state @contextmanager @@ -70,6 +71,7 @@ def _rewrite_map_to_args(rewrite_map): [signal.SIGINT, signal.SIGQUIT, signal.SIGCONT, signal.SIGTERM], ), + # Lowest possible and highest possible signals. ( {1: 31, 31: 1}, [1, 31], @@ -83,3 +85,27 @@ def test_proxies_signals_with_rewrite(rewrite_map, sequence, expected): for send, expect_receive in zip(sequence, expected): proc.send_signal(send) assert proc.stdout.readline() == '{0}\n'.format(expect_receive).encode('ascii') + + +@pytest.mark.usefixtures('both_debug_modes', 'setsid_enabled') +def test_default_rewrites_can_be_overriden_with_setsid_enabled(): + """In setsid mode, dumb-init should allow overwriting the default + rewrites (but still suspend itself). + """ + rewrite_map = { + signal.SIGTTIN: signal.SIGTERM, + signal.SIGTTOU: signal.SIGINT, + signal.SIGTSTP: signal.SIGHUP, + } + with _print_signals(_rewrite_map_to_args(rewrite_map)) as proc: + for send, expect_receive in rewrite_map.items(): + assert process_state(proc.pid) in ['running', 'sleeping'] + proc.send_signal(send) + + assert proc.stdout.readline() == '{0}\n'.format(expect_receive).encode('ascii') + os.waitpid(proc.pid, os.WUNTRACED) + assert process_state(proc.pid) == 'stopped' + + proc.send_signal(signal.SIGCONT) + assert proc.stdout.readline() == '{0}\n'.format(signal.SIGCONT).encode('ascii') + assert process_state(proc.pid) in ['running', 'sleeping']