diff --git a/test/Makefile.am b/test/Makefile.am index 6e5ddfce..da6eb905 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -41,6 +41,7 @@ EXTRA_DIST += global-envs.sh EXTRA_DIST += initctl-status-subset.sh EXTRA_DIST += notify.sh EXTRA_DIST += pidfile.sh +EXTRA_DIST += stale-pidfile.sh EXTRA_DIST += pre-post-serv.sh EXTRA_DIST += pre-fail.sh EXTRA_DIST += process-depends.sh @@ -84,6 +85,7 @@ TESTS += global-envs.sh TESTS += initctl-status-subset.sh TESTS += notify.sh TESTS += pidfile.sh +TESTS += stale-pidfile.sh TESTS += pre-post-serv.sh TESTS += pre-fail.sh TESTS += process-depends.sh diff --git a/test/src/serv.c b/test/src/serv.c index 8129efb1..16d35bb1 100644 --- a/test/src/serv.c +++ b/test/src/serv.c @@ -34,6 +34,7 @@ volatile sig_atomic_t reloading = 1; volatile sig_atomic_t running = 1; static char *ident = PROGNM; static char fn[80]; +static int exclusive = 0; /* -x: refuse to start if pidfile exists (dbus-style) */ static void verify_env(char *arg) { @@ -122,7 +123,7 @@ static void writefn(char *fn, int val) static int checkfn(char *fn) { - return !access(fn, R_OK); + return !access(fn, F_OK); } static void mine(char *fn) @@ -133,12 +134,18 @@ static void mine(char *fn) static void pidfile(char *pidfn) { + static int once = 0; + if (!pidfn) { if (fn[0] == 0) snprintf(fn, sizeof(fn), "%s%s.pid", _PATH_VARRUN, ident); pidfn = fn; } + if (exclusive && !once && checkfn(pidfn)) + errx(EX_SOFTWARE, "PID file %s exists, refusing to start", pidfn); + once = 1; + if (!checkfn(pidfn)) { pid_t pid; @@ -148,7 +155,7 @@ static void pidfile(char *pidfn) atexit(cleanup); } else { inf("Touching PID file %s", pidfn); - utimensat(0, fn, NULL, 0); + utimensat(AT_FDCWD, pidfn, NULL, 0); } } @@ -182,6 +189,7 @@ static int usage(int rc) " -p Create PID file despite running in foreground\n" " -P FILE Create PID file using FILE\n" " -r SVC Call initctl to restart service SVC (self)\n" + " -x Refuse to start if PID file already exists (dbus-style)\n" "\n" "By default this program daemonizes itself to the background, and,\n" "when it's done setting up its signal handler(s), creates a PID file\n" @@ -212,7 +220,7 @@ int main(int argc, char *argv[]) char cmd[80]; int c; - while ((c = getopt(argc, argv, "cCe:E:f:F:hi:nN:pP:r:")) != EOF) { + while ((c = getopt(argc, argv, "cCe:E:f:F:hi:nN:pP:r:x")) != EOF) { switch (c) { case 'c': do_crash = 1; @@ -249,12 +257,17 @@ int main(int argc, char *argv[]) break; case 'P': pidfn = optarg; + if ((size_t)snprintf(fn, sizeof(fn), "%s", optarg) >= sizeof(fn)) + errx(EX_USAGE, "-P path too long (max %zu)", sizeof(fn) - 1); do_pidfile++; break; case 'r': snprintf(cmd, sizeof(cmd), "initctl restart %s", optarg); do_restart = 1; break; + case 'x': + exclusive = 1; + break; default: return usage(1); } diff --git a/test/stale-pidfile.sh b/test/stale-pidfile.sh new file mode 100755 index 00000000..07b4be47 --- /dev/null +++ b/test/stale-pidfile.sh @@ -0,0 +1,38 @@ +#!/bin/sh +# Regression: Finit must remove a daemon-owned (pid:!) stale pidfile +# left behind by an unclean exit (SIGKILL), so the next instance can +# start. Simulates the dbus-daemon pattern: 'serv -x' refuses to +# start if its pidfile already exists. + +set -eu + +TEST_DIR=$(dirname "$0") +PIDFN="/run/serv.pid" + +test_teardown() +{ + say "Running test teardown." + run "rm -f $FINIT_CONF $PIDFN" +} + +# shellcheck source=/dev/null +. "$TEST_DIR/lib/setup.sh" + +say "Add service stanza '$FINIT_CONF' with pid:!$PIDFN" +run "echo 'service pid:!$PIDFN serv -np -P $PIDFN -x' > $FINIT_CONF" +run "initctl reload" + +retry "assert_num_children 1 serv" +assert_is_pidfile "serv" "$PIDFN" + +PID=$(texec cat "$PIDFN") +say "SIGKILL serv ($PID) -- leaves stale pidfile" +run "kill -9 $PID" + +# Without the fix, 'serv -x' refuses to start on each retry until +# Finit hits restart_max and marks the service crashed. With the fix +# Finit removes the stale pidfile and the next instance comes up. +retry "assert_pidiff serv $PID" +retry "assert_num_children 1 serv" + +sep