fix(fspy): unify shared memory on a sparse temp file across all platforms - #576
fix(fspy): unify shared memory on a sparse temp file across all platforms#576wan9chi wants to merge 1 commit into
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
4f029ee to
cfb9f15
Compare
fspy benchmarklinuxmacoswindows |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4f029ee6c9
ℹ️ 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".
| io::Error::new(io::ErrorKind::InvalidInput, "shared-memory size exceeds u64") | ||
| })?; | ||
|
|
||
| let backing_dir = temp_dir().join(BACKING_DIR); |
There was a problem hiding this comment.
When the creating process has a relative TMPDIR (supported on Unix), temp_dir() returns that relative path and this join preserves it, despite id being documented as absolute. A process opening the mapping from a different working directory then resolves the identifier elsewhere and fails; the new subprocess test misses this because it changes only the child's temporary-directory variables. Resolve the temporary directory against the creator's current directory before publishing the identifier.
AGENTS.md reference: AGENTS.md:L142-L145
Useful? React with 👍 / 👎.
b0ab415 to
73b40b8
Compare
…orms Codex CLI's and Claude Code's default sandboxes deny the primitives fspy's Unix shared memory was built on: the macOS Seatbelt profile denies `shm_open` (`ipc-posix-shm-write-create`), and both sandboxes block Unix-domain sockets, which the Linux memfd broker depended on. Plain files in the temp directory are writable under both. Replace all three backends with one file-backed implementation attached by path: a sparse temporary file plus `memmap2`, with the file's absolute path as the identifier. No broker, no tokio requirement, no global object names. Lifetime semantics converge too: dropping the owner makes the name disappear and later opens fail, while existing views stay usable — now on Windows as well, where closing the delete-on-close handle applies the delete disposition even while other processes hold views. Refs #563. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
73b40b8 to
941a231
Compare
1 participant
Motivation
Codex CLI's and Claude Code's default sandboxes deny the primitives fspy's Unix shared memory was built on: the macOS Seatbelt profile denies
shm_open(ipc-posix-shm-write-create, #563), and both sandboxes block Unix-domain sockets, which the Linux memfd broker depended on. Plain files in the temp directory are writable under both sandboxes — the existing IPC lock file is already created intemp_dir()successfully before the shm/socket step fails.The Windows backend already backed its mapping with a sparse temp file and differed only in how other processes attach (a named section). This replaces all three backends with one file-backed implementation attached by path:
std::fs::Fileplusmemmap2, with the file's absolute path as the identifier. One implementation, one set of lifetime semantics, no broker, no tokio requirement, no global object names. The platform-specific parts shrink to four short passages: the creation flags,FSCTL_SET_SPARSEon NTFS,mode(0o600)on Unix, and how the owner's cleanup removes the name.Lifetime semantics converge as a result: dropping the owner makes the name disappear and later opens fail, while existing views stay usable — now on Windows as well, where closing the delete-on-close handle applies the delete disposition even while other processes hold views, because those are section references rather than handles. Crash cleanup is strictly better on Windows, where the kernel closing handles deletes the file; on Unix a crashed runner leaves a sparse temp file for the standard reapers.
One platform detail did not survive contact with CI and is worth a reviewer's eye: Windows maps the file itself rather than through
memmap2, becausememmap2duplicates the file handle and holds it for the mapping's lifetime, andFILE_FLAG_DELETE_ON_CLOSEonly fires once every handle is closed. Withmemmap2on both sides, a process that merely opened a view kept the owner's cleanup from taking effect. Mapping the section directly and keeping only the view restores the semantics above.Refs #563.
🤖 Generated with Claude Code