fix(lease): the directory lock names its holder; broken only when the holder is dead or its pid reused, unlock removes only its own lock, touch exits 3 under a live holder

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

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 <checkout>] [--exclude-ancestry-of <pid>]
 #       print `<pid> <age>m <command>` 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 (`<pid> <start identity>`): 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

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

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();