From 807db9be271aeb2f224bc9ff25c56ba5103e1b7f Mon Sep 17 00:00:00 2001 From: Chris Kuehl Date: Wed, 26 Aug 2015 09:05:54 -0700 Subject: [PATCH 1/5] Add process group support --- Makefile | 4 ++- dumb-init.c | 59 +++++++++++++++++++++++++++++++++----------- test | 12 ++++++--- tests/test-no-pgroup | 25 +++++++++++++++++++ tests/test-pgroup | 27 ++++++++++++++++++++ 5 files changed, 109 insertions(+), 18 deletions(-) create mode 100755 tests/test-no-pgroup create mode 100755 tests/test-pgroup diff --git a/Makefile b/Makefile index a126402..3f971a7 100644 --- a/Makefile +++ b/Makefile @@ -1,3 +1,5 @@ +CFLAGS=-std=gnu99 -static -Wall -Werror + DOCKER_RUN_TEST := docker run -v $(PWD):/mnt:ro DOCKER_DEB_TEST := sh -euxc ' \ apt-get update \ @@ -22,7 +24,7 @@ DOCKER_PYTHON_TEST := sh -uexc ' \ .PHONY: build build: - $(CC) -static -Wall -Werror -o dumb-init dumb-init.c + $(CC) $(CFLAGS) -o dumb-init dumb-init.c .PHONY: clean clean: clean-tox diff --git a/dumb-init.c b/dumb-init.c index 48bdee6..07f5525 100644 --- a/dumb-init.c +++ b/dumb-init.c @@ -5,9 +5,10 @@ * Usage: * ./dumb-init python -c 'while True: pass' * - * To get debug output on stderr, run with DUMB_INIT_DEBUG=1. + * To get debug output on stderr, run with DUMB_INIT_DEBUG=1 */ +#include #include #include #include @@ -16,18 +17,24 @@ #include #include +#define DEBUG(...) do { \ + if (debug) { \ + fprintf(stderr, __VA_ARGS__); \ + } \ +} while (0) + pid_t child = -1; char debug = 0; +char use_process_group = 1; void signal_handler(int signum) { - if (debug) - fprintf(stderr, "Received signal %d.\n", signum); + DEBUG("Received signal %d.\n", signum); if (child > 0) { - kill(child, signum); - - if (debug) - fprintf(stderr, "Forwarded signal to child.\n"); + kill(use_process_group ? -child : child, signum); + DEBUG("Forwarded signal to child.\n"); + } else { + DEBUG("Didn't forward signal, no child exists yet."); } } @@ -49,14 +56,18 @@ void print_help(char *argv[]) { "\n" "The proxy dies when your process dies, so it must not double-fork or do other\n" "weird things (this is basically a requirement for doing things sanely in\n" - "Docker anyway).\n", + "Docker anyway).\n" + "\n" + "By default, dumb-init starts a process group and kills all processes in it.\n" + "This is usually useful behavior, but if for some reason you wish to disable\n" + "it, run with DUMB_INIT_PROCESS_GROUP=0.\n", argv[0] ); } int main(int argc, char *argv[]) { int signum, exit_status, status = 0; - char *debug_env; + char *debug_env, *pgroup_env; if (argc < 2) { print_help(argv); @@ -64,9 +75,16 @@ int main(int argc, char *argv[]) { } debug_env = getenv("DUMB_INIT_DEBUG"); - if (debug_env && strcmp(debug_env, "1") == 0) + if (debug_env && strcmp(debug_env, "1") == 0) { debug = 1; + DEBUG("Running in debug mode.\n"); + } + pgroup_env = getenv("DUMB_INIT_PROCESS_GROUP"); + if (pgroup_env && strcmp(pgroup_env, "0") == 0) { + use_process_group = 0; + DEBUG("Not running in process group mode.\n"); + } /* register signal handlers */ for (signum = 1; signum < 32; signum++) { @@ -88,16 +106,29 @@ int main(int argc, char *argv[]) { } if (child == 0) { + if (use_process_group) { + pid_t result = setpgid(0, 0); + if (result != 0) { + fprintf( + stderr, + "Unable to create process group (errno=%d %s). Exiting.\n", + errno, + strerror(errno) + ); + exit(1); + } + DEBUG("Set process group ID of child to its own PID.\n"); + } + execvp(argv[1], &argv[1]); } else { - if (debug) - fprintf(stderr, "Child spawned with PID %d.\n", child); + DEBUG("Child spawned with PID %d.\n", child); + /* wait for child to exit */ waitpid(child, &status, 0); exit_status = WEXITSTATUS(status); - if (debug) - fprintf(stderr, "Child exited with status %d, goodbye.\n", exit_status); + DEBUG("Child exited with status %d, goodbye.\n", exit_status); return exit_status; } diff --git a/test b/test index 421b346..ce6785a 100755 --- a/test +++ b/test @@ -11,9 +11,15 @@ fi echo "Running with dumb-init at '$dumb_init_bin'" run_tests() { - ./test-proxies-signals "$dumb_init_bin" - ./test-exit-status "$dumb_init_bin" - ./test-help-message "$dumb_init_bin" + export DUMB_INIT_PROCESS_GROUP + for DUMB_INIT_PROCESS_GROUP in 0 1; do + ./test-proxies-signals "$dumb_init_bin" + ./test-exit-status "$dumb_init_bin" + ./test-help-message "$dumb_init_bin" + done + + ./test-pgroup "$dumb_init_bin" + ./test-no-pgroup "$dumb_init_bin" } cd tests diff --git a/tests/test-no-pgroup b/tests/test-no-pgroup new file mode 100755 index 0000000..8a2d4dd --- /dev/null +++ b/tests/test-no-pgroup @@ -0,0 +1,25 @@ +#!/bin/bash -euxm +# dumb-init should not proxy signals to a process group rooted at its child if +# asked not to. +dumb_init="$1" + +DUMB_INIT_PROCESS_GROUP=0 $dumb_init nohup sh -c "yes 'oh, hi' | tail & yes error | tail >&2" > /dev/null & +pid="$!" + +sleep 1 +shell_pid=$(pgrep -P "$pid") +child_pids=$(pgrep -P $shell_pid) + +kill -TERM "$pid" +sleep 1 + +# ensure all processes are still running +while read pid; do + if pgrep "$pid"; then + echo "Error: expected process $pid to still be alive, but it wasn't." + exit 1 + else + echo "Process $pid was still alive (as expected)." + fi +done <<< "$child_pids" +xargs kill -9 <<< "$child_pids" diff --git a/tests/test-pgroup b/tests/test-pgroup new file mode 100755 index 0000000..d6bfccb --- /dev/null +++ b/tests/test-pgroup @@ -0,0 +1,27 @@ +#!/bin/bash -eux +# dumb-init should proxy signals to a process group rooted at its child when +# requested. +dumb_init="$1" + +DUMB_INIT_PROCESS_GROUP=1 $dumb_init nohup sh -c "yes 'oh, hi' | tail & yes error | tail >&2" > /dev/null & +pid="$!" + +sleep 1 +shell_pid=$(pgrep -P "$pid") + +# ensure processes are running +child_count=$(pgrep -g "$shell_pid" | grep -v "$shell_pid" | wc -l) || true + +if [ "$child_count" -ne 4 ]; then + echo "Error: Expected 4 children, instead we had ${child_count}." + exit 1 +fi + +# ensure processes are dead after signal +kill -TERM "$pid" +child_count=$(pgrep -g "$shell_pid" | grep -v "$shell_pid" | wc -l) || true + +if [ "$child_count" -ne 0 ]; then + echo "Error: Expected 0 children, instead we had ${child_count}." + exit 1 +fi From d0e9a8d02a25e96df4e9027cac653054fce8ca8f Mon Sep 17 00:00:00 2001 From: Buck Golemon Date: Fri, 28 Aug 2015 18:21:51 -0700 Subject: [PATCH 2/5] buck review --- dumb-init.c | 2 +- test | 4 ++-- tests/test-help-message | 18 +++++++++--------- tests/test-no-pgroup | 25 ------------------------- tests/test-pgroup | 21 +++++++++++++-------- 5 files changed, 25 insertions(+), 45 deletions(-) delete mode 100755 tests/test-no-pgroup diff --git a/dumb-init.c b/dumb-init.c index 07f5525..6c50baa 100644 --- a/dumb-init.c +++ b/dumb-init.c @@ -128,7 +128,7 @@ int main(int argc, char *argv[]) { waitpid(child, &status, 0); exit_status = WEXITSTATUS(status); - DEBUG("Child exited with status %d, goodbye.\n", exit_status); + DEBUG("Child exited with status %d. Goodbye.\n", exit_status); return exit_status; } diff --git a/test b/test index ce6785a..46c2513 100755 --- a/test +++ b/test @@ -18,8 +18,8 @@ run_tests() { ./test-help-message "$dumb_init_bin" done - ./test-pgroup "$dumb_init_bin" - ./test-no-pgroup "$dumb_init_bin" + DUMB_INIT_PROCESS_GROUP=0 ./test-pgroup "$dumb_init_bin" 4 + DUMB_INIT_PROCESS_GROUP=1 ./test-pgroup "$dumb_init_bin" 0 } cd tests diff --git a/tests/test-help-message b/tests/test-help-message index b1c45a1..76d9424 100755 --- a/tests/test-help-message +++ b/tests/test-help-message @@ -6,15 +6,15 @@ dumb_init="$1" status=$($dumb_init > /dev/null 2>&1; echo $?) -if [ "$status" -ne 0 ]; then - msg=$($dumb_init 2>&1 || true) - msg_len=${#msg} - - if [ "$msg_len" -le 50 ]; then - echo "Error: Expected dumb-init with no arguments to print a useful message, but it was only ${msg_len} chars long." - exit 1 - fi -else +if [ "$status" -eq 0 ]; then echo "Error: Expected dumb-init with no arguments to return nonzero, but it returned ${status}." exit 1 fi + +msg=$($dumb_init 2>&1 || true) +msg_len=${#msg} + +if [ "$msg_len" -le 50 ]; then + echo "Error: Expected dumb-init with no arguments to print a useful message, but it was only ${msg_len} chars long." + exit 1 +fi diff --git a/tests/test-no-pgroup b/tests/test-no-pgroup deleted file mode 100755 index 8a2d4dd..0000000 --- a/tests/test-no-pgroup +++ /dev/null @@ -1,25 +0,0 @@ -#!/bin/bash -euxm -# dumb-init should not proxy signals to a process group rooted at its child if -# asked not to. -dumb_init="$1" - -DUMB_INIT_PROCESS_GROUP=0 $dumb_init nohup sh -c "yes 'oh, hi' | tail & yes error | tail >&2" > /dev/null & -pid="$!" - -sleep 1 -shell_pid=$(pgrep -P "$pid") -child_pids=$(pgrep -P $shell_pid) - -kill -TERM "$pid" -sleep 1 - -# ensure all processes are still running -while read pid; do - if pgrep "$pid"; then - echo "Error: expected process $pid to still be alive, but it wasn't." - exit 1 - else - echo "Process $pid was still alive (as expected)." - fi -done <<< "$child_pids" -xargs kill -9 <<< "$child_pids" diff --git a/tests/test-pgroup b/tests/test-pgroup index d6bfccb..e80e14d 100755 --- a/tests/test-pgroup +++ b/tests/test-pgroup @@ -2,26 +2,31 @@ # dumb-init should proxy signals to a process group rooted at its child when # requested. dumb_init="$1" +after_count="$2" -DUMB_INIT_PROCESS_GROUP=1 $dumb_init nohup sh -c "yes 'oh, hi' | tail & yes error | tail >&2" > /dev/null & +$dumb_init sh -c "yes 'oh, hi' | tail & yes error | tail >&2" & pid="$!" sleep 1 -shell_pid=$(pgrep -P "$pid") +pstree -p "$pid" +pids=$(pstree -p "$pid" | grep -Po '(\d+)' | grep -Po '\d+') # ensure processes are running -child_count=$(pgrep -g "$shell_pid" | grep -v "$shell_pid" | wc -l) || true +child_count=$(ps -o pid= $pids | wc -l) || true -if [ "$child_count" -ne 4 ]; then - echo "Error: Expected 4 children, instead we had ${child_count}." +if [ "$child_count" -ne 6 ]; then + echo "Error: Expected 6 children, instead we had ${child_count}." exit 1 fi # ensure processes are dead after signal kill -TERM "$pid" -child_count=$(pgrep -g "$shell_pid" | grep -v "$shell_pid" | wc -l) || true +sleep 1 +child_count=$(ps -o pid= $pids | wc -l) || true -if [ "$child_count" -ne 0 ]; then - echo "Error: Expected 0 children, instead we had ${child_count}." +if [ "$child_count" -ne "$after_count" ]; then + echo "Error: Expected $after_count children, instead we had ${child_count}." exit 1 fi + +xargs kill -9 <<< "$pids" From 508b8e29d2667244a385a7c9adce9a77eb1334af Mon Sep 17 00:00:00 2001 From: Chris Kuehl Date: Thu, 3 Sep 2015 15:01:34 -0700 Subject: [PATCH 3/5] Allow killing leftover processes to fail --- tests/test-pgroup | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test-pgroup b/tests/test-pgroup index e80e14d..99ac2a1 100755 --- a/tests/test-pgroup +++ b/tests/test-pgroup @@ -29,4 +29,5 @@ if [ "$child_count" -ne "$after_count" ]; then exit 1 fi -xargs kill -9 <<< "$pids" +echo 'Killing any leftover processes.' +xargs kill -9 <<< "$pids" || true From 497b7d0fcc20a6dac404ae04afef089691aad161 Mon Sep 17 00:00:00 2001 From: Chris Kuehl Date: Thu, 3 Sep 2015 15:53:14 -0700 Subject: [PATCH 4/5] print-signals: switch shebang to sh --- tests/lib/print-signals | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/lib/print-signals b/tests/lib/print-signals index cc4e6ce..55b7c08 100755 --- a/tests/lib/print-signals +++ b/tests/lib/print-signals @@ -1,4 +1,8 @@ -#!/bin/bash -eux +#!/bin/sh -eux +# XXX: We use /bin/sh instead of /bin/bash since some old versions of bash +# exhibit an issue where they seem to receive the same signal twice. +# With /bin/sh, this does not seem to happen. + # Print received signals into a file, one per line file="$1" From 6dc426613fe2f7888ffd174b8ae6879096fbe35e Mon Sep 17 00:00:00 2001 From: Chris Kuehl Date: Thu, 3 Sep 2015 16:02:41 -0700 Subject: [PATCH 5/5] Install psmisc for itests (needed for pstree) --- Dockerfile | 2 +- Makefile | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Dockerfile b/Dockerfile index 5f138b0..0c33d70 100644 --- a/Dockerfile +++ b/Dockerfile @@ -3,7 +3,7 @@ FROM debian:jessie MAINTAINER Chris Kuehl RUN DEBIAN_FRONTEND=noninteractive apt-get update && \ apt-get install -y --no-install-recommends \ - build-essential devscripts equivs && \ + build-essential devscripts equivs procps psmisc && \ apt-get clean WORKDIR /mnt diff --git a/Makefile b/Makefile index 3f971a7..6c48ff3 100644 --- a/Makefile +++ b/Makefile @@ -3,7 +3,7 @@ CFLAGS=-std=gnu99 -static -Wall -Werror DOCKER_RUN_TEST := docker run -v $(PWD):/mnt:ro DOCKER_DEB_TEST := sh -euxc ' \ apt-get update \ - && apt-get install -y --no-install-recommends procps \ + && apt-get install -y --no-install-recommends procps psmisc \ && (which timeout || apt-get install -y --no-install-recommends timeout) \ && dpkg -i /mnt/dist/*.deb \ && cd /mnt \ @@ -11,7 +11,7 @@ DOCKER_DEB_TEST := sh -euxc ' \ ' DOCKER_PYTHON_TEST := sh -uexc ' \ apt-get update \ - && apt-get install -y --no-install-recommends python-pip build-essential procps \ + && apt-get install -y --no-install-recommends python-pip build-essential procps psmisc \ && (which timeout || apt-get install -y --no-install-recommends timeout) \ && tmp=$$(mktemp -d) \ && cp -r /mnt/* "$$tmp" \