From 109019f917e6683955455cf1a21b19200b94a0ea Mon Sep 17 00:00:00 2001 From: David Wragg Date: Sun, 25 Oct 2015 17:25:32 +0000 Subject: [PATCH 1/2] Support for killing the whole child process group tini only kills the immediate child process. This means that if you do, for example, docker run krallin/ubuntu-tini sh -c 'sleep 10' and ctrl-C it, nothing happens: SIGINT is sent to the 'sh' process, but that shell won't react to it while it is waiting for the 'sleep' to finish. This change adds a -g option to put the child process of tini into a new process group, and sends signals to that group, so that every process in the group gets a signal. This corresponds more closely to what happens when you do ctrl-C etc. in a terminal: The signal is sent to the foreground process group. So if you try the example above with a container image that passes -g to tini, the SIGINT will be received by the 'sleep', and the container promptly exits. --- README.md | 18 ++++++++++++++++++ src/tini.c | 20 +++++++++++++++++--- tpl/README.md.in | 18 ++++++++++++++++++ 3 files changed, 53 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 5c07725..6b01464 100644 --- a/README.md +++ b/README.md @@ -81,6 +81,24 @@ as PID 1. and isn't registered as a subreaper. If you don't see a warning, you're fine.* +### Process group killing ### + +By default, Tini only kills its immediate child process. This can be +inconvenient if sending a signal to that process does have the desired +effect. For example, if you do + + docker run krallin/ubuntu-tini sh -c 'sleep 10' + +and ctrl-C it, nothing happens: SIGINT is sent to the 'sh' process, +but that shell won't react to it while it is waiting for the 'sleep' +to finish. + +With the `-g` option, Tini kills the child process group , so that +every process in the group gets the signal. This corresponds more +closely to what happens when you do ctrl-C etc. in a terminal: The +signal is sent to the foreground process group. + + More ---- diff --git a/src/tini.c b/src/tini.c index c307fe3..260ae0a 100644 --- a/src/tini.c +++ b/src/tini.c @@ -27,11 +27,11 @@ #ifdef PR_SET_CHILD_SUBREAPER #define HAS_SUBREAPER 1 -#define OPT_STRING "hsv" +#define OPT_STRING "hsvg" #define SUBREAPER_ENV_VAR "TINI_SUBREAPER" #else #define HAS_SUBREAPER 0 -#define OPT_STRING "hv" +#define OPT_STRING "hvg" #endif @@ -39,6 +39,7 @@ static int subreaper = 0; #endif static int verbosity = 1; +static int kill_process_group = 0; static struct timespec ts = { .tv_sec = 1, .tv_nsec = 0 }; @@ -68,6 +69,13 @@ int spawn(const sigset_t* const child_sigset_ptr, char* const argv[], int* const PRINT_FATAL("Setting child signal mask failed: '%s'", strerror(errno)); return 1; } + + // Put the child into a new process group + if (setpgid(0, 0) < 0) { + PRINT_FATAL("setpgid failed: '%s'", strerror(errno)); + return 1; + } + execvp(argv[0], argv); PRINT_FATAL("Executing child process '%s' failed: '%s'", argv[0], strerror(errno)); return 1; @@ -89,6 +97,7 @@ void print_usage(char* const name, FILE* const file) { fprintf(file, " -s: Register as a process subreaper (requires Linux >= 3.4).\n"); #endif fprintf(file, " -v: Generate more verbose output. Repeat up to 3 times.\n"); + fprintf(file, " -g: Send signals to the child's process group.\n"); fprintf(file, "\n"); } @@ -112,6 +121,11 @@ int parse_args(const int argc, char* const argv[], char* (**child_args_ptr_ptr)[ case 'v': verbosity++; break; + + case 'g': + kill_process_group++; + break; + case '?': print_usage(name, stderr); return 1; @@ -242,7 +256,7 @@ int wait_and_forward_signal(sigset_t const* const parent_sigset_ptr, pid_t const default: PRINT_DEBUG("Passing signal: '%s'", strsignal(sig.si_signo)); /* Forward anything else */ - if (kill(child_pid, sig.si_signo)) { + if (kill(kill_process_group ? -child_pid : child_pid, sig.si_signo)) { if (errno == ESRCH) { PRINT_WARNING("Child was dead when forwarding signal"); } else { diff --git a/tpl/README.md.in b/tpl/README.md.in index 4e451ce..9217667 100644 --- a/tpl/README.md.in +++ b/tpl/README.md.in @@ -81,6 +81,24 @@ as PID 1. and isn't registered as a subreaper. If you don't see a warning, you're fine.* +### Process group killing ### + +By default, Tini only kills its immediate child process. This can be +inconvenient if sending a signal to that process does have the desired +effect. For example, if you do + + docker run krallin/ubuntu-tini sh -c 'sleep 10' + +and ctrl-C it, nothing happens: SIGINT is sent to the 'sh' process, +but that shell won't react to it while it is waiting for the 'sleep' +to finish. + +With the `-g` option, Tini kills the child process group , so that +every process in the group gets the signal. This corresponds more +closely to what happens when you do ctrl-C etc. in a terminal: The +signal is sent to the foreground process group. + + More ---- From e7bae983c6468a3735a6de05cb6f35db98eb2905 Mon Sep 17 00:00:00 2001 From: Thomas Orozco Date: Sun, 25 Oct 2015 16:53:38 +0100 Subject: [PATCH 2/2] Add tests for process group support --- test/pgroup/stage_1.py | 18 ++++++++++++++++++ test/run_inner_tests.py | 26 +++++++++++++++++++++++++- 2 files changed, 43 insertions(+), 1 deletion(-) create mode 100755 test/pgroup/stage_1.py diff --git a/test/pgroup/stage_1.py b/test/pgroup/stage_1.py new file mode 100755 index 0000000..84eabde --- /dev/null +++ b/test/pgroup/stage_1.py @@ -0,0 +1,18 @@ +#!/usr/bin/env python +import os +import subprocess +import signal + + +def reset_sig_handler(): + signal.signal(signal.SIGUSR1, signal.SIG_DFL) + + +if __name__ == "__main__": + signal.signal(signal.SIGUSR1, signal.SIG_IGN) + p = subprocess.Popen( + ["sleep", "1000"], + preexec_fn=reset_sig_handler + ) + p.wait() + diff --git a/test/run_inner_tests.py b/test/run_inner_tests.py index 7cbef47..e332bf3 100755 --- a/test/run_inner_tests.py +++ b/test/run_inner_tests.py @@ -4,6 +4,20 @@ import os import sys import signal import subprocess +import time +import psutil + + +def busy_wait(condition_callable, timeout): + checks = 100 + increment = float(timeout) / checks + + for _ in xrange(checks): + if condition_callable(): + return + time.sleep(increment) + + assert False, "Condition was never met" def main(): @@ -47,7 +61,17 @@ def main(): sig = getattr(signal, signame) p.send_signal(sig) ret = p.wait() - assert ret == - sig, "Signals test failed!" + assert ret == -sig, "Signals test failed!" + + # Run the process group test + # This test has Tini spawn a process that ignores SIGUSR1 and spawns a child that doesn't (and waits on the child) + # We send SIGUSR1 to Tini, and expect the grand-child to terminate, then the child, and then Tini. + print "Running process group test" + p = subprocess.Popen([tini, '-g', '--', os.path.join(src, "test", "pgroup", "stage_1.py")], env=dict(os.environ, **env)) + + busy_wait(lambda: len(psutil.Process(p.pid).children(recursive=True)) == 2, 10) + p.send_signal(signal.SIGUSR1) + busy_wait(lambda: p.poll() is not None, 10) # Run failing test print "Running failing test"