From ee6d4a1dae3db5b9762b669bc490b70ebc0c2f3c Mon Sep 17 00:00:00 2001 From: Joachim Wiberg Date: Sun, 27 Sep 2026 18:40:44 +0200 Subject: [PATCH] service: keep a pending reload across a second conf reload A service whose condition goes into flux during a reload is paused with its reload still pending. If another reload was requested in the meantime, re-parsing its unchanged .conf file cleared the pending mark, so the service was resumed without ever being reloaded. Seen with sshd on Infix, where a configuration change that touched both landed as two reloads in a row and sshd kept its old listen addresses. The mark is only ever cleared once the change has been applied, so a mark that is still set when the file is parsed again means exactly that: not applied yet. Leave it alone. Signed-off-by: Joachim Wiberg --- src/service.c | 9 +++-- test/Makefile.am | 2 + test/reload-while-paused.sh | 76 +++++++++++++++++++++++++++++++++++++ 3 files changed, 84 insertions(+), 3 deletions(-) create mode 100755 test/reload-while-paused.sh diff --git a/src/service.c b/src/service.c index 36ddbfe0..59c47c30 100644 --- a/src/service.c +++ b/src/service.c @@ -2703,11 +2703,14 @@ svc_t *service_register(int type, char *cfg, struct rlimit rlimit[], char *file) if (cgroup) parse_cgroup(svc, cgroup); - /* New, recently modified or unchanged ... used on reload. */ + /* + * New or modified since the last reload. The mark is cleared when + * the change has been applied, on start or reload, so one that is + * still set here is a change that has not been applied yet, e.g. + * the service is paused waiting for a condition. Leave it. + */ if ((file && conf_changed(file)) || conf_changed(svc_getenv(svc)) || svc->args_dirty) svc_mark_dirty(svc); - else - svc_mark_clean(svc); svc_enable(svc); diff --git a/test/Makefile.am b/test/Makefile.am index b45e62a9..b943e1ef 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -43,6 +43,7 @@ EXTRA_DIST += conf-template.sh EXTRA_DIST += script-timeout.sh EXTRA_DIST += crashing.sh EXTRA_DIST += dep-chain-reload.sh +EXTRA_DIST += reload-while-paused.sh EXTRA_DIST += depserv.sh EXTRA_DIST += devmon.sh EXTRA_DIST += failing-sysv.sh @@ -109,6 +110,7 @@ TESTS += conf-template.sh TESTS += script-timeout.sh TESTS += crashing.sh TESTS += dep-chain-reload.sh +TESTS += reload-while-paused.sh TESTS += depserv.sh TESTS += devmon.sh TESTS += failing-sysv.sh diff --git a/test/reload-while-paused.sh b/test/reload-while-paused.sh new file mode 100755 index 00000000..d616be95 --- /dev/null +++ b/test/reload-while-paused.sh @@ -0,0 +1,76 @@ +#!/bin/sh +# A reload must not be lost on a service paused by a condition +# +# svc_s writes its PID file one second after a SIGHUP, so a service +# with is paused for that second whenever svc_s reloads. +# svc_x counts the SIGHUPs it gets. Both .conf files are touched and +# two reloads are requested back to back, so the second one arrives +# while svc_x is paused with its own reload still pending. svc_x must +# get exactly one SIGHUP and keep its PID. + +set -eu + +TEST_DIR=$(dirname "$0") + +test_teardown() +{ + say "Running test teardown." + run "rm -f $FINIT_RCSD/svc_s.conf $FINIT_RCSD/svc_x.conf /tmp/hup.sh /tmp/hup.log" +} + +pidof() +{ + texec initctl -j status "$1" | jq .pid +} + +test_setup() +{ + run "cat > /tmp/hup.sh" <> /tmp/hup.log' HUP +trap 'exit 0' TERM +while true; do + sleep 1 +done +EOF + run "cat > $FINIT_RCSD/svc_s.conf" < $FINIT_RCSD/svc_x.conf" < name:svc_x /bin/sh /tmp/hup.sh -- Paused while svc_s reloads +EOF +} + +# shellcheck source=/dev/null +. "$TEST_DIR/lib/setup.sh" + +sep "Configuration" +run "cat $FINIT_RCSD/svc_s.conf" +run "cat $FINIT_RCSD/svc_x.conf" + +say "Reload Finit to start both services" +run "initctl reload" +retry 'assert_status "svc_x" "running"' 10 1 + +pid_x=$(pidof svc_x) +say "svc_x PID before: $pid_x" + +sep "Touch both, reload twice" +run "rm -f /tmp/hup.log" +run "initctl touch svc_s.conf" +run "initctl touch svc_x.conf" +run "initctl reload" +run "initctl reload" + +say "Wait for services to settle" +retry 'assert_status "svc_x" "running"' 15 1 +run "initctl status" + +# shellcheck disable=SC2016 +retry 'assert "svc_x got its SIGHUP" "$(texec sh -c "cat /tmp/hup.log 2>/dev/null | wc -l")" -eq 1' 10 1 + +new_pid_x=$(pidof svc_x) +# shellcheck disable=SC2086 +assert "svc_x was not restarted" $new_pid_x -eq $pid_x + +return 0