fix(posixage/flock): close ghost-lock race via inode verify + heartbeat

[Benehiko]

Closes #530.

Summary

The posixage flock layer had a race where two callers could simultaneously believe they held the exclusive lock. Stale-recovery unlinked the lock file while another process still held flock(2) on the now-orphaned inode, then created a fresh inode at the same path and flocked that.

The race has two halves:

• Acquirer side — a caller mid-acquisition can have its fd flock the inode that recovery just unlinked, while the path now points to a fresh inode held by someone else.
• Holder side — a caller already holding the lock can have its critical section exceed the 30s stale threshold, at which point another process classifies it as abandoned and hijacks it.

This PR closes both halves and fixes a TOCTOU in recoverStaleLock between its Stat and Remove calls. There are now two commits:

1. Inode verification on acquire — after a successful flock, verify (via os.SameFile) that the locked descriptor still refers to the file at the lock-file path. If not, drop the bad lock and retry. This closes the acquirer-side half and the recovery TOCTOU.
2. Modtime heartbeat for holders — a background goroutine started by tryLock re-truncates the lock file every 10s and re-verifies the locked inode each tick. This closes the holder-side half by keeping the lock file young enough that concurrent recovery callers never see it as stale.

The bug

recoverStaleLock deletes the lock file based purely on its modification time being >30s old:

1. Process A holds flock on inode₁ (legitimate, slow operation).
2. Process B sees a stale modtime, calls root.Remove(lockFileName). On Unix this unlinks the dirent but keeps the inode alive for any open fd — A's flock is still kernel-valid.
3. Process B opens the path again (gets a brand-new inode₂), flocks it, and returns success.
4. A and B now both run inside what they think is an exclusive section.

The 30s threshold was always a kludge: flock is automatically released by the kernel on process death, so a held flock by definition belongs to a live holder. Removing the file does nothing to that holder; it only creates the inode-swap hazard.

A secondary bug lived in the recovery TOCTOU: two callers reaching recoverStaleLock concurrently both saw a stale modtime, both called Remove, the loser got ENOENT, and tryLock surfaced that as a hard lock-acquisition failure even though recovery had succeeded.

Fix part 1 — inode verification on acquire

acquireOnce(root, exclusive) runs the full open → flock → truncate → verify sequence atomically:

fl, _ := openFile(root)
if err := lockFile(fl.Fd(), exclusive); err != nil {
    fl.Close(); return nil, err
}
fl.Truncate(0)                              // refresh modtime
same, err := isCurrentLockFile(fl, root)    // os.SameFile(fl.Stat(), root.Stat(lockFileName))
if err != nil || !same {
    releaseLock(fl); fl.Close()
    return nil, errStaleInode               // or err
}
return fl, nil

isCurrentLockFile returns false when the path resolves to a different inode than the locked fd, or when the path no longer exists. Either condition means a concurrent recovery unlinked the file between our open and our flock — we drop the bad lock and let the retry loop start over against whatever is now at the path. retryLock now loops acquireOnce instead of retrying flock against a stale fd, so each retry gets a fresh descriptor.

recoverStaleLock lost its *os.File parameter (it now works purely by path) and treats ENOENT on both Stat and Remove as success — closing the TOCTOU window without a mutex.

To make the release-without-close step possible, releaseLock was split out from unlockFile on both Unix and Windows. unlockFile still releases + closes, so external callers' contracts are unchanged.

Fix part 2 — modtime heartbeat for holders

tryLock now starts a goroutine on successful acquisition that re-truncates the lock file every heartbeatInterval (10s) until unlock. Each tick also re-verifies the locked descriptor's inode against the path; a mismatch indicates the heartbeat itself was starved past staleThreshold and the lock has been hijacked, which is logged via log/slog.

The UnlockFunc returned to the caller cancels the goroutine's context and waits for it to exit before closing the fd, so the heartbeat can never operate on a closed descriptor.

heartbeatInterval (10s) and staleThreshold (30s) are package-private vars rather than consts so tests can shorten them. Production callers cannot tune them; the 3× safety ratio is fixed.

Hijack detection without in-band failure

When the heartbeat detects an inode mismatch, the holder's outstanding operation has no way to be cancelled in-band — UnlockFunc is the only callback we hold. The mismatch is therefore logged but the holder keeps running. This is a deliberate trade-off: the heartbeat fires every 10s vs. a 30s threshold, so on a non-pathological system the hijack window is effectively closed. The log line is the safety net for SIGSTOP / extreme GC pause / scheduler starvation.

Test plan

• go test ./store/posixage/... -race -count=1 passes
• go vet ./store/posixage/... clean
• golangci-lint run ./store/posixage/... reports 0 issues
• flock_race_test.go covers:
  • isCurrentLockFile correctly identifies an unlinked path
  • isCurrentLockFile correctly identifies inode replacement
  • Two concurrent recoverStaleLock calls both return success (TOCTOU fix)
  • Heartbeat keeps a long-running holder live (recovery refuses to fire after several stale-thresholds with the lock held)
• Existing TestFlock and TestRecoverLock suites still pass

What was dropped during review

• Two t.Skip() placeholder tests that documented the now-fixed holder-side limitations.
• A "two waiters cannot both acquire after recovery" test that used a live raw-flock holder. With a live holder, flock on the unlinked orphan always returns EWOULDBLOCK, so the test never exercised the ghost-lock path it claimed to cover. It gave false confidence and is removed rather than left as theatre.

🤖 Generated with Claude Code

[docker-agent]

Assessment: 🔴 CRITICAL

Note: Verifier returned an empty response (anti-loop rule applied). Findings below are from the drafter, manually reviewed against source. High-confidence issues only are reported.

This PR correctly fixes the core ghost-lock race on the acquiring side using /. However, there is a residual TOCTOU window in itself and a silently swallowed truncate error that can re-expose a valid lock to stale-recovery removal.