diff --git a/bin/gstack-lease b/bin/gstack-lease index a386604bc..f1ed99870 100755 --- a/bin/gstack-lease +++ b/bin/gstack-lease @@ -13,7 +13,8 @@ # pid defaults to the harness pid behind this call # (bin/gstack-harness-pid.sh, the same derivation the session marker # uses), or behind --from (a caller whose own parent is short-lived -# passes its parent pid); leases older than 120 min are pruned +# passes its parent pid); leases older than 120 min are pruned. Exits 3 +# and writes nothing when a live process holds the directory lock # gstack-lease live [--root ] [--exclude-ancestry-of ] # print ` m ` per lease held by a live process that # is neither the given pid nor one of its ancestors (the caller's own @@ -30,7 +31,7 @@ # runtime root's .source-path, else ../). A root that is not a git checkout # exits 2 and writes nothing. GSTACK_LEASE_DIR overrides the lease directory # (hermetic tests; a dev checkout's own tests must not lease it). Bash 3.2 -# plus git, ps, find. +# plus git, ps, find, ln, readlink. set -u BIN="$(cd "$(dirname "$0")" && pwd -P)" # shellcheck source=bin/gstack-install-verdict.sh @@ -98,20 +99,39 @@ state() { } # 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. +# lease never disappears while it is live. The lock is a symlink whose target +# names its holder (` `): ln -s creates it and its owner +# in one atomic step. It is broken only when that holder is dead or its pid +# now belongs to another process, never by age, and unlock removes only the +# caller's own lock. A live holder makes lock() give up after +# GSTACK_LEASE_LOCK_TRIES tries 0.1 s apart (default 300). +LOCK="$DIR/.lock" TOKEN="" lock() { + [ -n "$TOKEN" ] || TOKEN="$$ $(identity $$)" _tries=0 - until mkdir "$DIR/.lock" 2>/dev/null; do + until ln -s "$TOKEN" "$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 + LOCK_OWNER=$(readlink "$LOCK" 2>/dev/null) + if [ -n "$LOCK_OWNER" ]; then + case "$(state "${LOCK_OWNER%% *}" "${LOCK_OWNER#* }" 0)" in + dead|stale) break_lock "$LOCK_OWNER"; continue ;; + esac fi + [ "$_tries" -ge "${GSTACK_LEASE_LOCK_TRIES:-300}" ] && return 1 sleep 0.1 done } -unlock() { rmdir "$DIR/.lock" 2>/dev/null; } +# break_lock OWNER — move the lock aside and delete it only if it still names +# OWNER; a lock another process took meanwhile is put back. +break_lock() { + _aside="$DIR/.lock.broken.$$" + rm -f "$_aside" + mv "$LOCK" "$_aside" 2>/dev/null || return 0 + _moved=$(readlink "$_aside" 2>/dev/null) + [ "$_moved" = "$1" ] || ln -s "$_moved" "$LOCK" 2>/dev/null + rm -f "$_aside" +} +unlock() { [ "$(readlink "$LOCK" 2>/dev/null)" = "$TOKEN" ] && rm -f "$LOCK"; return 0; } # 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() { @@ -129,10 +149,11 @@ case "$VERB" in touch) [ -n "$PID" ] || PID=$(gstack_harness_pid "${FROM:-$PPID}") mkdir -p "$DIR" || exit 2 - _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; } + lock || { echo "gstack-lease: the lease directory is locked by live process ${LOCK_OWNER%% *}; lease $PID not renewed" >&2; exit 3; } + _tmp="$DIR/.$PID.$$" + identity "$PID" > "$_tmp" && mv -f "$_tmp" "$DIR/$PID" || { rm -f "$_tmp"; unlock; exit 2; } + find "$DIR" -mmin +120 -type f -exec rm -f {} + 2>/dev/null + unlock exit 0 ;; list) for f in "$DIR"/*; do diff --git a/docs/capy-install.md b/docs/capy-install.md index a49802525..acddc678a 100644 --- a/docs/capy-install.md +++ b/docs/capy-install.md @@ -90,9 +90,11 @@ 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. `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 +stale) run under one lock in the lease directory, a symlink naming its holder's +pid and start identity, so a renewal is never deleted and a live lease never +disappears from a concurrent scan. The lock is broken only when its holder is +dead or its pid was reused, never by age; while a live process holds it, +`touch` exits 3 without writing and a reclaim keeps the lease. `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 485fb1e32..4a34a8468 100644 --- a/test/gstack-lease.test.ts +++ b/test/gstack-lease.test.ts @@ -160,7 +160,7 @@ describe('gstack-lease', () => { const renewed = path.join(dir, 'renewed'); fs.writeFileSync(renewed, fresh); // 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 racing = bashEnv(dir, `ln() { case "$*" in *"/.lock") command cp "${renewed}" "${file}" ;; esac; command ln "$@"; }\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,22 +168,64 @@ 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', () => { + test('a lock whose holder died, or whose pid now belongs to another process, is broken at once', () => { 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)]); + fs.mkdirSync(leases, { recursive: true }); + fs.symlinkSync(`${deadPid()} `, path.join(leases, '.lock')); + const r = run(['touch', '--root', dir, '--pid', String(holder)], { GSTACK_LEASE_LOCK_TRIES: '3' }); 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); + const identity = fs.readFileSync(path.join(leases, String(holder)), 'utf8').trim(); + expect(identity).toMatch(/^(linux|ps) /); + fs.symlinkSync(`${holder} ${otherStart(identity)}`, path.join(leases, '.lock')); + const again = run(['touch', '--root', dir, '--pid', String(holder)], { GSTACK_LEASE_LOCK_TRIES: '3' }); + expect(again.status, again.stdout + again.stderr).toBe(0); + expect(fs.existsSync(path.join(leases, '.lock'))).toBe(false); + }); + + test('a live holder\'s lock is never broken, however old: touch exits 3 without writing and a reclaim keeps the lease', () => { + const { dir, leases } = checkout(); + const owner = sleeper(); + const holder = sleeper(); + expect(run(['touch', '--root', dir, '--pid', String(owner)]).status).toBe(0); + expect(run(['touch', '--root', dir, '--pid', String(holder)]).status).toBe(0); + const ownerToken = `${owner} ${fs.readFileSync(path.join(leases, String(owner)), 'utf8').trim()}`; + const lockPath = path.join(leases, '.lock'); + fs.symlinkSync(ownerToken, lockPath); + const old = new Date(Date.now() - 5 * 60_000); + fs.lutimesSync(lockPath, old, old); + const holderFile = path.join(leases, String(holder)); + const stale = otherStart(fs.readFileSync(holderFile, 'utf8').trim()) + '\n'; + fs.writeFileSync(holderFile, stale); + const t = run(['touch', '--root', dir, '--pid', String(holder)], { GSTACK_LEASE_LOCK_TRIES: '3' }); + expect(t.status, t.stdout + t.stderr).toBe(3); + expect(t.stderr).toContain(`locked by live process ${owner}`); + expect(fs.readFileSync(holderFile, 'utf8')).toBe(stale); + const l = run(['live', '--root', dir], { GSTACK_LEASE_LOCK_TRIES: '3' }); + expect(l.status, l.stdout + l.stderr).toBe(0); + expect(l.stderr).not.toContain('reclaimed'); + expect(l.stdout).toContain(`${holder} `); + expect(fs.existsSync(holderFile)).toBe(true); + expect(fs.readlinkSync(lockPath)).toBe(ownerToken); + }); + + test('unlock removes only its own lock: a lock that changed hands during a reclaim stays', () => { + const { dir, leases } = checkout(); + const owner = sleeper(); + const holder = sleeper(); + expect(run(['touch', '--root', dir, '--pid', String(owner)]).status).toBe(0); + expect(run(['touch', '--root', dir, '--pid', String(holder)]).status).toBe(0); + const ownerToken = `${owner} ${fs.readFileSync(path.join(leases, String(owner)), 'utf8').trim()}`; + const holderFile = path.join(leases, String(holder)); + fs.writeFileSync(holderFile, otherStart(fs.readFileSync(holderFile, 'utf8').trim()) + '\n'); + const lockPath = path.join(leases, '.lock'); + // While reclaim deletes the stale lease under its lock, the lock changes hands. + const swap = bashEnv(dir, `rm() { if [ "$*" = "-f ${holderFile}" ]; then command rm -f "${lockPath}"; command ln -s "${ownerToken}" "${lockPath}"; fi; command rm "$@"; }\n`); + const r = run(['live', '--root', dir, '--exclude-ancestry-of', String(owner)], { BASH_ENV: swap }); + expect(r.stderr).toContain(`reclaimed stale lease ${holder}`); + expect(fs.readlinkSync(lockPath)).toBe(ownerToken); + }); test('a lease without a recorded identity (older gstack) counts while fresh and is stale past the 120-minute window', () => { const { dir, leases } = checkout();