Repository navigation
Never pad a file that shrank under a mount with zeros - #186
Conversation
…eros A mount of a folder lists each file once and reads its bytes live. A file that shrank in the folder since (a copy onto it truncates it first) read short of its listed size, and macOS's NFS client padded the rest with zeros, so copying from a mount onto its own folder wrote zeros into it. A short read is now the file's end, and that one file's entry is asked again (Source::entry_now, Workspace::recheck), so the NFS reply, whose attributes are now taken after the read, and later stats carry the size the folder has now. Errors from a folder name the path. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGETmgGUb8hcSEpppQK
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: a8922bc7e3
ℹ️ 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".
…macOS The macOS NFS client ignores a smaller size in READ post-op attributes, so it zero-filled the rest of the read. The read now fails once with a stale handle error carrying the new size, and the next read sees the file as it is. Rechecked entries are merged into the current listing under its lock, and DirSource::entry_now checks the file's folder stays inside the root. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGETmgGUb8hcSEpppQK
lydakis
changed the title
The macOS client retried the failed read and still zero-filled it. The recheck now drops the file's inode and bumps its folder's time, as an update does for changed files, so reads through the old handle keep failing and a new lookup gets the size now. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGETmgGUb8hcSEpppQK
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 446ff5c679
ℹ️ 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".
A chmod puts the file's source record, with its size and time, in the overlay, which a recheck of the listing alone left stale. Workspace::recheck now takes &mut self and refreshes that record too, keeping the mode. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGETmgGUb8hcSEpppQK
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 38760a2135
ℹ️ 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".
A hard-linked base file keeps its source record in the workspace's links, which the recheck now refreshes too, through either name. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGETmgGUb8hcSEpppQK
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d9ef999cfb
ℹ️ 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".
With a hard link, the recheck moved the inode to the other name, so the old handle kept working and kept the client's old size. Every name now leaves the inode at once, each folder gets a new time, and the inode goes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGETmgGUb8hcSEpppQK
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e2bfc95ca3
ℹ️ 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".
Two rechecks of one file racing could leave the listing with one answer and the path cache with the other; the path cache is now written while the listing's lock is held. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGETmgGUb8hcSEpppQK
Requested by George · project thread
What changed
A mount of a folder lists each file once and then reads its bytes live from the folder. If a file got shorter in the folder after that, a read came back short of the listed size. A copy onto the file does this, for example, because it truncates the file first. The NFS READ reply then said
eof = falseand carried the old cached size, so macOS's NFS client filled the rest with zeros. In George's evaluation, acpfrom a Sonny task's mount onto the mount's own folder zeroed seven real files this way.nfs.rs): when a read comes back short of the cached size, the mount asks the source about that one file again. If the file changed, that read fails withNFS3ERR_STALE.dontshrink. CI showed thateof = trueplus fresh attributes still gave zeros.nfsproc3_readin the vendored nfsserve now takes the attributes after the read.fs.rs): the same recheck runs on a short read, so a laterstatagrees with what the kernel saw. Before,statkept the old size for the attribute TTL. FUSE keeps the inode, since the kernel takes the new size from attributes.Workspace::recheck(path)replaces the cached entry and cached bytes of one base file with what the source says now.Source::entry_now(key)is new. Its default returnsNone, which changes nothing.DirSourceimplements it with onelstat, after checking that the file's folder still resolves inside the root.DirSourcenow name the path (source error: PATH: Permission denied). The evaluation's daemon log had 140 of these with no path.What this doesn't fix:
Frozen copies on the source's disk (next PRs) fix both, along with briefs that name mount paths only.
API changes
sandfs-core: new trait methodSource::entry_now, with a default so existing sources keep compiling, and newWorkspace::recheck.sandfs: no changes to the public API. The Python API is unchanged.Tests
cargo test -p sandfs-core --test workspace recheck:cargo test -p sandfs --lib shrank: an NFS read of a 24-byte file shrunk to 3 bytes fails, keeps failing through the old id, the folder's time moves, and a fresh lookup gives size 3 and reads"new". With a hard link, both names look up to the same new inode.SANDFS_REQUIRE_FUSE=1 cargo test -p sandfs-cli --test mount shrank: an end-to-end mount test.statstayed at 24, and passes after.cargo test --workspacepasses here, except the twosandfs shelltests that need bubblewrap, which isn't installed in this container.🤖 Generated with Claude Code
https://claude.ai/code/session_013jkdGETmgGUb8hcSEpppQK