Skip to content

fix(fspy): replace the IPC file lock with an in-mapping close gate - #577

Draft
wan9chi wants to merge 1 commit into
mainfrom
fspy-close-gate
Draft

fix(fspy): replace the IPC file lock with an in-mapping close gate#577
wan9chi wants to merge 1 commit into
mainfrom
fspy-close-gate

Conversation

@wan9chi

@wan9chi wan9chi commented Jul 28, 2026

Copy link
Copy Markdown
Member

Motivation

The current quiescence protocol attaches "may write" to a shared mapping but "is still writing" to a file-lock descriptor. A descendant that closes descriptors it does not recognize releases the lock while it keeps write access to the mapping, so the receiver can read frames while a straggler mutates them. The #544 investigation pinned this as the root cause.

This PR replaces the lock with a gate stored in the shared memory itself, where a writer cannot lose it while it can still write. One atomic word admits and counts each claim. The runner closes with one fetch_or at root-process exit, which fences all future claims and reports whether a write was still in flight. A zero count proves every claim ran to completion: the memory is frozen, and the runner reads and caches. A nonzero count means the run is not cached. In-flight windows last microseconds and the runner closes milliseconds after wait returns, so a nonzero count is rare. The closed bit and the count share one word on purpose; with two atomics, a writer could check the flag before the close and publish its count after it, which is the file lock's race in new clothes.

The close drops the shared memory's keeper, so the attach window ends with the close: a process that starts later fails at open, and a sender that attached earlier has its claims refused. On Linux, the seccomp supervisor's stop also returns without waiting: each listener task hands over what it recorded and a detached task keeps answering notifications, so a live filtered process keeps working while its later accesses go unrecorded. Draining those listeners to EOF used to mean waiting for every filtered descendant to exit, which reintroduced the daemon hang this PR removes.

A process may also call exit while another of its threads is mid-send, which would abandon a gate guard and mark the whole run incomplete. The preload now interposes exit and _exit and waits the few microseconds until in-flight sends finish before the process dies. Signals and crashes still skip this, and the runner handles those by not caching the run. Windows parity is a follow-up.

Semantics change: fspy no longer waits for lingering descendants after the root process exits, and it does not track accesses made after that exit. Collecting the trace no longer delays vp run; a task that leaks a daemon no longer blocks the read step. This fixes #396, where the receiver's exclusive flock waited on shared flocks inherited by Playwright/Chromium's setsid tree. Stdio pipe draining can still hold a run open and is tracked separately in #485.

Closes #544
Closes #396

🤖 Generated with Claude Code

@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown

fspy benchmark

linux

dynamic/launch             change +155.07%  [+132.20% .. +170.56%]  overhead  +287.00%
dynamic/access             change  +1.10%  [ -0.41% ..  +2.04%]  overhead    +8.81%
dynamic/access-relative    change  +0.93%  [ +0.19% ..  +1.77%]  overhead   +47.11%
static/launch              change +202.22%  [+170.37% .. +221.80%]  overhead  +696.41%
static/access              change  +1.29%  [ +0.14% ..  +2.00%]  overhead  +827.93%
static/access-relative     change  +1.27%  [ +0.48% ..  +1.97%]  overhead +1336.81%

macos

dynamic/launch             change  -0.22%  [ -3.97% ..  +3.90%]  overhead  +225.80%
dynamic/access             change  +2.46%  [ +0.68% ..  +5.23%]  overhead    +7.32%
dynamic/access-relative    change  +0.74%  [ -3.70% ..  +4.58%]  overhead  +264.13%

windows

dynamic/launch             change  -1.29%  [ -4.57% ..  +2.52%]  overhead   +25.87%
dynamic/access             change  -0.19%  [ -1.50% ..  +1.50%]  overhead    +1.51%
dynamic/access-relative    change  +0.00%  [ -2.68% ..  +1.41%]  overhead    +1.12%

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: eb42117817

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread crates/fspy/src/unix/mod.rs
Comment thread CHANGELOG.md Outdated
@wan9chi
wan9chi force-pushed the fspy-close-gate branch 2 times, most recently from 38e7f20 to 5d642d1 Compare July 30, 2026 04:36
@wan9chi
wan9chi changed the base branch from fspy-sparse-file-shm to graphite-base/577 July 30, 2026 06:12
@wan9chi
wan9chi force-pushed the fspy-close-gate branch 2 times, most recently from c0720e2 to 8b4e19a Compare July 31, 2026 07:09
@wan9chi
wan9chi force-pushed the fspy-close-gate branch 2 times, most recently from bbb60ad to 800f15d Compare August 10, 2026 06:59
@wan9chi
wan9chi force-pushed the fspy-close-gate branch 2 times, most recently from 5e9003e to f4e2f6e Compare August 10, 2026 07:26
Base automatically changed from agent/reapply-runner-fifo to main August 10, 2026 07:32
The old quiescence protocol attached "may write" to the shared mapping but
"is still writing" to a file-lock descriptor. A descendant that closes
descriptors it does not recognize released the lock while keeping full
write access to the mapping, so the receiver could read frames while a
straggler was mutating them.

Put the gate in the shared memory itself, where a writer cannot drop it
while still being able to write: one atomic word admits and counts claims,
and the runner's close is a single `fetch_or` at root-process exit that
fences all future claims and reports whether any write was in flight. Zero
in flight proves every admitted claim ran to completion and the memory is
frozen; anything else means the run is conservatively not cached.

Tracking now stops when the root process exits instead of waiting for
lingering descendants, so a task that leaks a daemon no longer blocks the
read step, and post-exit accesses are treated as what they are: racy with
respect to the task's contract.

Closes #544.
Closes #396.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@wan9chi
wan9chi marked this pull request as draft August 10, 2026 07:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant