From 4bb2c338592dcb43c6324ab2b1a3b4688336417e Mon Sep 17 00:00:00 2001 From: Joachim Wiberg Date: Fri, 13 Feb 2026 07:24:55 +0100 Subject: [PATCH] Dependents not restarted after SIGHUP reload of service When 'initctl reload' is called after marking a service in a dependency chain dirty, Finit fails to restart (unfreeze) affected services. This patch updates the pidfile plugin to watch for IN_ATTRIB changes, e.g. when a process uses utimensat() to update its pidfile, and adds service_step_all() at end of reload cycle to guarantee convergence after conditions are reasserted. Issue #476 Signed-off-by: Joachim Wiberg --- plugins/pidfile.c | 3 +- src/sm.c | 12 ++++ test/dep-chain-reload.sh | 122 +++++++++++++++++++++++++++++---------- 3 files changed, 104 insertions(+), 33 deletions(-) diff --git a/plugins/pidfile.c b/plugins/pidfile.c index c64e18ff..ae0fdeea 100644 --- a/plugins/pidfile.c +++ b/plugins/pidfile.c @@ -59,7 +59,7 @@ static int pidfile_add_path(struct iwatch *iw, char *path) } } - return iwatch_add(iw, path, IN_ONLYDIR | IN_CLOSE_WRITE); + return iwatch_add(iw, path, IN_ONLYDIR | IN_CLOSE_WRITE | IN_ATTRIB); } static void pidfile_update_conds(char *dir, char *name, uint32_t mask) @@ -276,6 +276,7 @@ static void pidfile_reconf(void *arg) if (cond_get(cond) == COND_ON) continue; + dbg("Reassert condition %s", cond); cond_set_path(cond_path(cond), COND_ON); } diff --git a/src/sm.c b/src/sm.c index 69f61c86..85510ff8 100644 --- a/src/sm.c +++ b/src/sm.c @@ -510,15 +510,27 @@ restart: /* Cleanup stale services */ svc_clean_dynamic(service_unregister); + dbg("Step all services waiting for reasserted conditions ..."); + service_step_all(SVC_TYPE_ANY); + dbg("Calling reconf hooks ..."); plugin_run_hooks(HOOK_SVC_RECONF); + dbg("Step all services waiting for reasserted conditions ..."); + service_step_all(SVC_TYPE_ANY); + dbg("Update configuration generation of device conditions ..."); devmon_reconf(); + dbg("Step all services waiting for reasserted conditions ..."); + service_step_all(SVC_TYPE_ANY); + dbg("Update configuration generation of unmodified non-native services ..."); service_notify_reconf(); + dbg("Step all services waiting for reasserted conditions ..."); + service_step_all(SVC_TYPE_ANY); + dbg("Reconfiguration done"); sm.state = SM_RUNNING_STATE; break; diff --git a/test/dep-chain-reload.sh b/test/dep-chain-reload.sh index 43111701..b5cfea49 100755 --- a/test/dep-chain-reload.sh +++ b/test/dep-chain-reload.sh @@ -1,26 +1,32 @@ #!/bin/sh # Verify transitive dependency chain during reload and crash # -# Three services in a chain: A → B → C, where B depends on pid/A -# and C depends on pid/B. B is placed in a sub-config file so -# we can use 'initctl touch' on it. B uses (with -# leading '!') so it does not support SIGHUP, causing a full -# stop/start cycle on 'initctl touch svc_b.conf' + reload. +# Four services in a chain: A → B → C → D, all using notify:pid. +# B and C are placed in separate sub-config files so we can use +# 'initctl touch' on them independently. B supports SIGHUP (reload), +# while C and D use the '!' condition prefix (noreload), matching +# a common pattern in real-world setups (e.g., FRR daemons on Infix). # -# Test 1 - touch + reload: -# After 'initctl touch svc_b.conf' + 'initctl reload': -# - A should be unaffected (same PID) -# - B is restarted (config was touched, noreload) -# - C must be restarted (transitive, depends on pid/B) +# Chain: A ← B ← C ← D +# +# Test 1 - touch C + reload (the "Infix" scenario): +# C is in the middle of the chain. After 'initctl touch svc_c.conf' +# + 'initctl reload': +# - A and B should be unaffected (same PID) +# - C is stopped/started (config was touched, noreload) +# - D must be restarted (reverse dep of C) # # Test 2 - crash (kill -9): -# When B is killed with SIGKILL the crash path (RUNNING → -# HALTED) bypasses STOPPING, where cond_clear() used to be -# the only call site. The pidfile plugin only watches for -# IN_CLOSE_WRITE, so neither the pidfile removal (IN_DELETE) -# nor a pidfile touch (IN_ATTRIB) triggers an inotify event. -# Without the fix in service_cleanup(), pid/B is never -# invalidated and C is never restarted. +# When B is killed with SIGKILL the crash path (RUNNING → HALTED) +# bypasses STOPPING. Without cond_clear() in service_cleanup(), +# pid/B is never invalidated and C, D are never restarted. +# +# Test 3 - touch B + reload: +# B supports SIGHUP (no '!' prefix). After 'initctl touch +# svc_b.conf' + 'initctl reload': +# - A should be unaffected (same PID) +# - B is SIGHUP'd (same PID, config was touched) +# - C and D must be restarted (transitive reverse deps) set -eu @@ -29,7 +35,7 @@ TEST_DIR=$(dirname "$0") test_teardown() { say "Running test teardown." - run "rm -f $FINIT_RCSD/svc_b.conf" + run "rm -f $FINIT_RCSD/svc_b.conf $FINIT_RCSD/svc_c.conf" } pidof() @@ -41,9 +47,14 @@ test_setup() { run "cat >> $FINIT_CONF" < name:svc_c serv -np -i svc_c -- Needs B +service log:stdout notify:pid name:svc_d serv -np -i svc_d -- Needs C +EOF + run "cat >> $FINIT_RCSD/svc_b.conf" < name:svc_b serv -np -i svc_b -- Needs A +EOF + run "cat >> $FINIT_RCSD/svc_c.conf" < name:svc_c serv -np -i svc_c -- Needs B EOF - run "echo 'service log:stdout notify:pid name:svc_b serv -np -i svc_b -- Needs A' > $FINIT_RCSD/svc_b.conf" } # shellcheck source=/dev/null @@ -52,31 +63,33 @@ EOF sep "Configuration" run "cat $FINIT_CONF" run "cat $FINIT_RCSD/svc_b.conf" +run "cat $FINIT_RCSD/svc_c.conf" say "Reload Finit to start all services" run "initctl reload" say "Wait for full chain to start" -retry 'assert_status "svc_c" "running"' 10 1 +retry 'assert_status "svc_d" "running"' 10 1 run "initctl status" run "initctl cond dump" # ―――――――――――――――――――――――――――――――――――――――――――――――――――――― -# Test 1: touch + reload +# Test 1: touch C (middle of chain, noreload) + reload # ―――――――――――――――――――――――――――――――――――――――――――――――――――――― -sep "Test 1: Touch B and global reload" +sep "Test 1: Touch C (middle) and global reload" pid_a=$(pidof svc_a) pid_b=$(pidof svc_b) pid_c=$(pidof svc_c) -say "PIDs before: A=$pid_a B=$pid_b C=$pid_c" +pid_d=$(pidof svc_d) +say "PIDs before: A=$pid_a B=$pid_b C=$pid_c D=$pid_d" -run "initctl touch svc_b.conf" +run "initctl touch svc_c.conf" run "initctl reload" say "Wait for chain to settle" -retry 'assert_status "svc_c" "running"' 15 1 +retry 'assert_status "svc_d" "running"' 15 1 run "initctl status" run "initctl cond dump" @@ -84,14 +97,17 @@ run "initctl cond dump" new_pid_a=$(pidof svc_a) new_pid_b=$(pidof svc_b) new_pid_c=$(pidof svc_c) -say "PIDs after: A=$new_pid_a B=$new_pid_b C=$new_pid_c" +new_pid_d=$(pidof svc_d) +say "PIDs after: A=$new_pid_a B=$new_pid_b C=$new_pid_c D=$new_pid_d" # shellcheck disable=SC2086 assert "A was not restarted" $new_pid_a -eq $pid_a # shellcheck disable=SC2086 -assert "B was restarted (touched)" $new_pid_b -ne $pid_b +assert "B was not restarted" $new_pid_b -eq $pid_b # shellcheck disable=SC2086 -assert "C was restarted (transitive dep)" $new_pid_c -ne $pid_c +assert "C was restarted (touched)" $new_pid_c -ne $pid_c +# shellcheck disable=SC2086 +assert "D was restarted (transitive dep)" $new_pid_d -ne $pid_d # ―――――――――――――――――――――――――――――――――――――――――――――――――――――― # Test 2: crash (kill -9), bypasses STOPPING @@ -100,23 +116,65 @@ sep "Test 2: Kill B with SIGKILL (bypasses STOPPING)" pid_b=$(pidof svc_b) pid_c=$(pidof svc_c) -say "PIDs before: B=$pid_b C=$pid_c" +pid_d=$(pidof svc_d) +say "PIDs before: B=$pid_b C=$pid_c D=$pid_d" run "kill -9 $pid_b" say "Wait for B to respawn and chain to settle" -retry 'assert_status "svc_c" "running"' 15 1 +retry 'assert_status "svc_d" "running"' 15 1 run "initctl status" run "initctl cond dump" new_pid_b=$(pidof svc_b) new_pid_c=$(pidof svc_c) -say "PIDs after: B=$new_pid_b C=$new_pid_c" +new_pid_d=$(pidof svc_d) +say "PIDs after: B=$new_pid_b C=$new_pid_c D=$new_pid_d" # shellcheck disable=SC2086 assert "B was restarted (crashed+respawn)" $new_pid_b -ne $pid_b # shellcheck disable=SC2086 assert "C was restarted (transitive dep)" $new_pid_c -ne $pid_c +# shellcheck disable=SC2086 +assert "D was restarted (transitive dep)" $new_pid_d -ne $pid_d + +# ―――――――――――――――――――――――――――――――――――――――――――――――――――――― +# Test 3: touch B (supports SIGHUP) + reload +# ―――――――――――――――――――――――――――――――――――――――――――――――――――――― +sep "Test 3: Touch B (SIGHUP) and global reload" + +pid_a=$(pidof svc_a) +pid_b=$(pidof svc_b) +pid_c=$(pidof svc_c) +pid_d=$(pidof svc_d) +say "PIDs before: A=$pid_a B=$pid_b C=$pid_c D=$pid_d" + +run "initctl debug" +run "initctl touch svc_b.conf" +run "initctl reload" +sleep 2 +run "initctl status" + +say "Wait for chain to settle" +retry 'assert_status "svc_d" "running"' 15 1 + +run "initctl status" +run "initctl cond dump" + +new_pid_a=$(pidof svc_a) +new_pid_b=$(pidof svc_b) +new_pid_c=$(pidof svc_c) +new_pid_d=$(pidof svc_d) +say "PIDs after: A=$new_pid_a B=$new_pid_b C=$new_pid_c D=$new_pid_d" + +# shellcheck disable=SC2086 +assert "A was not restarted" $new_pid_a -eq $pid_a +# shellcheck disable=SC2086 +assert "B was not restarted (SIGHUP)" $new_pid_b -eq $pid_b +# shellcheck disable=SC2086 +assert "C was restarted (transitive dep)" $new_pid_c -ne $pid_c +# shellcheck disable=SC2086 +assert "D was restarted (transitive dep)" $new_pid_d -ne $pid_d return 0