From ad3f839fc0ef4f93fb4e76040ac87b24ea6e4281 Mon Sep 17 00:00:00 2001 From: garrytan Date: Fri, 16 Oct 2026 23:51:37 +0000 Subject: [PATCH] fix(lease): one directory lock for publish, prune and reclaim; reclaim rereads the record judged stale, so a renewal is never deleted or hidden Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- bin/gstack-lease | 63 ++++++++++++++++++++++++--------------- docs/capy-install.md | 8 +++-- test/gstack-lease.test.ts | 23 ++++++++++++-- 3 files changed, 64 insertions(+), 30 deletions(-) diff --git a/bin/gstack-lease b/bin/gstack-lease index 30b0f1d86..a386604bc 100755 --- a/bin/gstack-lease +++ b/bin/gstack-lease @@ -80,31 +80,44 @@ valid_identity() { *) return 1 ;; esac } -# state FILE PID — alive | dead | stale. Stale needs proof: two well-formed -# identities from the same backend that differ. An identity that cannot be read -# now, a malformed record or a backend change keeps the lease (never reclaim a -# live session on missing evidence). +# state PID RECORD AGE — alive | dead | stale for a lease whose record (first +# line) and age were read once by the caller. Stale needs proof: two +# well-formed identities from the same backend that differ. An identity that +# cannot be read now, a malformed record or a backend change keeps the lease +# (never reclaim a live session on missing evidence). state() { - alive "$2" || { echo dead; return; } - _rec=$(head -1 "$1" 2>/dev/null) - if [ -z "$_rec" ]; then - [ "$(age_min "$1")" -ge 120 ] && echo stale || echo alive + alive "$1" || { echo dead; return; } + if [ -z "$2" ]; then + [ "$3" -ge 120 ] && echo stale || echo alive return fi - _now=$(identity "$2") - if valid_identity "$_rec" && [ -n "$_now" ] && [ "${_rec%% *}" = "${_now%% *}" ] && [ "$_rec" != "$_now" ]; then echo stale + _now=$(identity "$1") + if valid_identity "$2" && [ -n "$_now" ] && [ "${2%% *}" = "${_now%% *}" ] && [ "$2" != "$_now" ]; then echo stale else echo alive fi } -# reclaim FILE RECORD — remove a stale lease without losing a concurrent renewal: -# rename it aside atomically, and put it back (hard link: only if no newer lease -# was written meanwhile) when what was renamed is not the record judged stale. +# The lease directory's lock: touch's publish and prune, and reclaim's +# reread-and-delete, run under it, so a reclaim never deletes a renewal and a +# lease never disappears while it is live. mkdir is atomic everywhere; a lock +# left by a killed process is broken once it is a minute old. +lock() { + _tries=0 + until mkdir "$DIR/.lock" 2>/dev/null; do + _tries=$((_tries + 1)) + if [ "$_tries" -ge 50 ]; then + [ -n "$(find "$DIR/.lock" -maxdepth 0 -mmin +1 2>/dev/null)" ] && rmdir "$DIR/.lock" 2>/dev/null && continue + return 1 + fi + sleep 0.1 + done +} +unlock() { rmdir "$DIR/.lock" 2>/dev/null; } +# reclaim FILE RECORD — delete a stale lease only if, under the lock, it still +# holds the record judged stale; a renewal since then keeps it. reclaim() { - _aside="${1%/*}/.reclaim.${1##*/}.$$" - mv -f "$1" "$_aside" 2>/dev/null || return 1 - if [ "$(head -1 "$_aside" 2>/dev/null)" = "$2" ]; then rm -f "$_aside"; return 0; fi - ln "$_aside" "$1" 2>/dev/null - rm -f "$_aside" + lock || return 1 + if [ "$(head -1 "$1" 2>/dev/null)" = "$2" ]; then rm -f "$1"; unlock; return 0; fi + unlock return 1 } age_min() { # FILE — minutes since its mtime @@ -116,15 +129,17 @@ case "$VERB" in touch) [ -n "$PID" ] || PID=$(gstack_harness_pid "${FROM:-$PPID}") mkdir -p "$DIR" || exit 2 - _tmp="$DIR/.$PID.$$" - identity "$PID" > "$_tmp" && mv -f "$_tmp" "$DIR/$PID" || { rm -f "$_tmp"; exit 2; } - find "$DIR" -mmin +120 -type f -exec rm -f {} + 2>/dev/null + _tmp="$DIR/.$PID.$$" _locked=1 + lock && _locked=0 + identity "$PID" > "$_tmp" && mv -f "$_tmp" "$DIR/$PID" || { rm -f "$_tmp"; [ "$_locked" = 0 ] && unlock; exit 2; } + [ "$_locked" = 0 ] && { find "$DIR" -mmin +120 -type f -exec rm -f {} + 2>/dev/null; unlock; } exit 0 ;; list) for f in "$DIR"/*; do [ -f "$f" ] || continue p=${f##*/} - echo "$p $(age_min "$f")m $(state "$f" "$p")" + _age=$(age_min "$f") + echo "$p ${_age}m $(state "$p" "$(head -1 "$f" 2>/dev/null)" "$_age")" done exit 0 ;; esac @@ -143,10 +158,10 @@ for f in "$DIR"/*; do [ -f "$f" ] || continue p=${f##*/} case "$ANCESTRY" in *" $p "*) continue ;; esac - case "$(state "$f" "$p")" in + _rec=$(head -1 "$f" 2>/dev/null) _age=$(age_min "$f") + case "$(state "$p" "$_rec" "$_age")" in dead) continue ;; stale) - _rec=$(head -1 "$f" 2>/dev/null) _age=$(age_min "$f") if reclaim "$f" "$_rec"; then echo "gstack-lease: reclaimed stale lease $p (${_age}m; recorded '$_rec', pid $p is now $(ps -o comm= -p "$p" 2>/dev/null | head -1) '$(identity "$p")')" >&2 continue diff --git a/docs/capy-install.md b/docs/capy-install.md index ce96b23f9..a49802525 100644 --- a/docs/capy-install.md +++ b/docs/capy-install.md @@ -88,9 +88,11 @@ process, or an identity-less lease from an older gstack past 120 minutes, is stale: `bin/gstack-lease live` reclaims it (`gstack-lease: reclaimed stale lease …`). Stale needs proof: two well-formed identities from the same backend that differ (`ps` is read under `LC_ALL=C TZ=UTC0`). An identity that cannot be -read, a malformed record or a backend change keeps the lease. Reclaiming renames -the lease aside first and restores it when a renewal landed meanwhile, and -`touch` publishes by rename, so a reader never sees a half-written record. `bin/gstack-lease list` shows `alive|dead|stale`, and +read, a malformed record or a backend change keeps the lease. `touch` (publish +by rename, prune) and reclaim (reread, then delete only the record judged +stale) run under one `mkdir` lock in the lease directory, so a renewal is never +deleted and a live lease never disappears from a concurrent scan; a lock left +by a killed process is broken after a minute. `bin/gstack-lease list` shows `alive|dead|stale`, and `GSTACK_UPGRADE_FORCE=1` overrides. ## What the installer does diff --git a/test/gstack-lease.test.ts b/test/gstack-lease.test.ts index a408751a1..485fb1e32 100644 --- a/test/gstack-lease.test.ts +++ b/test/gstack-lease.test.ts @@ -150,7 +150,7 @@ describe('gstack-lease', () => { expect(r.stderr).not.toContain('reclaimed'); }); - test('a renewal that lands while a stale lease is being reclaimed survives (rename aside, then restore)', () => { + test('a renewal that lands between the stale verdict and the reclaim survives: reclaim rereads under the lock', () => { const { dir, leases } = checkout(); const holder = sleeper(); const file = path.join(leases, String(holder)); @@ -159,8 +159,8 @@ describe('gstack-lease', () => { fs.writeFileSync(file, otherStart(fresh.trim()) + '\n'); const renewed = path.join(dir, 'renewed'); fs.writeFileSync(renewed, fresh); - // The first rename-aside of the lease is preceded by the holder's renewal, as if touch ran between the stale verdict and the rename. - const racing = bashEnv(dir, `mv() { case "$*" in *".reclaim."*) command cp "${renewed}" "${file}" ;; esac; command mv "$@"; }\n`); + // The holder renews just before reclaim takes the lock, as if touch ran after the stale verdict. + const racing = bashEnv(dir, `mkdir() { case "$*" in *"/.lock") command cp "${renewed}" "${file}" ;; esac; command mkdir "$@"; }\n`); const r = run(['live', '--root', dir], { BASH_ENV: racing }); expect(r.status, r.stdout + r.stderr).toBe(0); expect(r.stderr).not.toContain('reclaimed'); @@ -168,6 +168,23 @@ describe('gstack-lease', () => { expect(fs.readdirSync(leases).filter((n) => n.startsWith('.'))).toEqual([]); }); + test('touch and reclaim take the directory lock; a lock a killed process left behind is broken after a minute', () => { + const { dir, leases } = checkout(); + fs.mkdirSync(path.join(leases, '.lock'), { recursive: true }); + const old = new Date(Date.now() - 5 * 60_000); + fs.utimesSync(path.join(leases, '.lock'), old, old); + const holder = sleeper(); + const r = run(['touch', '--root', dir, '--pid', String(holder)]); + expect(r.status, r.stdout + r.stderr).toBe(0); + expect(fs.existsSync(path.join(leases, '.lock'))).toBe(false); + expect(fs.readFileSync(path.join(leases, String(holder)), 'utf8')).toMatch(/^(linux|ps) /); + fs.mkdirSync(path.join(leases, '.lock')); + fs.writeFileSync(path.join(leases, String(holder)), otherStart(fs.readFileSync(path.join(leases, String(holder)), 'utf8').trim()) + '\n'); + const held = run(['live', '--root', dir]); + expect(held.status, held.stdout + held.stderr).toBe(0); + expect(fs.existsSync(path.join(leases, String(holder)))).toBe(true); + }, 30_000); + test('a lease without a recorded identity (older gstack) counts while fresh and is stale past the 120-minute window', () => { const { dir, leases } = checkout(); const holder = sleeper();