Skip to content

Lock the mandate record with flock (work-system 1.28.4, pr-flow 1.8.2) - #95

Merged
gering merged 1 commit into
mainfrom
task/flock-round-lock
Oct 30, 2026
Merged

gering merged 1 commit into
mainfrom
task/flock-round-lock

Conversation

@gering

@gering gering commented Oct 27, 2026

Copy link
Copy Markdown
Owner

Why

test_mandate.py's concurrent round test flaked in CI: all 8 bookings reported success, yet one was lost. The cause is a real race in the record lock, which has been on main since #91. Breaking an "abandoned" mkdir lock is ABA:

  1. B reads A's pid from the lock.
  2. A releases the lock and exits; C takes a new one.
  3. kill -0 on A's pid fails, so B judges the lock stale and deletes it. That lock is now C's.
  4. B and C both write, and a booking is lost.

A directory cannot be deleted conditionally, so no staleness rule fixes this.

What

  • mandate.sh locks the record writers (round, reviewed, fixed, acknowledged) with flock(2) on .MANDATE.lock. Bash holds fd 9 and python3 -I locks it; this is the same fcntl.flock pattern as followups.py. macOS has no flock(1).
  • The kernel frees the lock when its owner dies, so the pid/age staleness rule is gone.
  • git is wrapped to close fd 9, so a daemon git spawns (fsmonitor) cannot inherit and hold the lock.
  • The lock file stays in place by design (deleting a flock file reopens the race). The .MANDATE.* exclude covers it.
  • Refused with exit 4 and a recovery hint: an old 1.28.x lock directory, a symlink or FIFO at the lock path, and a host without a working python3. No write ever happens unlocked.

Evidence

16 concurrent bookings per run: 1 of 40 runs lost a booking on 1.28.2, 0 of 160 after this change.

Tests

New tests cover a live holder (waited for, then exit 4, nothing booked), a holder killed with kill -9 (freed with no rule), and the fd 9 leak into git. The leak test uses a PATH shim; it fails with the wrapper removed. Further tests cover the refusals for an old directory, a symlink (not followed) and a FIFO (no hang), and for no python3. The obsolete mkdir shim and stale-pid tests are removed. check-structure.py passes locally.

Version 1.28.4, because #93 holds 1.28.3.

🤖 Generated with Claude Code

https://claude.ai/code/session_013pcrPYhUMWQ1gkrrqtSmic

@gering
gering force-pushed the task/flock-round-lock branch from f926fd1 to 0ffb351 Compare October 27, 2026 14:01
- Replace the mkdir lock with flock(2) on .MANDATE.lock: breaking an
  "abandoned" mkdir lock could delete a live booker's lock (ABA), so a
  concurrent round booking was lost (test_mandate CI flake)
- The kernel frees the lock when its owner dies; the pid/age staleness
  rule is gone. python3 takes the lock (macOS has no flock(1)); without
  it a write is refused with exit 4
- git calls close the lock descriptor so no daemon can inherit it
- An old lock directory is refused with the rm -rf to run
- A filesystem that refuses flock is named, not reported as contention
- Every writer re-ensures the .MANDATE.* exclude; close-request counts
  the persistent .MANDATE.lock as a lane file, so the zero-loss
  auto-close still applies
- Exit-4 docs (continue, pr-flow REVIEW-ROUTING) match the new causes;
  CHANGELOG asks to reload 1.28.2 sessions before a lane books again
- Tests: live holder, killed holder, no fd leak into git, symlink/FIFO/
  old-dir refusals, no python3, refused flock, restored exclude, lock
  file exempt in close-request

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@gering
gering force-pushed the task/flock-round-lock branch from 0ffb351 to 42928e9 Compare October 28, 2026 22:43
@gering gering changed the title Lock the mandate record with flock (work-system 1.28.4) Lock the mandate record with flock (work-system 1.28.4, pr-flow 1.8.2) Oct 28, 2026
@gering
gering merged commit e5bfa2e into main Oct 30, 2026
2 checks passed
@gering
gering deleted the task/flock-round-lock branch October 30, 2026 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant