From bb80d7c295cc3704744be7473045008cf8ba6a8d Mon Sep 17 00:00:00 2001 From: august-agent <269404572+august-agent@users.noreply.github.com> Date: Mon, 7 Sep 2026 11:23:15 +1200 Subject: [PATCH] fix(lock): reject zombie process owners --- bin/fm-session-lock-lib.sh | 20 ++++++++++++++++++- bin/fm-wake-lib.sh | 23 +++++++++++++++++++++- tests/fm-claude-stop-autoarm.test.sh | 29 ++++++++++++++++++++++++++++ tests/fm-watcher-lock.test.sh | 25 ++++++++++++++++++++++++ 4 files changed, 95 insertions(+), 2 deletions(-) diff --git a/bin/fm-session-lock-lib.sh b/bin/fm-session-lock-lib.sh index c2a117b0b84..2ab29135745 100644 --- a/bin/fm-session-lock-lib.sh +++ b/bin/fm-session-lock-lib.sh @@ -145,10 +145,28 @@ EOF printf '%s\n' "$outermost" } -# True if $1 is a live process that looks like a verified harness. +fm_harness_pid_zombie() { # + local pid=$1 proc_root stat_line state + local -a stat_fields + case "$pid" in ''|*[!0-9]*) return 1 ;; esac + proc_root=${FM_PROC_ROOT_OVERRIDE:-/proc} + if [ -r "$proc_root/$pid/stat" ]; then + stat_line=$(cat "$proc_root/$pid/stat" 2>/dev/null) || return 1 + read -r -a stat_fields <<< "${stat_line##*)}" + [ "${stat_fields[0]:-}" = Z ] + return + fi + state=$(ps -o stat= -p "$pid" 2>/dev/null) || return 1 + state=${state#"${state%%[![:space:]]*}"} + case "$state" in Z*) return 0 ;; esac + return 1 +} + +# True if $1 is a live, non-zombie process that looks like a verified harness. fm_harness_pid_alive() { local pid=$1 comm args kill -0 "$pid" 2>/dev/null || return 1 + fm_harness_pid_zombie "$pid" && return 1 comm=$(ps -o comm= -p "$pid" 2>/dev/null) || return 1 args=$(ps -o args= -p "$pid" 2>/dev/null) fm_harness_process_matches "$comm" "$args" diff --git a/bin/fm-wake-lib.sh b/bin/fm-wake-lib.sh index 17d55afdcea..1261f2c3d87 100755 --- a/bin/fm-wake-lib.sh +++ b/bin/fm-wake-lib.sh @@ -45,12 +45,33 @@ fm_current_pid() { # [output-variable] fi } +fm_pid_zombie() { # + local pid=$1 proc_root stat_line state + local -a stat_fields + case "$pid" in + ''|*[!0-9]*) return 1 ;; + esac + proc_root=${FM_PROC_ROOT_OVERRIDE:-/proc} + if [ -r "$proc_root/$pid/stat" ]; then + stat_line=$(cat "$proc_root/$pid/stat" 2>/dev/null) || return 1 + read -r -a stat_fields <<< "${stat_line##*)}" + [ "${stat_fields[0]:-}" = Z ] + return + fi + state=$(ps -o stat= -p "$pid" 2>/dev/null) || return 1 + state=${state#"${state%%[![:space:]]*}"} + case "$state" in Z*) return 0 ;; esac + return 1 +} + fm_pid_alive() { local pid=$1 case "$pid" in ''|*[!0-9]*) return 1 ;; esac - kill -0 "$pid" 2>/dev/null + kill -0 "$pid" 2>/dev/null || return 1 + fm_pid_zombie "$pid" && return 1 + return 0 } fm_pid_identity() { diff --git a/tests/fm-claude-stop-autoarm.test.sh b/tests/fm-claude-stop-autoarm.test.sh index 6917d21aac7..7d8fc4f0d58 100755 --- a/tests/fm-claude-stop-autoarm.test.sh +++ b/tests/fm-claude-stop-autoarm.test.sh @@ -268,6 +268,34 @@ test_reclaims_stale_session_lock_before_arming() { pass "auto-arm: a demonstrably dead recorded session owner is reclaimed through fm-lock.sh before arming" } +test_reclaims_zombie_session_lock_before_arming() { + local dir fakeproc owner out status expected_owner actual_owner + dir=$(make_primary_dir "$TMP_ROOT/zombie-session-lock") + fakeproc="$dir/proc" + : > "$dir/state/task.meta" + write_arm_fixture "$dir" actionable + "$FAKE_CLAUDE" -c 'sleep 60; :' & + owner=$! + printf '%s\n' "$owner" > "$dir/state/.lock" + mkdir -p "$fakeproc/$owner" + printf '%s (claude) Z 1\n' "$owner" > "$fakeproc/$owner/stat" + out=$(printf '%s\n' '{"session_id":"zombie"}' \ + | FM_PROC_ROOT_OVERRIDE="$fakeproc" FM_HOME="$dir" "$FAKE_CLAUDE" -c ' + printf "%s\n" "$$" > "$FM_HOME/state/expected-owner" + "$FM_HOME/bin/fm-claude-stop-autoarm.sh" + ' 2>&1); status=$? + expected_owner=$(cat "$dir/state/expected-owner") + actual_owner=$(cat "$dir/state/.lock") + kill "$owner" 2>/dev/null || true + wait "$owner" 2>/dev/null || true + expect_code 2 "$status" "a zombie recorded session owner must be reclaimed before the actionable rewake" + [ "$actual_owner" = "$expected_owner" ] \ + || fail "zombie session lock was not claimed by the current harness: expected $expected_owner, got $actual_owner" + [ -e "$dir/state/arm-ran" ] || fail "hook did not arm after reclaiming the zombie session lock" + [ "$(epoch_outcome "$dir")" = rewake ] || fail "zombie-lock recovery must record outcome=rewake" + pass "auto-arm: a zombie recorded session owner is reclaimed through fm-lock.sh before arming" +} + test_inert_when_lock_held_by_other_harness() { local dir other out status owner_after dir=$(make_primary_dir "$TMP_ROOT/other-lock") @@ -1166,6 +1194,7 @@ test_fm_lock_status_still_works_with_shared_lib() { test_inert_in_child_worktree test_inert_without_session_lock test_reclaims_stale_session_lock_before_arming +test_reclaims_zombie_session_lock_before_arming test_inert_when_lock_held_by_other_harness test_inert_when_afk test_stale_lock_recovery_preserves_afk_and_need_gates diff --git a/tests/fm-watcher-lock.test.sh b/tests/fm-watcher-lock.test.sh index 77fd4fbcca3..e9f9c704921 100755 --- a/tests/fm-watcher-lock.test.sh +++ b/tests/fm-watcher-lock.test.sh @@ -237,6 +237,30 @@ test_lock_steals_dead_pid_lock() { pass "dead-pid stale lock is reclaimed by a single acquirer" } +test_lock_steals_zombie_pid_lock() { + local dir state lockdir fakeproc owner rc newpid + dir=$(make_case lock-zombie-steal) + state="$dir/state" + lockdir="$state/.contend.lock" + fakeproc="$dir/proc" + sleep 300 & + owner=$! + mkdir -p "$fakeproc/$owner" "$lockdir" + printf '%s (zombie fixture) Z 1\n' "$owner" > "$fakeproc/$owner/stat" + printf '%s\n' "$owner" > "$lockdir/pid" + rc=0 + newpid=$(FM_PROC_ROOT_OVERRIDE="$fakeproc" FM_STATE_OVERRIDE="$state" bash -c ' + . "$1" + if fm_lock_try_acquire "$2"; then cat "$2/pid"; else exit 7; fi + ' _ "$LIB" "$lockdir") || rc=$? + kill "$owner" 2>/dev/null || true + wait "$owner" 2>/dev/null || true + [ "$rc" -eq 0 ] || fail "acquirer failed to steal a zombie-pid stale lock (rc=$rc)" + [ "$newpid" != "$owner" ] || fail "stale zombie-pid lock was not replaced (still $owner)" + [ -n "$newpid" ] || fail "reclaimed zombie lock has no pid recorded" + pass "zombie-pid stale lock is reclaimed by a single acquirer" +} + test_lock_stale_steal_single_winner_under_concurrency() { local dir state lockdir dead marker i pids pid wins dir=$(make_case lock-stale-concurrency) @@ -1112,6 +1136,7 @@ test_live_stale_watch_lock_is_actionable test_guard_warnings test_lock_single_winner_under_concurrency test_lock_steals_dead_pid_lock +test_lock_steals_zombie_pid_lock test_lock_stale_steal_single_winner_under_concurrency test_lock_live_steal_mutex_is_not_reclaimed test_lock_does_not_steal_live_lock