From 7a042c3bf2280e724041da63bc489f9ef6abcd8f Mon Sep 17 00:00:00 2001 From: SchokoShake <28960930+SchokoShake@users.noreply.github.com> Date: Thu, 17 Sep 2026 14:28:42 +0200 Subject: [PATCH 1/3] fix(bin): stop lock waits in a removed home from spinning forever fm_lock_try_acquire read a lock it could not create as stale even when no lock existed at all, then tried to steal it through "$lock.steal", which failed the same way one level deeper, without bound. Any caller whose lock directory had been removed recursed like that, forking at every level and burning a CPU core for minutes. It now refuses an absent lock outright; recovering an existing stale lock is unchanged. fm_lock_acquire_wait still waited forever for a lock whose directory was gone, and now fails instead. Most callers already handle a failed wait, and the rest act only on the directory that no longer exists. fm_wake_append stops at that failure, so the deferred startup-network worker, which outlives the session start that launched it and waits on its publication lock and the wake queue, returns once its home is removed. tests/fm-startup-network.test.sh left five such workers behind on every run, once its fixture homes were removed at exit. On a 4-vCPU CI runner they slowed every later suite in the shard two to three times, which turned load-sensitive cases in tests/fm-pi-watch-extension.test.sh into a deterministic failure whenever a shard ran that suite after this one. tests/fm-watcher-lock.test.sh pins both lock calls refusing a removed directory without a steal chain, and tests/fm-startup-network.test.sh pins a worker exiting when its home is removed while it waits on its publication lock or while it delivers its wake; each fails on the previous code. --- bin/fm-startup-network.sh | 20 +++--- bin/fm-wake-lib.sh | 20 +++++- tests/fm-startup-network.test.sh | 107 +++++++++++++++++++++++++++++++ tests/fm-watcher-lock.test.sh | 30 +++++++++ 4 files changed, 167 insertions(+), 10 deletions(-) diff --git a/bin/fm-startup-network.sh b/bin/fm-startup-network.sh index 3cc9097b739..04ee0a61df2 100755 --- a/bin/fm-startup-network.sh +++ b/bin/fm-startup-network.sh @@ -200,7 +200,7 @@ cmd_start() { # return 1 fi - fm_lock_acquire_wait "$PUBLISH_LOCK" + fm_lock_acquire_wait "$PUBLISH_LOCK" || return 1 if [ "$(status_get state)" = running ] && worker_alive \ && { [ "$locked" != 1 ] || [ "$(status_get lock_pid)" = "$lock_pid" ]; }; then # A worker from this or a previous session is still going. Starting a second @@ -299,7 +299,7 @@ await_delivery() { # limit=$(( $(delivery_budget) * 10 )) while [ "$waited" -lt "$limit" ]; do claim_live=0 - fm_lock_acquire_wait "$PUBLISH_LOCK" + fm_lock_acquire_wait "$PUBLISH_LOCK" || return 0 if [ "$(status_get generation)" != "$generation" ]; then fm_lock_release "$PUBLISH_LOCK" return 0 @@ -332,7 +332,7 @@ EOF sleep 0.1 waited=$((waited + 1)) done - fm_lock_acquire_wait "$PUBLISH_LOCK" + fm_lock_acquire_wait "$PUBLISH_LOCK" || return 0 if [ "$(status_get generation)" != "$generation" ] || [ -f "$DELIVERED_FILE" ]; then fm_lock_release "$PUBLISH_LOCK" return 0 @@ -345,7 +345,7 @@ EOF publish() { # local generation=$1 state=$2 phases=$3 locked=$4 started=$5 rc=$6 out=$7 timings=${8:-} report_published=1 - fm_lock_acquire_wait "$PUBLISH_LOCK" + fm_lock_acquire_wait "$PUBLISH_LOCK" || return 0 if [ "$(status_get generation)" != "$generation" ]; then fm_lock_release "$PUBLISH_LOCK" return 0 @@ -387,7 +387,7 @@ cmd_run() { # budget=$(stage_budget) phases=probe if [ -n "$generation" ]; then - fm_lock_acquire_wait "$PUBLISH_LOCK" + fm_lock_acquire_wait "$PUBLISH_LOCK" || return 1 if [ "$(status_get generation)" = "$generation" ] && [ "$(status_get pid)" = "$$" ]; then internal=1 started=$(status_get started) @@ -410,7 +410,7 @@ cmd_run() { # if [ "$internal" -eq 0 ]; then generation="$(now).$$.manual" - fm_lock_acquire_wait "$PUBLISH_LOCK" + fm_lock_acquire_wait "$PUBLISH_LOCK" || return 1 if [ "$(status_get state)" = running ] && worker_alive; then fm_lock_release "$PUBLISH_LOCK" return 1 @@ -438,7 +438,11 @@ EOF stage_started=$(fm_timing_now_ms) rc=0 if [ "$sweep_locked" -eq 1 ]; then - fm_lock_acquire_wait "$STATE/.lock.acquire" + if ! fm_lock_acquire_wait "$STATE/.lock.acquire"; then + rm -f "$out" 2>/dev/null || true + [ -z "$timings" ] || rm -f "$timings" 2>/dev/null || true + return 1 + fi lease_held=1 if ! lock_unchanged "$lock_pid"; then sweep_locked=0 @@ -544,7 +548,7 @@ print_state() { cmd_harvest() { # local pid=$1 generation state claim_record claim_generation claim_pid - fm_lock_acquire_wait "$PUBLISH_LOCK" + fm_lock_acquire_wait "$PUBLISH_LOCK" || return 1 generation=$(status_get generation) # Another session's live claim is left alone; the worker reaps a dead one. if [ -f "$CLAIM_FILE" ]; then diff --git a/bin/fm-wake-lib.sh b/bin/fm-wake-lib.sh index ff32d88196e..aed31f80816 100755 --- a/bin/fm-wake-lib.sh +++ b/bin/fm-wake-lib.sh @@ -732,6 +732,14 @@ fm_lock_try_acquire() { if fm_lock_try_create "$lockdir"; then return 0 fi + # A lock that does not exist has no stale holder to recover. Creation failed + # because the lock's directory is gone or unwritable, or it lost a release race + # that a caller's retry absorbs. Stealing it would fail the same way one + # "$lockdir.steal" deeper, recursing without bound and burning a CPU core for + # as long as the directory stays gone. + if [ ! -e "$lockdir" ] && [ ! -L "$lockdir" ]; then + return 1 + fi # Compare against ${BASHPID:-$$} inline, never via a command substitution: # $() forks a subshell whose BASHPID is not this frame's pid. @@ -824,9 +832,17 @@ fm_lock_try_acquire() { return "$rc" } +# Waits until the lock is held, and fails only once the lock's directory no +# longer exists: a lock can never be taken there again, as when a process +# outlives the home it was working in. fm_lock_acquire_wait() { - local lockdir=$1 + local lockdir=$1 parent + case "$lockdir" in + */*) parent=${lockdir%/*}; parent=${parent:-/} ;; + *) parent=. ;; + esac while ! fm_lock_try_acquire "$lockdir"; do + [ -d "$parent" ] || return 1 sleep 0.1 done } @@ -942,7 +958,7 @@ fm_wake_append() { recovery_marker="$STATE/.watcher-down" status=0 - fm_lock_acquire_wait "$FM_WAKE_QUEUE_LOCK" + fm_lock_acquire_wait "$FM_WAKE_QUEUE_LOCK" || return 1 _fm_recovery_marker_publish "$recovery_marker" downtime || status=$? if [ "$status" -eq 0 ]; then seq=$(cat "$seq_file" 2>/dev/null || echo 0) diff --git a/tests/fm-startup-network.test.sh b/tests/fm-startup-network.test.sh index 17ca93d5596..e5f740a375b 100755 --- a/tests/fm-startup-network.test.sh +++ b/tests/fm-startup-network.test.sh @@ -263,6 +263,111 @@ EOF pass "fm-startup-network: a report-publication failure is failed, diagnosed, and still wakes" } +# The worker outlives the command that launched it, so the home it waits to +# deliver into can be removed underneath it: a retired home, or a test fixture +# cleaned up at exit. Its locks can never be taken there again, so a worker that +# kept waiting for them would keep loading the host long after its caller ended. +test_a_worker_whose_home_is_removed_exits() { + local rec home root log claimant held holder worker_pid survived=0 waited=0 + rec=$(new_world home-removed) + IFS='|' read -r home root log </dev/null || fail "the removed-home worker never published" + worker_pid=$(sed -n 's/^pid=//p' "$home/state/.startup-network.status") + kill -0 "$worker_pid" 2>/dev/null \ + || fail "the worker stopped before the removal it is meant to outlive" + + # Holding the publication lock puts the worker provably inside its lock wait, + # rather than between a lock and its next status read, when the home goes. + FM_STATE_OVERRIDE="$home/state" bash -c ' + . "$1" + fm_lock_acquire_wait "$2" + : > "$3" + exec sleep 30 + ' _ "$ROOT/bin/fm-wake-lib.sh" "$home/state/.startup-network.lock" "$held" & + holder=$! + while [ ! -e "$held" ] && [ "$waited" -lt 50 ]; do + sleep 0.1 + waited=$((waited + 1)) + done + [ -e "$held" ] || fail "the test could not take the worker's publication lock" + # Renamed away in one step, so the worker loses the whole home at once. + mv "$home" "$home.removed" || fail "the fixture home could not be moved away" + waited=0 + while kill -0 "$worker_pid" 2>/dev/null && [ "$waited" -lt 50 ]; do + sleep 0.1 + waited=$((waited + 1)) + done + if kill -0 "$worker_pid" 2>/dev/null; then + survived=1 + kill "$worker_pid" 2>/dev/null || true + fi + kill "$holder" "$claimant" 2>/dev/null || true + wait "$holder" "$claimant" 2>/dev/null || true + [ "$survived" -eq 0 ] || fail "the worker kept waiting on its locks after its home was removed" + pass "fm-startup-network: a worker whose home is removed exits instead of waiting forever" +} + +# A worker whose claimant is gone appends its wake while it holds the publication +# lock. A home removed during that append, as when a fixture's cleanup deletes the +# claim before the rest of the home, must end the worker too. +test_a_worker_delivering_its_wake_exits_when_its_home_is_removed() { + local rec home root log claimant queue_held holder worker_pid survived=0 waited=0 + rec=$(new_world home-removed-mid-wake) + IFS='|' read -r home root log </dev/null || fail "the mid-wake worker never published" + worker_pid=$(sed -n 's/^pid=//p' "$home/state/.startup-network.status") + + # With the wake queue held, a dead claimant sends the worker into its wake + # append; removing the claim is the worker's own signal that it is past the + # point of no return. + FM_STATE_OVERRIDE="$home/state" bash -c ' + . "$1" + fm_lock_acquire_wait "$2" + : > "$3" + exec sleep 30 + ' _ "$ROOT/bin/fm-wake-lib.sh" "$home/state/.wake-queue.lock" "$queue_held" & + holder=$! + while [ ! -e "$queue_held" ] && [ "$waited" -lt 50 ]; do + sleep 0.1 + waited=$((waited + 1)) + done + [ -e "$queue_held" ] || fail "the test could not take the wake queue lock" + kill "$claimant" 2>/dev/null || true + wait "$claimant" 2>/dev/null || true + waited=0 + while [ -e "$home/state/.startup-network.claim" ] && [ "$waited" -lt 50 ]; do + sleep 0.1 + waited=$((waited + 1)) + done + [ ! -e "$home/state/.startup-network.claim" ] || fail "the worker never started delivering its wake" + mv "$home" "$home.removed" || fail "the fixture home could not be moved away" + waited=0 + while kill -0 "$worker_pid" 2>/dev/null && [ "$waited" -lt 50 ]; do + sleep 0.1 + waited=$((waited + 1)) + done + if kill -0 "$worker_pid" 2>/dev/null; then + survived=1 + kill "$worker_pid" 2>/dev/null || true + fi + kill "$holder" 2>/dev/null || true + wait "$holder" 2>/dev/null || true + [ "$survived" -eq 0 ] || fail "the worker kept waiting to deliver its wake after its home was removed" + pass "fm-startup-network: a worker delivering its wake exits when its home is removed" +} + # The worker outlives the command that launched it. If another session took the # lock meanwhile, running the mutating sweeps would sweep underneath that # session, so they are refused - and the refusal is reported, not silent. @@ -616,6 +721,8 @@ test_start_returns_without_holding_the_callers_stdout test_harvest_acknowledgement_suppresses_the_wake_and_no_claim_produces_it test_a_claimant_crash_after_publish_still_queues_the_wake test_a_report_publication_failure_is_failed_and_still_wakes +test_a_worker_whose_home_is_removed_exits +test_a_worker_delivering_its_wake_exits_when_its_home_is_removed test_mutating_sweeps_are_refused_when_the_lock_changed_hands test_the_stage_bound_is_reported_not_swallowed test_an_abandoned_run_reads_as_needing_a_rerun diff --git a/tests/fm-watcher-lock.test.sh b/tests/fm-watcher-lock.test.sh index dcbef5edcbe..8efa1cc3d60 100755 --- a/tests/fm-watcher-lock.test.sh +++ b/tests/fm-watcher-lock.test.sh @@ -423,6 +423,35 @@ test_lock_paused_mid_acquire_claim_fails_during_steal() { pass "paused mid-acquire claimant backs off to active stealer" } +# A detached worker can outlive the home its locks live in. Once that directory +# is gone no lock can be created there: an acquirer that read the absent lock as +# stale recursed through ever-deeper .steal mutexes, burning a core for minutes, +# and a waiter kept waiting for good. The bound only turns either regression into +# a failure instead of a hang. +test_lock_in_removed_directory_is_refused_without_stealing() { + local dir state lockdir driver out rc + dir=$(make_case lock-removed-dir) + state="$dir/state" + lockdir="$state/.contend.lock" + driver="$dir/acquire-after-removal.sh" + cat > "$driver" <<'SH' +#!/usr/bin/env bash +. "$1" +rm -rf "$STATE" +if fm_lock_try_acquire "$2"; then echo acquired; else echo refused; fi +if fm_lock_acquire_wait "$2"; then echo waited-and-acquired; else echo wait-refused; fi +if [ -e "$STATE" ]; then echo recreated; fi +SH + rc=0 + out=$(FM_STATE_OVERRIDE="$state" bash -c '. "$1"; fm_run_timed 10 bash "$2" "$3" "$4"' \ + _ "$ROOT/bin/fm-timeout-lib.sh" "$driver" "$LIB" "$lockdir" 2>&1) || rc=$? + [ "$rc" -ne 124 ] || fail "acquiring or waiting for a lock in a removed directory never returned" + [ "$rc" -eq 0 ] || fail "lock acquisition in a removed directory crashed (rc=$rc): $out" + [ "$out" = "refused +wait-refused" ] || fail "a lock in a removed directory was not simply refused: $out" + pass "a lock whose directory was removed is refused, and waited for, without a steal chain" +} + test_watch_restart_rejects_reused_pid() { local dir state fakebin out live pid i dir=$(make_case restart-reused-pid) @@ -1117,6 +1146,7 @@ test_lock_does_not_steal_live_lock test_lock_empty_pid_uses_minimum_grace test_lock_late_claim_loses_after_recreate test_lock_paused_mid_acquire_claim_fails_during_steal +test_lock_in_removed_directory_is_refused_without_stealing test_watch_restart_rejects_reused_pid test_watch_restart_attaches_to_healthy_peer test_watcher_self_evicts_on_lock_takeover From 1df199b1396d82f277c4ed2958539ad5bec8ce37 Mon Sep 17 00:00:00 2001 From: SchokoShake <28960930+SchokoShake@users.noreply.github.com> Date: Thu, 17 Sep 2026 14:28:42 +0200 Subject: [PATCH 2/3] test(pi-watch): order recovery budgets explicitly instead of racing them Several watch-arm recovery cases raced their own timing. A successor launched through a login shell could still be starting when a 250ms readiness budget lapsed, so it was retired before it recorded itself or installed the TERM trap its case depends on. A 120ms sleep stood in for the OpenCode plugin's unowned launch settling, so a slow launch answered the next idle event with the lock it had already read and never armed. And the stub turn-end guards exited without reading the payload the plugin writes, which failed that write with an unhandled EPIPE on a loaded host. The recovery cases now run on a mock clock: each readiness, retirement, or retry budget lapses only when the case elapses it, and a case does so only after the child that budget measures has visibly reached the state under test. The lock case waits on the plugin's in-flight launch through its shared coordinator, and the stub guards read their payload as bin/fm-turnend-guard.sh does. No budget value changes. A case that fails leaves its arm fixtures looping on release files that never appear, so the suite now reaps every process still running from its own temp root when it exits, instead of leaving that load to later suites. With 20 CPU spinners on the test's CPU, the previous recovery cases passed 0 or 1 of 3 runs each, failing as CI did ("expected one successor plus two retries, got 2"), while the new ones pass every run. --- tests/fm-pi-watch-extension.test.sh | 300 ++++++++++++++++++++-------- 1 file changed, 215 insertions(+), 85 deletions(-) diff --git a/tests/fm-pi-watch-extension.test.sh b/tests/fm-pi-watch-extension.test.sh index ce43c0e25e5..ff8a5ef575e 100755 --- a/tests/fm-pi-watch-extension.test.sh +++ b/tests/fm-pi-watch-extension.test.sh @@ -7,11 +7,91 @@ set -u TMP_ROOT=$(fm_test_tmproot fm-pi-watch-extension) EXT="$ROOT/.pi/extensions/fm-primary-pi-watch.ts" + +# Arm fixtures loop until their case releases or retires them, so a case that +# fails first would leave them running after their temp root is gone, loading +# whatever runs next on the host. Reap every process still running from this +# suite's temp root before removing it. +cleanup_arm_fixtures() { + local pid args + ps -eo pid=,args= | while read -r pid args; do + case "$args" in + *"$TMP_ROOT/"*) kill -KILL "$pid" 2>/dev/null || true ;; + esac + done + fm_test_cleanup +} +trap cleanup_arm_fixtures EXIT # Node 24 warns when these test-only dynamic imports load tracked ESM plugins # from a clean checkout with no tracked .opencode/package.json. The warning is # unrelated to plugin output, which the assertions intentionally require empty. export NODE_NO_WARNINGS=1 +# The recovery cases below import this clock. The plugins bound each recovery +# step with a budget that starts when they launch an arm child (readiness, +# retirement, retry backoff), and a login-shell child on a loaded host may not +# run its first line within a short one: the budget could lapse before the child +# under test existed. With this clock a budget lapses only when a case elapses +# it, and a case elapses it only after the child that budget measures has +# visibly reached the state under test, so load can slow a case but never +# reorder it. Every case keeps the budget values it configures. +RECOVERY_CLOCK="$TMP_ROOT/recovery-clock.mjs" +cat > "$RECOVERY_CLOCK" <<'JS' +import { subscribe } from "node:diagnostics_channel"; +import { mock } from "node:test"; + +const realSetTimeout = globalThis.setTimeout; +const realSetImmediate = globalThis.setImmediate; +const armClosures = []; + +// Arm launches carry --restart. A close is recorded from launch time, so a case +// can wait for the same close a plugin waits for before it retries. +subscribe("child_process", ({ process: child }) => { + const closed = new Promise((resolve) => child.once("close", resolve)); + queueMicrotask(() => { + if (child.spawnargs.some((arg) => arg.includes("--restart"))) armClosures.push(closed); + }); +}); + +export function freeze() { + mock.timers.enable({ apis: ["setTimeout"] }); +} + +export function elapse(ms) { + mock.timers.tick(ms); +} + +export function sleep(ms) { + return new Promise((resolve) => realSetTimeout(resolve, ms)); +} + +// One real event-loop turn, after which every promise chain a lapsed budget or +// a handled close started has run up to its next budget or process event. +export function settle() { + return new Promise((resolve) => realSetImmediate(resolve)); +} + +export async function waitFor(predicate, message) { + for (let i = 0; i < 500; i += 1) { + if (predicate()) return; + await sleep(10); + } + throw new Error(message); +} + +// The index-th arm launch (0 is the first) has closed and its plugin has +// handled that close. +export async function retired(index) { + if (!armClosures[index]) throw new Error(`arm launch ${index} was never observed`); + let closed = false; + void armClosures[index].then(() => { + closed = true; + }); + await waitFor(() => closed, `arm launch ${index} never closed`); + await settle(); +} +JS + install_pi_watch_extension_fixture() { local repo=$1 mkdir -p \ @@ -430,13 +510,20 @@ trap 'exit 0' TERM INT while :; do sleep 0.02; done SH chmod +x "$repo/bin/fm-watch-arm.sh" - out=$(PLUGIN="$plugin" FM_HOME="$home" FM_ROOT_OVERRIDE="$repo" FM_ARM_LOG="$log" FM_PI_ARM_READY_TIMEOUT_MS=250 FM_WATCH_REARM_RETRY_BASE_MS=5 FM_WATCH_REARM_RETRY_MAX_MS=10 FM_WATCH_REARM_RETRY_LIMIT=2 node --input-type=module 2>&1 <<'EOF' + out=$(PLUGIN="$plugin" CLOCK="$RECOVERY_CLOCK" FM_HOME="$home" FM_ROOT_OVERRIDE="$repo" FM_ARM_LOG="$log" FM_PI_ARM_READY_TIMEOUT_MS=250 FM_WATCH_REARM_RETRY_BASE_MS=5 FM_WATCH_REARM_RETRY_MAX_MS=10 FM_WATCH_REARM_RETRY_LIMIT=2 node --input-type=module 2>&1 <<'EOF' import { existsSync, readFileSync, writeFileSync } from "node:fs"; import { pathToFileURL } from "node:url"; +const clock = await import(pathToFileURL(process.env.CLOCK).href); +clock.freeze(); +const readyMs = Number(process.env.FM_PI_ARM_READY_TIMEOUT_MS); +const retryMaxMs = Number(process.env.FM_WATCH_REARM_RETRY_MAX_MS); let tool = null; let prompt = ""; let rowsAtPrompt = 0; +const rows = () => existsSync(process.env.FM_ARM_LOG) + ? readFileSync(process.env.FM_ARM_LOG, "utf8").trim().split("\n") + : []; const pi = { on() {}, registerCommand() {}, @@ -445,28 +532,29 @@ const pi = { }, sendUserMessage: async (message) => { prompt += message; - rowsAtPrompt = existsSync(process.env.FM_ARM_LOG) - ? readFileSync(process.env.FM_ARM_LOG, "utf8").trim().split("\n").length - : 0; + rowsAtPrompt = rows().length; }, }; writeFileSync(`${process.env.FM_HOME}/state/.lock`, `${process.pid}\n`); const mod = await import(pathToFileURL(process.env.PLUGIN).href); mod.default(pi); await tool.execute("tool-call-hung-successor", {}, undefined, undefined, {}); -for (let i = 0; i < 500 && !prompt; i += 1) { - await new Promise((resolve) => setTimeout(resolve, 10)); -} -const rows = existsSync(process.env.FM_ARM_LOG) - ? readFileSync(process.env.FM_ARM_LOG, "utf8").trim().split("\n") - : []; -if (rows.length !== 4) throw new Error(`expected one successor plus two retries, got ${rows.length}: ${rows.join(" | ")}`); +// Each successor is running, and never becomes ready, before its readiness +// budget lapses; the next retry comes due only once that successor is retired. +for (let successor = 1; successor <= 3; successor += 1) { + await clock.waitFor(() => rows().length > successor, `successor ${successor} never started`); + clock.elapse(readyMs); + await clock.retired(successor); + clock.elapse(retryMaxMs); +} +await clock.waitFor(() => prompt, "the actionable wake was never delivered"); +if (rows().length !== 4) throw new Error(`expected one successor plus two retries, got ${rows().length}: ${rows().join(" | ")}`); if (rowsAtPrompt !== 4) throw new Error(`wake arrived before restoration exhausted (${rowsAtPrompt} arm rows)`); if (!prompt.includes("signal: synthetic wake")) throw new Error(`original wake was lost: ${prompt}`); if (!prompt.includes("could not restore watcher continuity after 2 retries")) throw new Error(`missing typed restoration failure: ${prompt}`); -await new Promise((resolve) => setTimeout(resolve, 100)); -const stableRows = readFileSync(process.env.FM_ARM_LOG, "utf8").trim().split("\n"); -if (stableRows.length !== 4) throw new Error(`single-flight recovery launched ${stableRows.length} arms`); +clock.elapse(readyMs + retryMaxMs); +await clock.sleep(100); +if (rows().length !== 4) throw new Error(`single-flight recovery launched ${rows().length} arms`); EOF ) status=$? @@ -502,13 +590,18 @@ printf 'arm=%s\n' "$$" >> "${FM_ARM_LOG:?}" while [ ! -e "$FM_RELEASE_FILE" ]; do sleep 0.1; done SH chmod +x "$repo/bin/fm-watch-arm.sh" - out=$(PLUGIN="$plugin" FM_HOME="$home" FM_ROOT_OVERRIDE="$repo" FM_ARM_LOG="$log" FM_RELEASE_FILE="$release" FM_PI_ARM_READY_TIMEOUT_MS=250 FM_WATCH_ARM_RETIRE_TIMEOUT_MS=20 FM_WATCH_REARM_RETRY_BASE_MS=5 FM_WATCH_REARM_RETRY_MAX_MS=10 FM_WATCH_REARM_RETRY_LIMIT=2 node --input-type=module 2>&1 <<'EOF' + out=$(PLUGIN="$plugin" CLOCK="$RECOVERY_CLOCK" FM_HOME="$home" FM_ROOT_OVERRIDE="$repo" FM_ARM_LOG="$log" FM_RELEASE_FILE="$release" FM_PI_ARM_READY_TIMEOUT_MS=250 FM_WATCH_ARM_RETIRE_TIMEOUT_MS=20 FM_WATCH_REARM_RETRY_BASE_MS=5 FM_WATCH_REARM_RETRY_MAX_MS=10 FM_WATCH_REARM_RETRY_LIMIT=2 node --input-type=module 2>&1 <<'EOF' import { existsSync, readFileSync, writeFileSync } from "node:fs"; import { pathToFileURL } from "node:url"; +const clock = await import(pathToFileURL(process.env.CLOCK).href); +clock.freeze(); let tool = null; let prompt = ""; let rowsAtPrompt = 0; +const rows = () => existsSync(process.env.FM_ARM_LOG) + ? readFileSync(process.env.FM_ARM_LOG, "utf8").trim().split("\n") + : []; const pi = { on() {}, registerCommand() {}, @@ -517,27 +610,26 @@ const pi = { }, sendUserMessage: async (message) => { prompt += message; - rowsAtPrompt = existsSync(process.env.FM_ARM_LOG) - ? readFileSync(process.env.FM_ARM_LOG, "utf8").trim().split("\n").length - : 0; + rowsAtPrompt = rows().length; }, }; writeFileSync(`${process.env.FM_HOME}/state/.lock`, `${process.pid}\n`); const mod = await import(pathToFileURL(process.env.PLUGIN).href); mod.default(pi); await tool.execute("tool-call-unretired-successor", {}, undefined, undefined, {}); -for (let i = 0; i < 500 && !prompt; i += 1) { - await new Promise((resolve) => setTimeout(resolve, 10)); -} -const rows = existsSync(process.env.FM_ARM_LOG) - ? readFileSync(process.env.FM_ARM_LOG, "utf8").trim().split("\n") - : []; -if (rows.length !== 2) throw new Error(`unretired arm overlapped a retry: ${rows.join(" | ")}`); +// The successor records itself only after it ignores TERM, so its readiness +// budget lapses on a successor that will outlast its retirement budget. +await clock.waitFor(() => rows().length >= 2, "the successor never started"); +clock.elapse(Number(process.env.FM_PI_ARM_READY_TIMEOUT_MS)); +await clock.settle(); +clock.elapse(Number(process.env.FM_WATCH_ARM_RETIRE_TIMEOUT_MS)); +await clock.waitFor(() => prompt, "the actionable wake was never delivered"); +if (rows().length !== 2) throw new Error(`unretired arm overlapped a retry: ${rows().join(" | ")}`); if (rowsAtPrompt !== 2) throw new Error(`wake arrived after an overlapping retry (${rowsAtPrompt} arm rows)`); if (!prompt.includes("signal: synthetic wake")) throw new Error(`original wake was lost: ${prompt}`); if (!prompt.includes("unready successor arm did not exit within 20ms")) throw new Error(`missing unretired-arm failure: ${prompt}`); writeFileSync(process.env.FM_RELEASE_FILE, "release\n"); -await new Promise((resolve) => setTimeout(resolve, 80)); +await clock.sleep(80); EOF ) status=$? @@ -580,10 +672,12 @@ trap 'exit 0' TERM INT while [ ! -e "$FM_STOP_FILE" ]; do sleep 0.02; done SH chmod +x "$repo/bin/fm-watch-arm.sh" - out=$(PLUGIN="$plugin" FM_HOME="$home" FM_ROOT_OVERRIDE="$repo" FM_ARM_LOG="$log" FM_UNRETIRED_READY_FILE="$ready" FM_UNRETIRED_RETIRE_FILE="$retired" FM_RELEASE_FILE="$release" FM_STOP_FILE="$stop" FM_LATE_KIND="$kind" FM_PI_ARM_READY_TIMEOUT_MS=250 FM_WATCH_ARM_RETIRE_TIMEOUT_MS=20 FM_WATCH_REARM_RETRY_BASE_MS=5 FM_WATCH_REARM_RETRY_MAX_MS=10 FM_WATCH_REARM_RETRY_LIMIT=2 node --input-type=module 2>&1 <<'EOF' + out=$(PLUGIN="$plugin" CLOCK="$RECOVERY_CLOCK" FM_HOME="$home" FM_ROOT_OVERRIDE="$repo" FM_ARM_LOG="$log" FM_UNRETIRED_READY_FILE="$ready" FM_UNRETIRED_RETIRE_FILE="$retired" FM_RELEASE_FILE="$release" FM_STOP_FILE="$stop" FM_LATE_KIND="$kind" FM_PI_ARM_READY_TIMEOUT_MS=250 FM_WATCH_ARM_RETIRE_TIMEOUT_MS=20 FM_WATCH_REARM_RETRY_BASE_MS=5 FM_WATCH_REARM_RETRY_MAX_MS=10 FM_WATCH_REARM_RETRY_LIMIT=2 node --input-type=module 2>&1 <<'EOF' import { existsSync, readFileSync, writeFileSync } from "node:fs"; import { pathToFileURL } from "node:url"; +const clock = await import(pathToFileURL(process.env.CLOCK).href); +clock.freeze(); let tool = null; const prompts = []; const pi = { @@ -599,33 +693,37 @@ const pi = { const rows = () => existsSync(process.env.FM_ARM_LOG) ? readFileSync(process.env.FM_ARM_LOG, "utf8").trim().split("\n") : []; -async function waitFor(predicate, message) { - for (let i = 0; i < 500; i += 1) { - if (predicate()) return; - await new Promise((resolve) => setTimeout(resolve, 10)); - } - throw new Error(message); -} writeFileSync(`${process.env.FM_HOME}/state/.lock`, `${process.pid}\n`); const mod = await import(pathToFileURL(process.env.PLUGIN).href); mod.default(pi); await tool.execute("tool-call-late-close", {}, undefined, undefined, {}); -await waitFor( +await clock.waitFor( () => existsSync(process.env.FM_UNRETIRED_READY_FILE), "unretired successor did not enter its retirement wait", ); -await waitFor(() => prompts.length >= 1, "original fallback was not delivered"); -await waitFor( +// The successor's TERM trap is in place before its readiness budget lapses, so +// retirement is asked for and outlasted rather than raced. +clock.elapse(Number(process.env.FM_PI_ARM_READY_TIMEOUT_MS)); +await clock.settle(); +clock.elapse(Number(process.env.FM_WATCH_ARM_RETIRE_TIMEOUT_MS)); +await clock.waitFor(() => prompts.length >= 1, "original fallback was not delivered"); +await clock.waitFor( () => existsSync(process.env.FM_UNRETIRED_RETIRE_FILE), "unretired successor was not asked to retire before fallback", ); if (rows().length !== 2) throw new Error(`unretired arm overlapped before fallback: ${rows().join(" | ")}`); if (!prompts[0]?.includes("original wake")) throw new Error(`missing original fallback: ${prompts.join(" | ")}`); writeFileSync(process.env.FM_RELEASE_FILE, "release\n"); -for (let i = 0; i < 500; i += 1) { - if (rows().length >= 3 && (process.env.FM_LATE_KIND !== "actionable" || prompts.some((message) => message.includes("late wake")))) break; - await new Promise((resolve) => setTimeout(resolve, 10)); -} +if (process.env.FM_LATE_KIND !== "actionable") { + // A non-actionable close schedules a continuity retry, which comes due only + // once that close has been handled. + await clock.retired(1); + clock.elapse(Number(process.env.FM_WATCH_REARM_RETRY_MAX_MS)); +} +await clock.waitFor( + () => rows().length >= 3 && (process.env.FM_LATE_KIND !== "actionable" || prompts.some((message) => message.includes("late wake"))), + "late close did not restore supervision", +); if (rows().length !== 3) throw new Error(`late close did not restore one successor: ${rows().join(" | ")}`); if (process.env.FM_LATE_KIND === "actionable") { if (prompts.length !== 2 || !prompts[1].includes("late wake")) throw new Error(`late actionable close was not delivered: ${prompts.join(" | ")}`); @@ -633,7 +731,7 @@ if (process.env.FM_LATE_KIND === "actionable") { throw new Error(`late non-actionable close sent an extra wake: ${prompts.join(" | ")}`); } writeFileSync(process.env.FM_STOP_FILE, "stop\n"); -await new Promise((resolve) => setTimeout(resolve, 80)); +await clock.sleep(80); EOF ) status=$? @@ -1343,7 +1441,15 @@ const hooks = await mod.FmPrimaryWatchArm({ const event = { event: { type: "session.idle", properties: { sessionID: "session-test" } } }; writeFileSync(`${process.env.FM_HOME}/state/.lock`, "999999\n"); await hooks.event(event); -await new Promise((resolve) => setTimeout(resolve, 120)); +// The idle hook does not wait for the launch it starts, and a launch still in +// flight answers every later request with the lock it already read. The shared +// coordinator joins that same launch, so the lock changes only once the +// unowned launch has settled. +const unowned = await globalThis.__firstmateOpenCodeWatchArm.ensureArmed("session-test", client); +if (unowned !== "read-only") { + console.error(`expected read-only without the session lock, got ${unowned}`); + process.exit(1); +} if (existsSync(process.env.FM_ARM_LOG)) { console.error("watch arm ran without owning the session lock"); process.exit(1); @@ -1610,20 +1716,25 @@ trap 'exit 0' TERM INT while :; do sleep 0.02; done SH chmod +x "$repo/bin/fm-watch-arm.sh" - out=$(PLUGIN="$plugin" WORKTREE="$repo" FM_HOME="$home" FM_ARM_LOG="$log" FM_OPENCODE_ARM_READY_TIMEOUT_MS=250 FM_WATCH_REARM_RETRY_BASE_MS=5 FM_WATCH_REARM_RETRY_MAX_MS=10 FM_WATCH_REARM_RETRY_LIMIT=2 node 2>&1 <<'EOF' + out=$(PLUGIN="$plugin" CLOCK="$RECOVERY_CLOCK" WORKTREE="$repo" FM_HOME="$home" FM_ARM_LOG="$log" FM_OPENCODE_ARM_READY_TIMEOUT_MS=250 FM_WATCH_REARM_RETRY_BASE_MS=5 FM_WATCH_REARM_RETRY_MAX_MS=10 FM_WATCH_REARM_RETRY_LIMIT=2 node 2>&1 <<'EOF' import { existsSync, readFileSync, writeFileSync } from "node:fs"; import { pathToFileURL } from "node:url"; +const clock = await import(pathToFileURL(process.env.CLOCK).href); +clock.freeze(); +const readyMs = Number(process.env.FM_OPENCODE_ARM_READY_TIMEOUT_MS); +const retryMaxMs = Number(process.env.FM_WATCH_REARM_RETRY_MAX_MS); const mod = await import(pathToFileURL(process.env.PLUGIN).href); let prompt = ""; let rowsAtPrompt = 0; +const rows = () => existsSync(process.env.FM_ARM_LOG) + ? readFileSync(process.env.FM_ARM_LOG, "utf8").trim().split("\n") + : []; const client = { session: { promptAsync: async (request) => { prompt += request.body.parts[0].text; - rowsAtPrompt = existsSync(process.env.FM_ARM_LOG) - ? readFileSync(process.env.FM_ARM_LOG, "utf8").trim().split("\n").length - : 0; + rowsAtPrompt = rows().length; }, }, }; @@ -1634,19 +1745,22 @@ const hooks = await mod.FmPrimaryWatchArm({ }); writeFileSync(`${process.env.FM_HOME}/state/.lock`, `${process.pid}\n`); await hooks.event({ event: { type: "session.idle", properties: { sessionID: "session-test" } } }); -for (let i = 0; i < 500 && !prompt; i += 1) { - await new Promise((resolve) => setTimeout(resolve, 10)); -} -const rows = existsSync(process.env.FM_ARM_LOG) - ? readFileSync(process.env.FM_ARM_LOG, "utf8").trim().split("\n") - : []; -if (rows.length !== 4) throw new Error(`expected one successor plus two retries, got ${rows.length}: ${rows.join(" | ")}`); +// Each successor is running, and never becomes ready, before its readiness +// budget lapses; the next retry comes due only once that successor is retired. +for (let successor = 1; successor <= 3; successor += 1) { + await clock.waitFor(() => rows().length > successor, `successor ${successor} never started`); + clock.elapse(readyMs); + await clock.retired(successor); + clock.elapse(retryMaxMs); +} +await clock.waitFor(() => prompt, "the actionable wake was never delivered"); +if (rows().length !== 4) throw new Error(`expected one successor plus two retries, got ${rows().length}: ${rows().join(" | ")}`); if (rowsAtPrompt !== 4) throw new Error(`wake arrived before restoration exhausted (${rowsAtPrompt} arm rows)`); if (!prompt.includes("signal: synthetic wake")) throw new Error(`original wake was lost: ${prompt}`); if (!prompt.includes("could not restore watcher continuity after 2 retries")) throw new Error(`missing typed restoration failure: ${prompt}`); -await new Promise((resolve) => setTimeout(resolve, 100)); -const stableRows = readFileSync(process.env.FM_ARM_LOG, "utf8").trim().split("\n"); -if (stableRows.length !== 4) throw new Error(`single-flight recovery launched ${stableRows.length} arms`); +clock.elapse(readyMs + retryMaxMs); +await clock.sleep(100); +if (rows().length !== 4) throw new Error(`single-flight recovery launched ${rows().length} arms`); EOF ) status=$? @@ -1684,20 +1798,23 @@ printf 'arm=%s\n' "$$" >> "${FM_ARM_LOG:?}" while [ ! -e "$FM_RELEASE_FILE" ]; do sleep 0.1; done SH chmod +x "$repo/bin/fm-watch-arm.sh" - out=$(PLUGIN="$plugin" WORKTREE="$repo" FM_HOME="$home" FM_ARM_LOG="$log" FM_RELEASE_FILE="$release" FM_OPENCODE_ARM_READY_TIMEOUT_MS=250 FM_WATCH_ARM_RETIRE_TIMEOUT_MS=20 FM_WATCH_REARM_RETRY_BASE_MS=5 FM_WATCH_REARM_RETRY_MAX_MS=10 FM_WATCH_REARM_RETRY_LIMIT=2 node 2>&1 <<'EOF' + out=$(PLUGIN="$plugin" CLOCK="$RECOVERY_CLOCK" WORKTREE="$repo" FM_HOME="$home" FM_ARM_LOG="$log" FM_RELEASE_FILE="$release" FM_OPENCODE_ARM_READY_TIMEOUT_MS=250 FM_WATCH_ARM_RETIRE_TIMEOUT_MS=20 FM_WATCH_REARM_RETRY_BASE_MS=5 FM_WATCH_REARM_RETRY_MAX_MS=10 FM_WATCH_REARM_RETRY_LIMIT=2 node 2>&1 <<'EOF' import { existsSync, readFileSync, writeFileSync } from "node:fs"; import { pathToFileURL } from "node:url"; +const clock = await import(pathToFileURL(process.env.CLOCK).href); +clock.freeze(); const mod = await import(pathToFileURL(process.env.PLUGIN).href); let prompt = ""; let rowsAtPrompt = 0; +const rows = () => existsSync(process.env.FM_ARM_LOG) + ? readFileSync(process.env.FM_ARM_LOG, "utf8").trim().split("\n") + : []; const client = { session: { promptAsync: async (request) => { prompt += request.body.parts[0].text; - rowsAtPrompt = existsSync(process.env.FM_ARM_LOG) - ? readFileSync(process.env.FM_ARM_LOG, "utf8").trim().split("\n").length - : 0; + rowsAtPrompt = rows().length; }, }, }; @@ -1708,18 +1825,19 @@ const hooks = await mod.FmPrimaryWatchArm({ }); writeFileSync(`${process.env.FM_HOME}/state/.lock`, `${process.pid}\n`); await hooks.event({ event: { type: "session.idle", properties: { sessionID: "session-test" } } }); -for (let i = 0; i < 500 && !prompt; i += 1) { - await new Promise((resolve) => setTimeout(resolve, 10)); -} -const rows = existsSync(process.env.FM_ARM_LOG) - ? readFileSync(process.env.FM_ARM_LOG, "utf8").trim().split("\n") - : []; -if (rows.length !== 2) throw new Error(`unretired arm overlapped a retry: ${rows.join(" | ")}`); +// The successor records itself only after it ignores TERM, so its readiness +// budget lapses on a successor that will outlast its retirement budget. +await clock.waitFor(() => rows().length >= 2, "the successor never started"); +clock.elapse(Number(process.env.FM_OPENCODE_ARM_READY_TIMEOUT_MS)); +await clock.settle(); +clock.elapse(Number(process.env.FM_WATCH_ARM_RETIRE_TIMEOUT_MS)); +await clock.waitFor(() => prompt, "the actionable wake was never delivered"); +if (rows().length !== 2) throw new Error(`unretired arm overlapped a retry: ${rows().join(" | ")}`); if (rowsAtPrompt !== 2) throw new Error(`wake arrived after an overlapping retry (${rowsAtPrompt} arm rows)`); if (!prompt.includes("signal: synthetic wake")) throw new Error(`original wake was lost: ${prompt}`); if (!prompt.includes("unready successor arm did not exit within 20ms")) throw new Error(`missing unretired-arm failure: ${prompt}`); writeFileSync(process.env.FM_RELEASE_FILE, "release\n"); -await new Promise((resolve) => setTimeout(resolve, 80)); +await clock.sleep(80); EOF ) status=$? @@ -1764,10 +1882,12 @@ trap 'exit 0' TERM INT while [ ! -e "$FM_STOP_FILE" ]; do sleep 0.02; done SH chmod +x "$repo/bin/fm-watch-arm.sh" - out=$(PLUGIN="$plugin" WORKTREE="$repo" FM_HOME="$home" FM_ARM_LOG="$log" FM_UNRETIRED_READY_FILE="$ready" FM_UNRETIRED_RETIRE_FILE="$retired" FM_RELEASE_FILE="$release" FM_STOP_FILE="$stop" FM_LATE_KIND="$kind" FM_OPENCODE_ARM_READY_TIMEOUT_MS=250 FM_WATCH_ARM_RETIRE_TIMEOUT_MS=20 FM_WATCH_REARM_RETRY_BASE_MS=5 FM_WATCH_REARM_RETRY_MAX_MS=10 FM_WATCH_REARM_RETRY_LIMIT=2 node 2>&1 <<'EOF' + out=$(PLUGIN="$plugin" CLOCK="$RECOVERY_CLOCK" WORKTREE="$repo" FM_HOME="$home" FM_ARM_LOG="$log" FM_UNRETIRED_READY_FILE="$ready" FM_UNRETIRED_RETIRE_FILE="$retired" FM_RELEASE_FILE="$release" FM_STOP_FILE="$stop" FM_LATE_KIND="$kind" FM_OPENCODE_ARM_READY_TIMEOUT_MS=250 FM_WATCH_ARM_RETIRE_TIMEOUT_MS=20 FM_WATCH_REARM_RETRY_BASE_MS=5 FM_WATCH_REARM_RETRY_MAX_MS=10 FM_WATCH_REARM_RETRY_LIMIT=2 node 2>&1 <<'EOF' import { existsSync, readFileSync, writeFileSync } from "node:fs"; import { pathToFileURL } from "node:url"; +const clock = await import(pathToFileURL(process.env.CLOCK).href); +clock.freeze(); const mod = await import(pathToFileURL(process.env.PLUGIN).href); const prompts = []; const client = { @@ -1780,13 +1900,6 @@ const client = { const rows = () => existsSync(process.env.FM_ARM_LOG) ? readFileSync(process.env.FM_ARM_LOG, "utf8").trim().split("\n") : []; -async function waitFor(predicate, message) { - for (let i = 0; i < 500; i += 1) { - if (predicate()) return; - await new Promise((resolve) => setTimeout(resolve, 10)); - } - throw new Error(message); -} const hooks = await mod.FmPrimaryWatchArm({ client, directory: process.env.WORKTREE, @@ -1794,22 +1907,33 @@ const hooks = await mod.FmPrimaryWatchArm({ }); writeFileSync(`${process.env.FM_HOME}/state/.lock`, `${process.pid}\n`); await hooks.event({ event: { type: "session.idle", properties: { sessionID: "session-test" } } }); -await waitFor( +await clock.waitFor( () => existsSync(process.env.FM_UNRETIRED_READY_FILE), "unretired successor did not enter its retirement wait", ); -await waitFor(() => prompts.length >= 1, "original fallback was not delivered"); -await waitFor( +// The successor's TERM trap is in place before its readiness budget lapses, so +// retirement is asked for and outlasted rather than raced. +clock.elapse(Number(process.env.FM_OPENCODE_ARM_READY_TIMEOUT_MS)); +await clock.settle(); +clock.elapse(Number(process.env.FM_WATCH_ARM_RETIRE_TIMEOUT_MS)); +await clock.waitFor(() => prompts.length >= 1, "original fallback was not delivered"); +await clock.waitFor( () => existsSync(process.env.FM_UNRETIRED_RETIRE_FILE), "unretired successor was not asked to retire before fallback", ); if (rows().length !== 2) throw new Error(`unretired arm overlapped before fallback: ${rows().join(" | ")}`); if (!prompts[0]?.includes("original wake")) throw new Error(`missing original fallback: ${prompts.join(" | ")}`); writeFileSync(process.env.FM_RELEASE_FILE, "release\n"); -for (let i = 0; i < 500; i += 1) { - if (rows().length >= 3 && (process.env.FM_LATE_KIND !== "actionable" || prompts.some((message) => message.includes("late wake")))) break; - await new Promise((resolve) => setTimeout(resolve, 10)); -} +if (process.env.FM_LATE_KIND !== "actionable") { + // A non-actionable close schedules a continuity retry, which comes due only + // once that close has been handled. + await clock.retired(1); + clock.elapse(Number(process.env.FM_WATCH_REARM_RETRY_MAX_MS)); +} +await clock.waitFor( + () => rows().length >= 3 && (process.env.FM_LATE_KIND !== "actionable" || prompts.some((message) => message.includes("late wake"))), + "late close did not restore supervision", +); if (rows().length !== 3) throw new Error(`late close did not restore one successor: ${rows().join(" | ")}`); if (process.env.FM_LATE_KIND === "actionable") { if (prompts.length !== 2 || !prompts[1].includes("late wake")) throw new Error(`late actionable close was not delivered: ${prompts.join(" | ")}`); @@ -1817,7 +1941,7 @@ if (process.env.FM_LATE_KIND === "actionable") { throw new Error(`late non-actionable close sent an extra wake: ${prompts.join(" | ")}`); } writeFileSync(process.env.FM_STOP_FILE, "stop\n"); -await new Promise((resolve) => setTimeout(resolve, 80)); +await clock.sleep(80); EOF ) status=$? @@ -2024,6 +2148,9 @@ printf 'watcher: started pid=1 (beacon fresh)\n' SH cat > "$repo/bin/fm-turnend-guard.sh" <<'SH' #!/usr/bin/env bash +# Consume the hook payload as the real guard does; exiting unread races the +# plugin's write of it and can fail that write with EPIPE on a loaded host. +cat >/dev/null printf 'guard\n' >> "${FM_GUARD_LOG:?}" printf 'guard should not run\n' >&2 exit 2 @@ -2097,6 +2224,9 @@ printf 'watcher: healthy pid=1 (beacon 0s)\n' SH cat > "$repo/bin/fm-turnend-guard.sh" <<'SH' #!/usr/bin/env bash +# Consume the hook payload as the real guard does; exiting unread races the +# plugin's write of it and can fail that write with EPIPE on a loaded host. +cat >/dev/null printf 'guard\n' >> "${FM_GUARD_LOG:?}" printf 'guard ran after external healthy watcher\n' >&2 exit 2 From 651212bde4414a27d1bb23ee0d0e516a1ef556fd Mon Sep 17 00:00:00 2001 From: SchokoShake <28960930+SchokoShake@users.noreply.github.com> Date: Thu, 17 Sep 2026 15:51:11 +0200 Subject: [PATCH 3/3] no-mistakes(review): Guard test reaper, retry released locks, fail-closed claim mutex --- bin/fm-lock.sh | 5 ++- bin/fm-wake-lib.sh | 17 +++++--- tests/fm-pi-watch-extension.test.sh | 17 +++++--- tests/fm-startup-network.test.sh | 66 +++++++++++++++++++++++++++++ tests/fm-watcher-lock.test.sh | 57 +++++++++++++++++++++++++ 5 files changed, 149 insertions(+), 13 deletions(-) diff --git a/bin/fm-lock.sh b/bin/fm-lock.sh index 52d7c8aee4b..cb5a521a171 100755 --- a/bin/fm-lock.sh +++ b/bin/fm-lock.sh @@ -73,7 +73,10 @@ if ! fm_lock_try_acquire "$CLAIM_LOCK"; then echo "error: the prior session's bounded startup sweep is finishing; operate read-only until it releases the fleet lock" >&2 exit 1 fi - fm_lock_acquire_wait "$CLAIM_LOCK" + if ! fm_lock_acquire_wait "$CLAIM_LOCK"; then + echo "error: cannot take the fleet lock claim mutex; operate read-only until resolved" >&2 + exit 1 + fi fi CLAIM_LOCK_HELD=1 diff --git a/bin/fm-wake-lib.sh b/bin/fm-wake-lib.sh index aed31f80816..64ef26b2919 100755 --- a/bin/fm-wake-lib.sh +++ b/bin/fm-wake-lib.sh @@ -732,13 +732,18 @@ fm_lock_try_acquire() { if fm_lock_try_create "$lockdir"; then return 0 fi - # A lock that does not exist has no stale holder to recover. Creation failed - # because the lock's directory is gone or unwritable, or it lost a release race - # that a caller's retry absorbs. Stealing it would fail the same way one - # "$lockdir.steal" deeper, recursing without bound and burning a CPU core for - # as long as the directory stays gone. + # A lock that does not exist has no stale holder to recover. Either its holder + # released it just after creation failed, which one more creation attempt + # settles, or the lock's directory is gone or unwritable. Stealing it then + # would fail the same way one "$lockdir.steal" deeper, recursing without bound + # and burning a CPU core for as long as the directory stays gone. if [ ! -e "$lockdir" ] && [ ! -L "$lockdir" ]; then - return 1 + if fm_lock_try_create "$lockdir"; then + return 0 + fi + if [ ! -e "$lockdir" ] && [ ! -L "$lockdir" ]; then + return 1 + fi fi # Compare against ${BASHPID:-$$} inline, never via a command substitution: diff --git a/tests/fm-pi-watch-extension.test.sh b/tests/fm-pi-watch-extension.test.sh index ff8a5ef575e..8a217d0ad9e 100755 --- a/tests/fm-pi-watch-extension.test.sh +++ b/tests/fm-pi-watch-extension.test.sh @@ -5,7 +5,10 @@ set -u # shellcheck source=tests/lib.sh . "$(dirname "${BASH_SOURCE[0]}")/lib.sh" -TMP_ROOT=$(fm_test_tmproot fm-pi-watch-extension) +TMP_ROOT=$(fm_test_tmproot fm-pi-watch-extension) || TMP_ROOT= +if [ -z "$TMP_ROOT" ] || [ ! -d "$TMP_ROOT" ]; then + fail "could not create the suite's temp root" +fi EXT="$ROOT/.pi/extensions/fm-primary-pi-watch.ts" # Arm fixtures loop until their case releases or retires them, so a case that @@ -14,11 +17,13 @@ EXT="$ROOT/.pi/extensions/fm-primary-pi-watch.ts" # suite's temp root before removing it. cleanup_arm_fixtures() { local pid args - ps -eo pid=,args= | while read -r pid args; do - case "$args" in - *"$TMP_ROOT/"*) kill -KILL "$pid" 2>/dev/null || true ;; - esac - done + if [ -n "${TMP_ROOT:-}" ] && [ -d "$TMP_ROOT" ]; then + ps -eo pid=,args= | while read -r pid args; do + case "$args" in + *"$TMP_ROOT/"*) kill -KILL "$pid" 2>/dev/null || true ;; + esac + done + fi fm_test_cleanup } trap cleanup_arm_fixtures EXIT diff --git a/tests/fm-startup-network.test.sh b/tests/fm-startup-network.test.sh index e5f740a375b..1d2bec7dc1a 100755 --- a/tests/fm-startup-network.test.sh +++ b/tests/fm-startup-network.test.sh @@ -572,6 +572,71 @@ EOF pass "fm-startup-network: fleet-lock takeover cannot overlap a mutating sweep" } +# A takeover that queues behind another session's claim can lose its home while +# it waits. The claim mutex can never be taken there, so the takeover must say so +# and stop rather than carry on as if it held the mutex. +test_lock_takeover_fails_closed_when_its_home_is_removed_mid_claim() { + local rec home root log held holder lock_pid out rc=0 waited=0 queued=0 + rec=$(new_world claim-home-removed) + IFS='|' read -r home root log < "$3" + exec sleep 30 + ' _ "$ROOT/bin/fm-wake-lib.sh" "$home/state/.lock.acquire" "$held" & + holder=$! + while [ ! -e "$held" ] && [ "$waited" -lt 50 ]; do + sleep 0.1 + waited=$((waited + 1)) + done + [ -e "$held" ] || fail "the test could not take the claim mutex" + + PATH="$root/bin:$PATH" FM_FAKE_HARNESS_PID=$$ FM_HOME="$home" FM_ROOT_OVERRIDE="$root" \ + "$root/bin/fm-lock.sh" > "$out" 2>&1 & + lock_pid=$! + # The takeover sleeps only between attempts at the held claim mutex, so a + # sleeping child puts it provably inside that wait when the home goes. + waited=0 + while [ "$queued" -eq 0 ] && [ "$waited" -lt 100 ] && kill -0 "$lock_pid" 2>/dev/null; do + if pgrep -P "$lock_pid" -x sleep >/dev/null 2>&1; then + queued=1 + else + sleep 0.1 + waited=$((waited + 1)) + fi + done + if [ "$queued" -eq 0 ]; then + kill "$lock_pid" "$holder" 2>/dev/null || true + fail "the takeover never queued behind the held claim mutex: $(cat "$out" 2>/dev/null)" + fi + # Renamed away in one step, so the takeover loses the whole home at once. + mv "$home" "$home.removed" || fail "the fixture home could not be moved away" + waited=0 + while kill -0 "$lock_pid" 2>/dev/null && [ "$waited" -lt 50 ]; do + sleep 0.1 + waited=$((waited + 1)) + done + if kill -0 "$lock_pid" 2>/dev/null; then + kill "$lock_pid" "$holder" 2>/dev/null || true + fail "the takeover kept waiting for a claim mutex in a removed home" + fi + wait "$lock_pid" || rc=$? + kill "$holder" 2>/dev/null || true + wait "$holder" 2>/dev/null || true + [ "$rc" -eq 1 ] || fail "a takeover that lost its home mid-claim exited $rc: $(cat "$out")" + assert_contains "$(cat "$out")" "claim mutex" \ + "a takeover that never held the claim mutex did not report it: $(cat "$out")" + assert_contains "$(cat "$out")" "operate read-only" \ + "a takeover that lost its home mid-claim did not fail closed to read-only: $(cat "$out")" + [ ! -e "$home" ] || fail "the failed takeover recreated the removed home" + pass "fm-startup-network: a fleet-lock takeover that loses its home mid-claim fails closed" +} + # Every record carries a start offset from ONE origin, so the artifact reads as a # timeline and not just a bag of durations. The origin is normally exported by the # stage, but a process that starts recording without one has to adopt an origin @@ -723,6 +788,7 @@ test_a_claimant_crash_after_publish_still_queues_the_wake test_a_report_publication_failure_is_failed_and_still_wakes test_a_worker_whose_home_is_removed_exits test_a_worker_delivering_its_wake_exits_when_its_home_is_removed +test_lock_takeover_fails_closed_when_its_home_is_removed_mid_claim test_mutating_sweeps_are_refused_when_the_lock_changed_hands test_the_stage_bound_is_reported_not_swallowed test_an_abandoned_run_reads_as_needing_a_rerun diff --git a/tests/fm-watcher-lock.test.sh b/tests/fm-watcher-lock.test.sh index 8efa1cc3d60..c075fa2e07e 100755 --- a/tests/fm-watcher-lock.test.sh +++ b/tests/fm-watcher-lock.test.sh @@ -452,6 +452,62 @@ wait-refused" ] || fail "a lock in a removed directory was not simply refused: $ pass "a lock whose directory was removed is refused, and waited for, without a steal chain" } +# A holder can release the lock between an acquirer's failed creation and its +# look at what is there. The acquirer then finds no lock at all, which must read +# as free rather than held: a one-shot caller such as the watcher's singleton +# start would otherwise report a holder that no longer exists. +test_lock_released_during_acquisition_is_taken() { + local dir state lockdir driver out rc + dir=$(make_case lock-release-race) + state="$dir/state" + mkdir -p "$state" + lockdir="$state/.contend.lock" + driver="$dir/acquire-during-release.sh" + cat > "$driver" <<'SH' +#!/usr/bin/env bash +. "$1" +lockdir=$2 +signals=$3 +( + fm_lock_acquire_wait "$lockdir" || exit 1 + : > "$signals/held" + while [ ! -e "$signals/release" ]; do sleep 0.05; done + fm_lock_release "$lockdir" + : > "$signals/released" +) & +holder=$! +while [ ! -e "$signals/held" ]; do sleep 0.05; done +eval "real_$(declare -f fm_lock_try_create)" +attempts=0 +fm_lock_try_create() { + local rc=0 + attempts=$((attempts + 1)) + real_fm_lock_try_create "$@" || rc=$? + if [ "$attempts" -eq 1 ]; then + : > "$signals/release" + while [ ! -e "$signals/released" ]; do sleep 0.05; done + fi + return "$rc" +} +if fm_lock_try_acquire "$lockdir"; then echo acquired; else echo "refused holder=${FM_LOCK_HELD_PID:-none}"; fi +wait "$holder" +echo "attempts=$attempts" +if [ "$(cat "$lockdir/pid" 2>/dev/null)" = "$$" ]; then echo owned; fi +fm_lock_release "$lockdir" +if [ -e "$lockdir" ] || [ -L "$lockdir" ]; then echo leaked; fi +SH + mkdir -p "$dir/signals" + rc=0 + out=$(FM_STATE_OVERRIDE="$state" bash -c '. "$1"; fm_run_timed 10 bash "$2" "$3" "$4" "$5"' \ + _ "$ROOT/bin/fm-timeout-lib.sh" "$driver" "$LIB" "$lockdir" "$dir/signals" 2>&1) || rc=$? + [ "$rc" -ne 124 ] || fail "acquiring a lock released mid-acquisition never returned" + [ "$rc" -eq 0 ] || fail "acquiring a lock released mid-acquisition crashed (rc=$rc): $out" + [ "$out" = "acquired +attempts=2 +owned" ] || fail "a lock released mid-acquisition was not taken with one bounded retry: $out" + pass "a lock released while an acquirer was losing to it is taken with one bounded retry" +} + test_watch_restart_rejects_reused_pid() { local dir state fakebin out live pid i dir=$(make_case restart-reused-pid) @@ -1147,6 +1203,7 @@ test_lock_empty_pid_uses_minimum_grace test_lock_late_claim_loses_after_recreate test_lock_paused_mid_acquire_claim_fails_during_steal test_lock_in_removed_directory_is_refused_without_stealing +test_lock_released_during_acquisition_is_taken test_watch_restart_rejects_reused_pid test_watch_restart_attaches_to_healthy_peer test_watcher_self_evicts_on_lock_takeover