From a8922bc7e33cd475bc0ab0da6117bba1a2224e11 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 23:35:44 +0000 Subject: [PATCH 1/7] Read a file that shrank under a mount to its end, never padded with zeros 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 Claude-Session: https://claude.ai/code/session_013jkdGETmgGUb8hcSEpppQK --- crates/sandfs-cli/tests/mount.rs | 23 +++++ crates/sandfs-core/src/source.rs | 114 ++++++++++++++------- crates/sandfs-core/src/workspace.rs | 36 +++++++ crates/sandfs-core/tests/workspace.rs | 44 ++++++++ crates/sandfs/src/fs.rs | 27 ++++- crates/sandfs/src/nfs.rs | 39 ++++++- crates/sandfs/src/nfsserve/nfs_handlers.rs | 6 +- docs/changelog.md | 1 + 8 files changed, 251 insertions(+), 39 deletions(-) diff --git a/crates/sandfs-cli/tests/mount.rs b/crates/sandfs-cli/tests/mount.rs index 5457c37..d3e233e 100644 --- a/crates/sandfs-cli/tests/mount.rs +++ b/crates/sandfs-cli/tests/mount.rs @@ -1098,6 +1098,29 @@ fn rereading_a_changed_file_never_returns_cached_bytes() { sandfs.unmount(&ws); } +#[test] +fn a_file_that_shrank_in_the_folder_reads_back_without_zeros() { + if !fuse_available() { + return; + } + let folder = tempfile::tempdir().unwrap(); + std::fs::write(folder.path().join("notes.md"), "twenty-four bytes long..").unwrap(); + let mut sandfs = Sandfs::new(); + let dirs = tempfile::tempdir().unwrap(); + let ws = dirs.path().join("ws"); + sandfs.mount(&["mount", folder.path().to_str().unwrap(), ws.to_str().unwrap()], &ws); + + // Listed with its size but never read, as `ls -l` leaves it; then the folder's + // file shrinks, as a copy onto it truncates it first. The mount used to answer + // with the 3 bytes left padded by zeros to the 24 it had listed. + let file = ws.join("notes.md"); + assert_eq!(std::fs::metadata(&file).unwrap().len(), 24); + std::fs::write(folder.path().join("notes.md"), "new").unwrap(); + assert_eq!(std::fs::read(&file).unwrap(), b"new"); + assert_eq!(std::fs::metadata(&file).unwrap().len(), 3); + sandfs.unmount(&ws); +} + #[test] fn open_files_work_without_open_or_close_reaching_sandfs() { use std::io::{Read, Seek, SeekFrom, Write}; diff --git a/crates/sandfs-core/src/source.rs b/crates/sandfs-core/src/source.rs index d5f8f52..3aa2679 100644 --- a/crates/sandfs-core/src/source.rs +++ b/crates/sandfs-core/src/source.rs @@ -71,7 +71,8 @@ pub type Listing = BTreeMap; /// Implementations must return the same answer for the same key every time; the /// workspace caches results and shares them between forks. The one exception is a /// listing's sizes: after a [`Source::prefetch`], a listing may report sizes it left -/// out before. +/// out before. A source whose files can change underneath it (a folder on disk) can +/// also say what a file is now, through [`Source::entry_now`]. pub trait Source: Send + Sync + fmt::Debug { /// Key of the root directory. fn root(&self) -> Result; @@ -128,6 +129,17 @@ pub trait Source: Send + Sync + fmt::Debug { let _ = key; false } + + /// The file at `key` as the source has it now, for a source whose files can change + /// while a workspace is over it (a folder on disk). The workspace asks only once a + /// read shows a file is no longer what its listing said (see + /// [`crate::Workspace::recheck`]). `None`, the default, keeps the answer the + /// workspace has, as a source whose answers never change should; so does a source + /// that no longer has the file. + fn entry_now(&self, key: &str) -> Result> { + let _ = key; + Ok(None) + } } /// The part of `bytes` a ranged read asks for, clamped to its end. @@ -235,12 +247,12 @@ impl DirSource { if !root.is_dir() { return Err(Error::Source(format!("{} is not a directory", root.display()))); } - Ok(DirSource { root: root.canonicalize().map_err(io_error)? }) + Ok(DirSource { root: root.canonicalize().map_err(|err| io_error(root, err))? }) } /// Fail unless `dir`, with every symlink resolved, is still `root` or inside it. fn check_inside(&self, dir: &Path) -> Result<()> { - if dir.canonicalize().map_err(io_error)?.starts_with(&self.root) { + if dir.canonicalize().map_err(|err| io_error(dir, err))?.starts_with(&self.root) { Ok(()) } else { Err(Error::Source(format!("{} is outside the mounted directory", dir.display()))) @@ -258,8 +270,11 @@ impl DirSource { use std::os::unix::fs::OpenOptionsExt; // A directory-only open fails at once on anything else, such as a fifo // swapped in since the listing, which a plain open would block on. - let opened = - std::fs::OpenOptions::new().read(true).custom_flags(libc::O_DIRECTORY).open(dir).map_err(io_error)?; + let opened = std::fs::OpenOptions::new() + .read(true) + .custom_flags(libc::O_DIRECTORY) + .open(dir) + .map_err(|err| io_error(dir, err))?; if let Ok(path) = std::fs::read_link(format!("/proc/self/fd/{}", opened.as_raw_fd())) { return if path == dir { Ok(()) @@ -283,7 +298,7 @@ impl DirSource { /// reaches on the host; an absolute one outside is kept as written. fn link_target(&self, key: &str) -> Result { let path = self.host_path(key); - let target = std::fs::read_link(&path).map_err(io_error)?; + let target = std::fs::read_link(&path).map_err(|err| io_error(&path, err))?; let parent = Path::new(key).parent().unwrap_or(Path::new("")); let rewritten = match self.named_inside(parent, &target) { Some(_) if !target.has_root() => target.clone(), @@ -364,6 +379,33 @@ impl DirSource { Some(inside.into_iter().collect()) } + /// The entry for the file at `key`, of type `file_type`, with `meta` its metadata + /// (its own for a link, not its target's). `None` for what a workspace doesn't + /// hold: special files, and links whose target no workspace path could hold. + fn entry_of( + &self, + key: String, + file_type: std::fs::FileType, + meta: impl FnOnce() -> std::io::Result, + ) -> Result> { + let (kind, size, modified) = if file_type.is_dir() { + (Kind::Dir, None, None) + } else if file_type.is_file() { + let meta = meta().map_err(|err| io_error(&self.host_path(&key), err))?; + (Kind::File { executable: is_executable(&meta) }, Some(meta.len()), meta.modified().ok()) + } else if file_type.is_symlink() { + let modified = meta().ok().and_then(|meta| meta.modified().ok()); + match self.link_target(&key) { + Ok(target) => (Kind::Symlink, Some(target.len() as u64), modified), + Err(Error::NotText(_)) => return Ok(None), + Err(err) => return Err(err), + } + } else { + return Ok(None); + }; + Ok(Some(SourceEntry { kind, key: key.as_str().into(), size, modified })) + } + fn host_path(&self, key: &str) -> PathBuf { if key.is_empty() { self.root.clone() } else { self.root.join(key) } } @@ -378,7 +420,7 @@ impl DirSource { let path = self.host_path(key); // Checked before opening, so a special file (a fifo) is never opened, and a // link is never followed: its target is read from the folder holding it. - let file_type = path.symlink_metadata().map_err(io_error)?.file_type(); + let file_type = path.symlink_metadata().map_err(|err| io_error(&path, err))?.file_type(); if file_type.is_symlink() { let parent = Path::new(key).parent().and_then(Path::to_str).unwrap_or_default(); self.check_dir(parent, path.parent().unwrap_or(&self.root))?; @@ -387,7 +429,7 @@ impl DirSource { if !file_type.is_file() { return Err(Error::Source(format!("{} is not a regular file", path.display()))); } - let file = std::fs::File::open(&path).map_err(io_error)?; + let file = std::fs::File::open(&path).map_err(|err| io_error(&path, err))?; // On Linux the kernel names the file that was actually opened. It must be this // very path: a symlink anywhere on the way would resolve to another one. That // is one call, where resolving the parent costs one per component. @@ -424,46 +466,46 @@ impl Source for DirSource { fn read_dir(&self, key: &str) -> Result { let dir = self.host_path(key); self.check_dir(key, &dir)?; - let entries = std::fs::read_dir(dir).map_err(io_error)?; + let entries = std::fs::read_dir(&dir).map_err(|err| io_error(&dir, err))?; let mut listing = Listing::new(); for entry in entries { - let entry = entry.map_err(io_error)?; + let entry = entry.map_err(|err| io_error(&dir, err))?; let Ok(name) = entry.file_name().into_string() else { continue; // non-UTF-8 names cannot be addressed by a workspace path }; // The entry's type usually comes with the listing, so only files, which // need a size and a mode, cost a stat (relative to the open directory). - let file_type = entry.file_type().map_err(io_error)?; + let file_type = entry.file_type().map_err(|err| io_error(&entry.path(), err))?; let child_key = if key.is_empty() { name.clone() } else { format!("{key}/{name}") }; - let (kind, size, modified) = if file_type.is_dir() { - (Kind::Dir, None, None) - } else if file_type.is_file() { - let meta = entry.metadata().map_err(io_error)?; - (Kind::File { executable: is_executable(&meta) }, Some(meta.len()), meta.modified().ok()) - } else if file_type.is_symlink() { - // The listing's metadata is the link's own, not its target's. - let modified = entry.metadata().ok().and_then(|meta| meta.modified().ok()); - match self.link_target(&child_key) { - Ok(target) => (Kind::Symlink, Some(target.len() as u64), modified), - Err(Error::NotText(_)) => continue, // a target no workspace path could hold - Err(err) => return Err(err), - } - } else { - continue; // special files are not part of a workspace - }; - listing.insert(name, SourceEntry { kind, key: child_key.as_str().into(), size, modified }); + if let Some(found) = self.entry_of(child_key, file_type, || entry.metadata())? { + listing.insert(name, found); + } } Ok(listing) } + fn entry_now(&self, key: &str) -> Result> { + if key.is_empty() || !Path::new(key).components().all(|part| matches!(part, Component::Normal(_))) { + return Ok(None); + } + let path = self.host_path(key); + let meta = match path.symlink_metadata() { + Ok(meta) => meta, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(None), + Err(err) => return Err(io_error(&path, err)), + }; + self.entry_of(key.to_owned(), meta.file_type(), || Ok(meta.clone())) + } + fn read_file(&self, key: &str) -> Result { let mut file = match self.open_file(key)? { Opened::File(file) => file, Opened::Link(target) => return Ok(target), }; - let size = file.metadata().map_err(io_error)?.len(); + let error = |err| io_error(&self.host_path(key), err); + let size = file.metadata().map_err(error)?.len(); let mut data = Vec::with_capacity(usize::try_from(size).unwrap_or(0)); - file.read_to_end(&mut data).map_err(io_error)?; + file.read_to_end(&mut data).map_err(error)?; Ok(Bytes::from(data)) } @@ -472,10 +514,11 @@ impl Source for DirSource { Opened::File(file) => file, Opened::Link(target) => return Ok(slice(&target, offset, len)), }; - let available = file.metadata().map_err(io_error)?.len().saturating_sub(offset); - file.seek(SeekFrom::Start(offset)).map_err(io_error)?; + let error = |err| io_error(&self.host_path(key), err); + let available = file.metadata().map_err(error)?.len().saturating_sub(offset); + file.seek(SeekFrom::Start(offset)).map_err(error)?; let mut data = Vec::with_capacity(usize::try_from(len.min(available)).unwrap_or(0)); - file.take(len).read_to_end(&mut data).map_err(io_error)?; + file.take(len).read_to_end(&mut data).map_err(error)?; Ok(Bytes::from(data)) } } @@ -506,8 +549,9 @@ fn relative_link(from: &Path, to: &Path) -> PathBuf { if link.as_os_str().is_empty() { PathBuf::from(".") } else { link } } -fn io_error(err: std::io::Error) -> Error { - Error::Source(err.to_string()) +/// An error from the disk at `path`, naming it, so a log of failures says where. +fn io_error(path: &Path, err: std::io::Error) -> Error { + Error::Source(format!("{}: {err}", path.display())) } #[cfg(unix)] diff --git a/crates/sandfs-core/src/workspace.rs b/crates/sandfs-core/src/workspace.rs index 7550234..3f46450 100644 --- a/crates/sandfs-core/src/workspace.rs +++ b/crates/sandfs-core/src/workspace.rs @@ -101,6 +101,11 @@ impl Cache { self.dirs.lock().expect("cache lock").insert(key, Arc::new(Mutex::new(Some(listing)))); } + /// Drop the cached blob for `key`, whose file changed in its source. + fn forget_blob(&self, key: &CacheKey) { + self.blobs.lock().expect("cache lock").remove(key); + } + /// The cached blob for `key`, if it is loaded and not being loaded right now. fn peek_blob(&self, key: &CacheKey) -> Option { let slot = self.blobs.lock().expect("cache lock").get(key)?.clone(); @@ -409,6 +414,37 @@ impl Workspace { }) } + /// Ask the source again about the file at `path`, which this workspace hasn't + /// changed, after a read showed it is no longer what its listing said: a read that + /// came back shorter than the listed size, from a folder written to meanwhile. Its + /// size, time and cached bytes become what the source says now (see + /// [`Source::entry_now`]), for this file alone; the rest of its directory stays as + /// first listed. Returns whether anything changed. Forks share what was asked, + /// as they share every listing. + pub fn recheck(&self, path: &str) -> Result { + let path = VPath::parse(path)?; + let Ok(Resolved::File { content: Content::Base { mount, key, .. }, .. }) = self.resolve(&path) else { + return Ok(false); + }; + let (Some(parent), Some(name)) = (path.parent(), path.name()) else { return Ok(false) }; + let Some(dir) = self.base_dir(&parent, &mount)? else { return Ok(false) }; + let listing = self.cached_listing(&mount, &dir)?; + // A file moved here from another directory isn't in this listing. + let Some(listed) = listing.get(name).filter(|entry| entry.key == key) else { return Ok(false) }; + let Some(now) = mount.source.entry_now(&key)? else { return Ok(false) }; + if *listed == now { + return Ok(false); + } + let mut fresh = (*listing).clone(); + fresh.insert(name.to_owned(), now.clone()); + self.cache.replace_listing((mount.id, dir), Arc::new(fresh)); + self.cache.forget_blob(&(mount.id, key)); + if let Some((mount_path, _)) = self.mounts.iter().find(|(_, owner)| owner.id == mount.id) { + self.cache.remember_entry(mount.id, &path.as_str()[mount_path.as_str().len() + 1..], now); + } + Ok(true) + } + /// [`Metadata::modified`] alone: the source's time for a file nobody wrote, found /// without reading the file even when the source left its size out. pub fn modified(&self, path: &str) -> Result> { diff --git a/crates/sandfs-core/tests/workspace.rs b/crates/sandfs-core/tests/workspace.rs index 74f45bb..825bffa 100644 --- a/crates/sandfs-core/tests/workspace.rs +++ b/crates/sandfs-core/tests/workspace.rs @@ -1003,6 +1003,50 @@ fn dir_source_reads_ranges_from_disk() { std::fs::remove_dir_all(&dir).unwrap(); } +#[test] +fn a_recheck_takes_a_changed_folder_file_as_it_is_now() { + let dir = std::env::temp_dir().join(format!("sandfs-recheck-{}", std::process::id())); + std::fs::create_dir_all(dir.join("sub")).unwrap(); + std::fs::write(dir.join("sub/shrink.txt"), "twenty-four bytes long..").unwrap(); + std::fs::write(dir.join("sub/other.txt"), "other").unwrap(); + std::fs::write(dir.join("sub/mine.txt"), "mine").unwrap(); + + let mut ws = Workspace::new(); + ws.mount("/host", Arc::new(DirSource::new(&dir).unwrap())).unwrap(); + assert_eq!(ws.stat("/host/sub/shrink.txt").unwrap().size, 24); + assert_eq!(ws.read_text("/host/sub/other.txt").unwrap(), "other"); + ws.write("/host/sub/mine.txt", "written here").unwrap(); + assert!(!ws.recheck("/host/sub/shrink.txt").unwrap(), "nothing changed yet"); + + // The folder changes under the workspace; the listing still says 24 bytes. + std::fs::write(dir.join("sub/shrink.txt"), "new").unwrap(); + std::fs::write(dir.join("sub/other.txt"), "other, longer now").unwrap(); + std::fs::write(dir.join("sub/mine.txt"), "changed on disk").unwrap(); + std::fs::write(dir.join("sub/added.txt"), "added").unwrap(); + assert_eq!(ws.stat("/host/sub/shrink.txt").unwrap().size, 24); + assert_eq!(ws.read_range("/host/sub/shrink.txt", 0, 24).unwrap(), Bytes::from_static(b"new")); + + assert!(ws.recheck("/host/sub/shrink.txt").unwrap()); + assert_eq!(ws.stat("/host/sub/shrink.txt").unwrap().size, 3); + assert_eq!(ws.read_text("/host/sub/shrink.txt").unwrap(), "new"); + assert!(!ws.recheck("/host/sub/shrink.txt").unwrap(), "asked once per change"); + // Only that file: the rest of the folder stays as first seen, cached bytes included. + assert_eq!(ws.read_text("/host/sub/other.txt").unwrap(), "other"); + assert!(!ws.exists("/host/sub/added.txt")); + // A file written here is the workspace's own. + assert!(!ws.recheck("/host/sub/mine.txt").unwrap()); + assert_eq!(ws.read_text("/host/sub/mine.txt").unwrap(), "written here"); + // A file of a cached whole read is read again once rechecked. + assert!(ws.recheck("/host/sub/other.txt").unwrap()); + assert_eq!(ws.read_text("/host/sub/other.txt").unwrap(), "other, longer now"); + // A forked snapshot sees the same answer, as it shares every listing. + assert_eq!(ws.fork().stat("/host/sub/shrink.txt").unwrap().size, 3); + // Gone from the folder: kept as it was. + std::fs::remove_file(dir.join("sub/shrink.txt")).unwrap(); + assert!(!ws.recheck("/host/sub/shrink.txt").unwrap()); + std::fs::remove_dir_all(&dir).unwrap(); +} + /// A slow source that counts file reads and fails the first one when asked to. #[derive(Debug)] struct Slow { diff --git a/crates/sandfs/src/fs.rs b/crates/sandfs/src/fs.rs index 93f2f24..e94b329 100644 --- a/crates/sandfs/src/fs.rs +++ b/crates/sandfs/src/fs.rs @@ -1192,6 +1192,21 @@ impl Inner { self.entry(&path) } + /// Ask the source again about `ino`, a file of the source nobody wrote, after a + /// read of it came back short of its size: the folder under the mount changed + /// since it was listed (see [`Workspace::recheck`]). Its attributes then give the + /// folder's size now, so a client never fills the bytes it lacks with zeros. + /// Returns whether anything changed. + pub fn recheck(&self, ino: u64) -> FsResult { + if self.buffers.contains_key(&ino) { + return Ok(false); + } + match self.nodes.get(&ino) { + Some(Node { linked: true, path, .. }) => self.ws.recheck(path.as_str()).fs(), + _ => Ok(false), + } + } + /// The workspace and path holding `ino`'s contents: the workspace itself, or for /// a removed file the kernel may still have open, the one from before the removal. fn contents(&self, ino: u64) -> FsResult<(&Workspace, &VPath)> { @@ -2087,7 +2102,17 @@ impl Filesystem for SandFs { ) { let reading = self.0.lock().reading(ino.0, offset, size); match reading.and_then(|reading| reading.read(offset, size)) { - Ok(data) => reply.data(&data), + Ok(data) => { + // The kernel takes a short read as the file's end; its size from now on + // says the same (see `Inner::recheck`). + if data.len() < size as usize { + let inner = self.0.lock(); + if inner.attr(ino.0).is_ok_and(|attr| attr.size > offset + data.len() as u64) { + let _ = inner.recheck(ino.0); + } + } + reply.data(&data) + } Err(err) => reply.error(err), } } diff --git a/crates/sandfs/src/nfs.rs b/crates/sandfs/src/nfs.rs index 0e3a780..132e67a 100644 --- a/crates/sandfs/src/nfs.rs +++ b/crates/sandfs/src/nfs.rs @@ -275,8 +275,16 @@ impl NFSFileSystem for SandNfs { (inner.reading(id, offset, count).nfs()?, inner.attr(id).nfs()?.size) }; let data = reading.read(offset, count).nfs()?; - let eof = offset + data.len() as u64 >= size; - Ok((data.to_vec(), eof)) + let end = offset + data.len() as u64; + // Fewer bytes than asked for is the file's end. Short of its size, the file + // shrank in the folder under the mount since it was listed: the attributes this + // reply carries (taken after the read) must say so, or the client fills the rest + // with zeros, and a copy from the mount writes zeros. + let short = data.len() < count as usize; + if short && end < size { + let _ = self.shared.lock().recheck(id); + } + Ok((data.to_vec(), short || end >= size)) } async fn write(&self, id: fileid3, offset: u64, data: &[u8]) -> Result { @@ -926,6 +934,33 @@ mod tests { server.join().unwrap(); } + #[test] + fn a_file_that_shrank_in_the_folder_reads_to_its_end_without_zeros() { + // A copy from the mount onto the folder under it truncates the file there + // first: the mount's listing still says 24 bytes, and a client told so fills + // the rest of a 3-byte read with zeros. + let folder = tempfile::tempdir().unwrap(); + std::fs::write(folder.path().join("notes.md"), "twenty-four bytes long..").unwrap(); + let mut ws = Workspace::new(); + ws.mount("/mnt", Arc::new(sandfs_core::DirSource::new(folder.path()).unwrap())).unwrap(); + let root = VPath::parse("/mnt").unwrap(); + let mut inner = Inner::new(ws, &root, BTreeMap::new(), BTreeSet::new(), UNIX_EPOCH, (501, 20)); + inner.ranged_reads = true; + let dir = tempfile::tempdir().unwrap(); + let fs = SandNfs::new(Shared::new(inner, Saved::new(dir.path()), root)).unwrap(); + block(async { + let id = fs.lookup(ROOT_INO, &name("notes.md")).await.unwrap(); + assert_eq!(fs.getattr(id).await.unwrap().size, 24); + std::fs::write(folder.path().join("notes.md"), "new").unwrap(); + assert_eq!(fs.read(id, 0, 24).await.unwrap(), (b"new".to_vec(), true), "the end, not more to come"); + assert_eq!(fs.getattr(id).await.unwrap().size, 3, "the size the reply carries"); + // A read of part of a file that hasn't changed is not its end. + std::fs::write(folder.path().join("notes.md"), "a longer file now").unwrap(); + let id2 = fs.lookup(ROOT_INO, &name("notes.md")).await.unwrap(); + assert_eq!(fs.read(id2, 0, 2).await.unwrap(), (b"a ".to_vec(), false)); + }); + } + #[test] fn writes_are_read_back_and_saved() { let (fs, _dir) = served(); diff --git a/crates/sandfs/src/nfsserve/nfs_handlers.rs b/crates/sandfs/src/nfsserve/nfs_handlers.rs index d8c91ea..c8085a7 100644 --- a/crates/sandfs/src/nfsserve/nfs_handlers.rs +++ b/crates/sandfs/src/nfsserve/nfs_handlers.rs @@ -356,11 +356,15 @@ pub async fn nfsproc3_read( } let id = id.unwrap(); + // The attributes are taken after the read, which may have found the file + // smaller than it was (sandfs: a folder changed under the mount); with the older + // size, a client fills what it lacks with zeros. + let read = context.vfs.read(id, args.offset, args.count).await; let obj_attr = match context.vfs.getattr(id).await { Ok(v) => nfs::post_op_attr::attributes(v), Err(_) => nfs::post_op_attr::Void, }; - match context.vfs.read(id, args.offset, args.count).await { + match read { Ok((bytes, eof)) => { let res = READ3resok { file_attributes: obj_attr, count: bytes.len() as u32, eof, data: bytes }; make_success_reply(xid).serialize(output)?; diff --git a/docs/changelog.md b/docs/changelog.md index 225a482..8252642 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -2,6 +2,7 @@ ## Unreleased +- **sandfs and sandfs-core, a file that shrank in the folder under a mount**: a mount of a folder lists each file once and reads its bytes from the folder as they are. When a file got shorter in the folder after that (a copy onto it truncates it first), a read came back short of the listed size, and macOS's NFS client filled the rest with zeros: copying files from a mount onto the folder under it wrote zeros into them. A read that comes back short is now the file's end, and the file's size and time are asked again for that file alone, so the reply carries its size now; on Linux, the size a later `stat` gives says the same. In `sandfs-core` this is `Workspace::recheck` and a new `Source::entry_now` with a default that changes nothing, which `DirSource` implements. Errors from a folder now name the path they happened at ("source error: PATH: Permission denied"). - **sandfs and sandfs-cli, real folders on Linux**: `sandfs fork` of a git clone (or of a fork kept as a folder) into a folder on a disk that clones files, btrfs or XFS, now keeps the fork there as a real folder of clones of the clone's files, as on macOS, instead of mounting it. It takes no space for the files, about 30 ms to make, and builds and git run in it at the disk's own speed, where a mount ran a build after an edit 25-40% slower and created small files 2-5 times slower. sandfs finds out by cloning one file next to the folder and removing it. On ext4, or with `--read-only` or `--mount`, the fork is mounted as before. ## 0.8.0 (2026-10-27) From f474f7ce03001790768f1aa7e50949f60dcecff0 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 23:55:41 +0000 Subject: [PATCH 2/7] Fail a short NFS read of a shrunk file instead of zero-filling it on 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 Claude-Session: https://claude.ai/code/session_013jkdGETmgGUb8hcSEpppQK --- crates/sandfs-cli/tests/mount.rs | 16 +++++++++++++++- crates/sandfs-core/src/source.rs | 8 ++++++++ crates/sandfs-core/src/workspace.rs | 25 ++++++++++++++++++++----- crates/sandfs-core/tests/workspace.rs | 12 ++++++++++++ crates/sandfs/src/nfs.rs | 17 ++++++++++------- docs/changelog.md | 2 +- 6 files changed, 66 insertions(+), 14 deletions(-) diff --git a/crates/sandfs-cli/tests/mount.rs b/crates/sandfs-cli/tests/mount.rs index d3e233e..f08285b 100644 --- a/crates/sandfs-cli/tests/mount.rs +++ b/crates/sandfs-cli/tests/mount.rs @@ -1116,7 +1116,21 @@ fn a_file_that_shrank_in_the_folder_reads_back_without_zeros() { let file = ws.join("notes.md"); assert_eq!(std::fs::metadata(&file).unwrap().len(), 24); std::fs::write(folder.path().join("notes.md"), "new").unwrap(); - assert_eq!(std::fs::read(&file).unwrap(), b"new"); + // The macOS client won't take a smaller size from a read, so there the first read + // fails, and the next asks for the size again. A read never gives zeros. + let mut reads = 0; + let read = loop { + reads += 1; + match std::fs::read(&file) { + Ok(read) => break read, + Err(err) if cfg!(target_os = "macos") && reads < 10 => { + eprintln!("read {reads}: {err}"); + std::thread::sleep(std::time::Duration::from_millis(500)); + } + Err(err) => panic!("read {reads}: {err}"), + } + }; + assert_eq!(read, b"new"); assert_eq!(std::fs::metadata(&file).unwrap().len(), 3); sandfs.unmount(&ws); } diff --git a/crates/sandfs-core/src/source.rs b/crates/sandfs-core/src/source.rs index 3aa2679..68661d1 100644 --- a/crates/sandfs-core/src/source.rs +++ b/crates/sandfs-core/src/source.rs @@ -488,7 +488,15 @@ impl Source for DirSource { if key.is_empty() || !Path::new(key).components().all(|part| matches!(part, Component::Normal(_))) { return Ok(None); } + // The folder holding it is checked as a listing checks it: one swapped for a + // link since must not lead outside. + let parent = Path::new(key).parent().and_then(Path::to_str).unwrap_or_default(); let path = self.host_path(key); + let dir = path.parent().unwrap_or(&self.root); + if !dir.is_dir() { + return Ok(None); + } + self.check_dir(parent, dir)?; let meta = match path.symlink_metadata() { Ok(meta) => meta, Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(None), diff --git a/crates/sandfs-core/src/workspace.rs b/crates/sandfs-core/src/workspace.rs index 3f46450..a059faa 100644 --- a/crates/sandfs-core/src/workspace.rs +++ b/crates/sandfs-core/src/workspace.rs @@ -106,6 +106,22 @@ impl Cache { self.blobs.lock().expect("cache lock").remove(key); } + /// Put `entry` in the cached listing `key` under `name`, in place of the entry with + /// the same source key, and say whether that changed it. The listing's own lock is + /// held throughout, so two files of one directory changed at once both stay changed. + fn patch_listing(&self, key: &CacheKey, name: &str, entry: SourceEntry) -> bool { + let Some(slot) = self.dirs.lock().expect("cache lock").get(key).cloned() else { return false }; + let mut listing = slot.lock().expect("cache slot lock"); + let Some(current) = listing.as_ref() else { return false }; + if current.get(name).is_none_or(|listed| listed.key != entry.key || *listed == entry) { + return false; + } + let mut fresh = (**current).clone(); + fresh.insert(name.to_owned(), entry); + *listing = Some(Arc::new(fresh)); + true + } + /// The cached blob for `key`, if it is loaded and not being loaded right now. fn peek_blob(&self, key: &CacheKey) -> Option { let slot = self.blobs.lock().expect("cache lock").get(key)?.clone(); @@ -430,14 +446,13 @@ impl Workspace { let Some(dir) = self.base_dir(&parent, &mount)? else { return Ok(false) }; let listing = self.cached_listing(&mount, &dir)?; // A file moved here from another directory isn't in this listing. - let Some(listed) = listing.get(name).filter(|entry| entry.key == key) else { return Ok(false) }; + if listing.get(name).is_none_or(|entry| entry.key != key) { + return Ok(false); + } let Some(now) = mount.source.entry_now(&key)? else { return Ok(false) }; - if *listed == now { + if !self.cache.patch_listing(&(mount.id, dir), name, now.clone()) { return Ok(false); } - let mut fresh = (*listing).clone(); - fresh.insert(name.to_owned(), now.clone()); - self.cache.replace_listing((mount.id, dir), Arc::new(fresh)); self.cache.forget_blob(&(mount.id, key)); if let Some((mount_path, _)) = self.mounts.iter().find(|(_, owner)| owner.id == mount.id) { self.cache.remember_entry(mount.id, &path.as_str()[mount_path.as_str().len() + 1..], now); diff --git a/crates/sandfs-core/tests/workspace.rs b/crates/sandfs-core/tests/workspace.rs index 825bffa..6482faa 100644 --- a/crates/sandfs-core/tests/workspace.rs +++ b/crates/sandfs-core/tests/workspace.rs @@ -1044,6 +1044,18 @@ fn a_recheck_takes_a_changed_folder_file_as_it_is_now() { // Gone from the folder: kept as it was. std::fs::remove_file(dir.join("sub/shrink.txt")).unwrap(); assert!(!ws.recheck("/host/sub/shrink.txt").unwrap()); + // Its folder swapped for a link out of the folder: nothing there is looked at. + let outside = std::env::temp_dir().join(format!("sandfs-recheck-outside-{}", std::process::id())); + std::fs::create_dir_all(&outside).unwrap(); + std::fs::write(outside.join("other.txt"), "outside").unwrap(); + std::fs::rename(dir.join("sub"), dir.join("moved")).unwrap(); + #[cfg(unix)] + { + std::os::unix::fs::symlink(&outside, dir.join("sub")).unwrap(); + assert!(ws.recheck("/host/sub/other.txt").is_err()); + assert_eq!(ws.stat("/host/sub/other.txt").unwrap().size, 17); + } + std::fs::remove_dir_all(&outside).unwrap(); std::fs::remove_dir_all(&dir).unwrap(); } diff --git a/crates/sandfs/src/nfs.rs b/crates/sandfs/src/nfs.rs index 132e67a..5476e71 100644 --- a/crates/sandfs/src/nfs.rs +++ b/crates/sandfs/src/nfs.rs @@ -277,12 +277,14 @@ impl NFSFileSystem for SandNfs { let data = reading.read(offset, count).nfs()?; let end = offset + data.len() as u64; // Fewer bytes than asked for is the file's end. Short of its size, the file - // shrank in the folder under the mount since it was listed: the attributes this - // reply carries (taken after the read) must say so, or the client fills the rest - // with zeros, and a copy from the mount writes zeros. + // shrank in the folder under the mount since it was listed, and the client, + // which still has the old size, fills the rest with zeros: a copy from the + // mount would write zeros. The macOS client won't take a smaller size from a + // READ reply, so this read fails instead; its reply carries the new size (taken + // after the read), and the client asks again before its next look at the file. let short = data.len() < count as usize; - if short && end < size { - let _ = self.shared.lock().recheck(id); + if short && end < size && self.shared.lock().recheck(id).unwrap_or(false) { + return Err(nfsstat3::NFS3ERR_STALE); } Ok((data.to_vec(), short || end >= size)) } @@ -935,7 +937,7 @@ mod tests { } #[test] - fn a_file_that_shrank_in_the_folder_reads_to_its_end_without_zeros() { + fn a_file_that_shrank_in_the_folder_fails_a_read_then_reads_to_its_end() { // A copy from the mount onto the folder under it truncates the file there // first: the mount's listing still says 24 bytes, and a client told so fills // the rest of a 3-byte read with zeros. @@ -952,8 +954,9 @@ mod tests { let id = fs.lookup(ROOT_INO, &name("notes.md")).await.unwrap(); assert_eq!(fs.getattr(id).await.unwrap().size, 24); std::fs::write(folder.path().join("notes.md"), "new").unwrap(); - assert_eq!(fs.read(id, 0, 24).await.unwrap(), (b"new".to_vec(), true), "the end, not more to come"); + assert!(fs.read(id, 0, 24).await.is_err(), "no zeros where the client expects 24 bytes"); assert_eq!(fs.getattr(id).await.unwrap().size, 3, "the size the reply carries"); + assert_eq!(fs.read(id, 0, 24).await.unwrap(), (b"new".to_vec(), true), "the end, not more to come"); // A read of part of a file that hasn't changed is not its end. std::fs::write(folder.path().join("notes.md"), "a longer file now").unwrap(); let id2 = fs.lookup(ROOT_INO, &name("notes.md")).await.unwrap(); diff --git a/docs/changelog.md b/docs/changelog.md index 8252642..f1a7158 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -2,7 +2,7 @@ ## Unreleased -- **sandfs and sandfs-core, a file that shrank in the folder under a mount**: a mount of a folder lists each file once and reads its bytes from the folder as they are. When a file got shorter in the folder after that (a copy onto it truncates it first), a read came back short of the listed size, and macOS's NFS client filled the rest with zeros: copying files from a mount onto the folder under it wrote zeros into them. A read that comes back short is now the file's end, and the file's size and time are asked again for that file alone, so the reply carries its size now; on Linux, the size a later `stat` gives says the same. In `sandfs-core` this is `Workspace::recheck` and a new `Source::entry_now` with a default that changes nothing, which `DirSource` implements. Errors from a folder now name the path they happened at ("source error: PATH: Permission denied"). +- **sandfs and sandfs-core, a file that shrank in the folder under a mount**: a mount of a folder lists each file once and reads its bytes from the folder as they are. When a file got shorter in the folder after that (a copy onto it truncates it first), a read came back short of the listed size, and macOS's NFS client filled the rest with zeros: copying files from a mount onto the folder under it wrote zeros into them. Now a read that comes back short makes the mount ask the folder again for that file's size and time alone. On Linux the read is the file's end and a later `stat` gives the new size. On macOS, whose client won't take a smaller size from a read, that one read fails ("Stale NFS file handle") and the next one reads the file as it is now; a read never gives zeros. In `sandfs-core` this is `Workspace::recheck` and a new `Source::entry_now` with a default that changes nothing, which `DirSource` implements. Errors from a folder now name the path they happened at ("source error: PATH: Permission denied"). - **sandfs and sandfs-cli, real folders on Linux**: `sandfs fork` of a git clone (or of a fork kept as a folder) into a folder on a disk that clones files, btrfs or XFS, now keeps the fork there as a real folder of clones of the clone's files, as on macOS, instead of mounting it. It takes no space for the files, about 30 ms to make, and builds and git run in it at the disk's own speed, where a mount ran a build after an edit 25-40% slower and created small files 2-5 times slower. sandfs finds out by cloning one file next to the folder and removing it. On ext4, or with `--read-only` or `--mount`, the fork is mounted as before. ## 0.8.0 (2026-10-27) From 446ff5c6790787112202be03bbc7c4cc2f391741 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 28 Oct 2026 00:20:10 +0000 Subject: [PATCH 3/7] Give a shrunk file a new inode over NFS so its old size goes 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 Claude-Session: https://claude.ai/code/session_013jkdGETmgGUb8hcSEpppQK --- crates/sandfs/src/fs.rs | 20 +++++++++++++++----- crates/sandfs/src/nfs.rs | 16 +++++++++++++--- docs/changelog.md | 2 +- 3 files changed, 29 insertions(+), 9 deletions(-) diff --git a/crates/sandfs/src/fs.rs b/crates/sandfs/src/fs.rs index e94b329..4cd5646 100644 --- a/crates/sandfs/src/fs.rs +++ b/crates/sandfs/src/fs.rs @@ -1197,14 +1197,24 @@ impl Inner { /// since it was listed (see [`Workspace::recheck`]). Its attributes then give the /// folder's size now, so a client never fills the bytes it lacks with zeros. /// Returns whether anything changed. - pub fn recheck(&self, ino: u64) -> FsResult { + /// + /// With [`Inner::renumber_changed`] (NFS), a changed file also gets a new inode + /// and its folder a new time, as [`Inner::replace`] does: the macOS client won't + /// take a smaller size from a read, and keeps the one it has for up to a minute, + /// so the old inode goes, and reads through it fail rather than fill with zeros. + pub fn recheck(&mut self, ino: u64) -> FsResult { if self.buffers.contains_key(&ino) { return Ok(false); } - match self.nodes.get(&ino) { - Some(Node { linked: true, path, .. }) => self.ws.recheck(path.as_str()).fs(), - _ => Ok(false), + let Some(Node { linked: true, path, .. }) = self.nodes.get(&ino) else { return Ok(false) }; + let path = path.clone(); + let changed = self.ws.recheck(path.as_str()).fs()?; + if changed && self.renumber_changed { + let others = self.other_names(&path); + self.unlink_path(&path, None, &others); + self.times.extend(path.parent().map(|parent| (parent.to_string(), SystemTime::now()))); } + Ok(changed) } /// The workspace and path holding `ino`'s contents: the workspace itself, or for @@ -2106,7 +2116,7 @@ impl Filesystem for SandFs { // The kernel takes a short read as the file's end; its size from now on // says the same (see `Inner::recheck`). if data.len() < size as usize { - let inner = self.0.lock(); + let mut inner = self.0.lock(); if inner.attr(ino.0).is_ok_and(|attr| attr.size > offset + data.len() as u64) { let _ = inner.recheck(ino.0); } diff --git a/crates/sandfs/src/nfs.rs b/crates/sandfs/src/nfs.rs index 5476e71..bd1e935 100644 --- a/crates/sandfs/src/nfs.rs +++ b/crates/sandfs/src/nfs.rs @@ -280,8 +280,8 @@ impl NFSFileSystem for SandNfs { // shrank in the folder under the mount since it was listed, and the client, // which still has the old size, fills the rest with zeros: a copy from the // mount would write zeros. The macOS client won't take a smaller size from a - // READ reply, so this read fails instead; its reply carries the new size (taken - // after the read), and the client asks again before its next look at the file. + // READ reply, so this read fails instead, and the file gets a new inode (see + // `Inner::recheck`) that the client looks up afresh. let short = data.len() < count as usize; if short && end < size && self.shared.lock().recheck(id).unwrap_or(false) { return Err(nfsstat3::NFS3ERR_STALE); @@ -948,14 +948,24 @@ mod tests { let root = VPath::parse("/mnt").unwrap(); let mut inner = Inner::new(ws, &root, BTreeMap::new(), BTreeSet::new(), UNIX_EPOCH, (501, 20)); inner.ranged_reads = true; + (inner.keep_removed, inner.renumber_changed) = (false, true); let dir = tempfile::tempdir().unwrap(); let fs = SandNfs::new(Shared::new(inner, Saved::new(dir.path()), root)).unwrap(); block(async { let id = fs.lookup(ROOT_INO, &name("notes.md")).await.unwrap(); assert_eq!(fs.getattr(id).await.unwrap().size, 24); + let folder_time = |attr: fattr3| (attr.mtime.seconds, attr.mtime.nseconds); + let before = folder_time(fs.getattr(ROOT_INO).await.unwrap()); std::fs::write(folder.path().join("notes.md"), "new").unwrap(); assert!(fs.read(id, 0, 24).await.is_err(), "no zeros where the client expects 24 bytes"); - assert_eq!(fs.getattr(id).await.unwrap().size, 3, "the size the reply carries"); + assert!(fs.read(id, 0, 24).await.is_err(), "nor through the old inode later"); + assert_ne!( + folder_time(fs.getattr(ROOT_INO).await.unwrap()), + before, + "so the client looks the name up again" + ); + let id = fs.lookup(ROOT_INO, &name("notes.md")).await.unwrap(); + assert_eq!(fs.getattr(id).await.unwrap().size, 3, "a new inode with the size now"); assert_eq!(fs.read(id, 0, 24).await.unwrap(), (b"new".to_vec(), true), "the end, not more to come"); // A read of part of a file that hasn't changed is not its end. std::fs::write(folder.path().join("notes.md"), "a longer file now").unwrap(); diff --git a/docs/changelog.md b/docs/changelog.md index f1a7158..8e0f937 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -2,7 +2,7 @@ ## Unreleased -- **sandfs and sandfs-core, a file that shrank in the folder under a mount**: a mount of a folder lists each file once and reads its bytes from the folder as they are. When a file got shorter in the folder after that (a copy onto it truncates it first), a read came back short of the listed size, and macOS's NFS client filled the rest with zeros: copying files from a mount onto the folder under it wrote zeros into them. Now a read that comes back short makes the mount ask the folder again for that file's size and time alone. On Linux the read is the file's end and a later `stat` gives the new size. On macOS, whose client won't take a smaller size from a read, that one read fails ("Stale NFS file handle") and the next one reads the file as it is now; a read never gives zeros. In `sandfs-core` this is `Workspace::recheck` and a new `Source::entry_now` with a default that changes nothing, which `DirSource` implements. Errors from a folder now name the path they happened at ("source error: PATH: Permission denied"). +- **sandfs and sandfs-core, a file that shrank in the folder under a mount**: a mount of a folder lists each file once and reads its bytes from the folder as they are. When a file got shorter in the folder after that (a copy onto it truncates it first), a read came back short of the listed size, and macOS's NFS client filled the rest with zeros: copying files from a mount onto the folder under it wrote zeros into them. Now a read that comes back short makes the mount ask the folder again for that file's size and time alone. On Linux the read is the file's end and a later `stat` gives the new size. On macOS, whose client won't take a smaller size from a read, the read fails ("Stale NFS file handle") and the file gets a new inode, which the client looks up within a second or so and then reads as the file is now. A read never gives zeros. In `sandfs-core` this is `Workspace::recheck` and a new `Source::entry_now` with a default that changes nothing, which `DirSource` implements. Errors from a folder now name the path they happened at ("source error: PATH: Permission denied"). - **sandfs and sandfs-cli, real folders on Linux**: `sandfs fork` of a git clone (or of a fork kept as a folder) into a folder on a disk that clones files, btrfs or XFS, now keeps the fork there as a real folder of clones of the clone's files, as on macOS, instead of mounting it. It takes no space for the files, about 30 ms to make, and builds and git run in it at the disk's own speed, where a mount ran a build after an edit 25-40% slower and created small files 2-5 times slower. sandfs finds out by cloning one file next to the folder and removing it. On ext4, or with `--read-only` or `--mount`, the fork is mounted as before. ## 0.8.0 (2026-10-27) From 38760a2135d62331c28d5f94b4ba6d786017df28 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 28 Oct 2026 00:24:20 +0000 Subject: [PATCH 4/7] Recheck the size a file whose mode changed here holds itself 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 Claude-Session: https://claude.ai/code/session_013jkdGETmgGUb8hcSEpppQK --- crates/sandfs-core/src/workspace.rs | 45 +++++++++++++++++---------- crates/sandfs-core/tests/workspace.rs | 9 ++++++ 2 files changed, 37 insertions(+), 17 deletions(-) diff --git a/crates/sandfs-core/src/workspace.rs b/crates/sandfs-core/src/workspace.rs index a059faa..7736e51 100644 --- a/crates/sandfs-core/src/workspace.rs +++ b/crates/sandfs-core/src/workspace.rs @@ -430,34 +430,45 @@ impl Workspace { }) } - /// Ask the source again about the file at `path`, which this workspace hasn't + /// Ask the source again about the file at `path`, whose bytes this workspace hasn't /// changed, after a read showed it is no longer what its listing said: a read that /// came back shorter than the listed size, from a folder written to meanwhile. Its /// size, time and cached bytes become what the source says now (see /// [`Source::entry_now`]), for this file alone; the rest of its directory stays as - /// first listed. Returns whether anything changed. Forks share what was asked, - /// as they share every listing. - pub fn recheck(&self, path: &str) -> Result { + /// first listed. A file whose mode changed here keeps its mode. Returns whether + /// anything changed. Forks share the listing, as they share every listing. + pub fn recheck(&mut self, path: &str) -> Result { let path = VPath::parse(path)?; let Ok(Resolved::File { content: Content::Base { mount, key, .. }, .. }) = self.resolve(&path) else { return Ok(false); }; - let (Some(parent), Some(name)) = (path.parent(), path.name()) else { return Ok(false) }; - let Some(dir) = self.base_dir(&parent, &mount)? else { return Ok(false) }; - let listing = self.cached_listing(&mount, &dir)?; - // A file moved here from another directory isn't in this listing. - if listing.get(name).is_none_or(|entry| entry.key != key) { - return Ok(false); - } let Some(now) = mount.source.entry_now(&key)? else { return Ok(false) }; - if !self.cache.patch_listing(&(mount.id, dir), name, now.clone()) { - return Ok(false); + let mut changed = false; + // A file whose mode changed here holds the source's size and time itself. + if let Some(Overlay::File { content: Content::Base { key: held, size, modified, .. }, .. }) = + self.overlay.get_mut(&path) + && *held == key + && matches!(now.kind, Kind::File { .. }) + && (*size, *modified) != (now.size, now.modified) + { + (*size, *modified) = (now.size, now.modified); + changed = true; } - self.cache.forget_blob(&(mount.id, key)); - if let Some((mount_path, _)) = self.mounts.iter().find(|(_, owner)| owner.id == mount.id) { - self.cache.remember_entry(mount.id, &path.as_str()[mount_path.as_str().len() + 1..], now); + // A file moved here from another directory isn't in this listing. + if let (Some(parent), Some(name)) = (path.parent(), path.name()) + && let Some(dir) = self.base_dir(&parent, &mount)? + && self.cached_listing(&mount, &dir)?.get(name).is_some_and(|entry| entry.key == key) + && self.cache.patch_listing(&(mount.id, dir), name, now.clone()) + { + if let Some((mount_path, _)) = self.mounts.iter().find(|(_, owner)| owner.id == mount.id) { + self.cache.remember_entry(mount.id, &path.as_str()[mount_path.as_str().len() + 1..], now); + } + changed = true; } - Ok(true) + if changed { + self.cache.forget_blob(&(mount.id, key)); + } + Ok(changed) } /// [`Metadata::modified`] alone: the source's time for a file nobody wrote, found diff --git a/crates/sandfs-core/tests/workspace.rs b/crates/sandfs-core/tests/workspace.rs index 6482faa..d9fe965 100644 --- a/crates/sandfs-core/tests/workspace.rs +++ b/crates/sandfs-core/tests/workspace.rs @@ -1010,6 +1010,7 @@ fn a_recheck_takes_a_changed_folder_file_as_it_is_now() { std::fs::write(dir.join("sub/shrink.txt"), "twenty-four bytes long..").unwrap(); std::fs::write(dir.join("sub/other.txt"), "other").unwrap(); std::fs::write(dir.join("sub/mine.txt"), "mine").unwrap(); + std::fs::write(dir.join("sub/mode.txt"), "twenty-four bytes long..").unwrap(); let mut ws = Workspace::new(); ws.mount("/host", Arc::new(DirSource::new(&dir).unwrap())).unwrap(); @@ -1041,6 +1042,14 @@ fn a_recheck_takes_a_changed_folder_file_as_it_is_now() { assert_eq!(ws.read_text("/host/sub/other.txt").unwrap(), "other, longer now"); // A forked snapshot sees the same answer, as it shares every listing. assert_eq!(ws.fork().stat("/host/sub/shrink.txt").unwrap().size, 3); + // A file whose mode changed here takes the new size too, and keeps its mode. + let mut ws = ws.fork(); + ws.set_executable("/host/sub/mode.txt", true).unwrap(); + std::fs::write(dir.join("sub/mode.txt"), "new").unwrap(); + assert!(ws.recheck("/host/sub/mode.txt").unwrap()); + let meta = ws.stat("/host/sub/mode.txt").unwrap(); + assert_eq!((meta.size, meta.kind), (3, Kind::File { executable: true })); + assert!(!ws.recheck("/host/sub/mode.txt").unwrap()); // Gone from the folder: kept as it was. std::fs::remove_file(dir.join("sub/shrink.txt")).unwrap(); assert!(!ws.recheck("/host/sub/shrink.txt").unwrap()); From d9ef999cfb6c9b14029753e9945b9d4debf767b2 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 28 Oct 2026 00:27:17 +0000 Subject: [PATCH 5/7] Recheck the size a file with several names holds 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 Claude-Session: https://claude.ai/code/session_013jkdGETmgGUb8hcSEpppQK --- crates/sandfs-core/src/workspace.rs | 17 ++++++++++++++--- crates/sandfs-core/tests/workspace.rs | 7 +++++++ 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/crates/sandfs-core/src/workspace.rs b/crates/sandfs-core/src/workspace.rs index 7736e51..2bc9265 100644 --- a/crates/sandfs-core/src/workspace.rs +++ b/crates/sandfs-core/src/workspace.rs @@ -444,9 +444,20 @@ impl Workspace { }; let Some(now) = mount.source.entry_now(&key)? else { return Ok(false) }; let mut changed = false; - // A file whose mode changed here holds the source's size and time itself. - if let Some(Overlay::File { content: Content::Base { key: held, size, modified, .. }, .. }) = - self.overlay.get_mut(&path) + // A file whose mode changed here, or that has several names, holds the + // source's size and time itself. + let held = match self.overlay.get(&path) { + Some(Overlay::Linked { id }) => { + let id = *id; + self.links.get_mut(&id).map(|linked| &mut linked.content) + } + Some(Overlay::File { .. }) => match self.overlay.get_mut(&path) { + Some(Overlay::File { content, .. }) => Some(content), + _ => None, + }, + _ => None, + }; + if let Some(Content::Base { key: held, size, modified, .. }) = held && *held == key && matches!(now.kind, Kind::File { .. }) && (*size, *modified) != (now.size, now.modified) diff --git a/crates/sandfs-core/tests/workspace.rs b/crates/sandfs-core/tests/workspace.rs index d9fe965..501b624 100644 --- a/crates/sandfs-core/tests/workspace.rs +++ b/crates/sandfs-core/tests/workspace.rs @@ -1011,6 +1011,7 @@ fn a_recheck_takes_a_changed_folder_file_as_it_is_now() { std::fs::write(dir.join("sub/other.txt"), "other").unwrap(); std::fs::write(dir.join("sub/mine.txt"), "mine").unwrap(); std::fs::write(dir.join("sub/mode.txt"), "twenty-four bytes long..").unwrap(); + std::fs::write(dir.join("sub/linked.txt"), "twenty-four bytes long..").unwrap(); let mut ws = Workspace::new(); ws.mount("/host", Arc::new(DirSource::new(&dir).unwrap())).unwrap(); @@ -1050,6 +1051,12 @@ fn a_recheck_takes_a_changed_folder_file_as_it_is_now() { let meta = ws.stat("/host/sub/mode.txt").unwrap(); assert_eq!((meta.size, meta.kind), (3, Kind::File { executable: true })); assert!(!ws.recheck("/host/sub/mode.txt").unwrap()); + // So does a file with a second name, through either name. + ws.hard_link("/host/sub/linked.txt", "/host/sub/second.txt").unwrap(); + std::fs::write(dir.join("sub/linked.txt"), "new").unwrap(); + assert!(ws.recheck("/host/sub/second.txt").unwrap()); + assert_eq!(ws.stat("/host/sub/linked.txt").unwrap().size, 3); + assert_eq!(ws.read_text("/host/sub/second.txt").unwrap(), "new"); // Gone from the folder: kept as it was. std::fs::remove_file(dir.join("sub/shrink.txt")).unwrap(); assert!(!ws.recheck("/host/sub/shrink.txt").unwrap()); From e2bfc95ca35d109fad681f71e73d004ed0d39b72 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 28 Oct 2026 00:31:57 +0000 Subject: [PATCH 6/7] Take every name of a shrunk file off its old NFS inode 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 Claude-Session: https://claude.ai/code/session_013jkdGETmgGUb8hcSEpppQK --- crates/sandfs/src/fs.rs | 16 +++++++++++++--- crates/sandfs/src/nfs.rs | 14 ++++++++++++++ 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/crates/sandfs/src/fs.rs b/crates/sandfs/src/fs.rs index 4cd5646..29734a9 100644 --- a/crates/sandfs/src/fs.rs +++ b/crates/sandfs/src/fs.rs @@ -1209,10 +1209,20 @@ impl Inner { let Some(Node { linked: true, path, .. }) = self.nodes.get(&ino) else { return Ok(false) }; let path = path.clone(); let changed = self.ws.recheck(path.as_str()).fs()?; + // Every name of the file goes off the inode at once: a hard link left on it + // would bring back the size the client keeps for it. if changed && self.renumber_changed { - let others = self.other_names(&path); - self.unlink_path(&path, None, &others); - self.times.extend(path.parent().map(|parent| (parent.to_string(), SystemTime::now()))); + let now = SystemTime::now(); + for name in self.other_names(&path).iter().chain([&path]) { + if self.inos.get(name.as_str()) == Some(&ino) { + self.inos.remove(name.as_str()); + } + self.times.extend(name.parent().map(|parent| (parent.to_string(), now))); + } + if let Some(node) = self.nodes.get_mut(&ino) { + (node.linked, node.lookups) = (false, 0); + } + self.maybe_drop(ino); } Ok(changed) } diff --git a/crates/sandfs/src/nfs.rs b/crates/sandfs/src/nfs.rs index bd1e935..5456a45 100644 --- a/crates/sandfs/src/nfs.rs +++ b/crates/sandfs/src/nfs.rs @@ -943,6 +943,7 @@ mod tests { // the rest of a 3-byte read with zeros. let folder = tempfile::tempdir().unwrap(); std::fs::write(folder.path().join("notes.md"), "twenty-four bytes long..").unwrap(); + std::fs::write(folder.path().join("pair.md"), "twenty-four bytes long..").unwrap(); let mut ws = Workspace::new(); ws.mount("/mnt", Arc::new(sandfs_core::DirSource::new(folder.path()).unwrap())).unwrap(); let root = VPath::parse("/mnt").unwrap(); @@ -967,6 +968,19 @@ mod tests { let id = fs.lookup(ROOT_INO, &name("notes.md")).await.unwrap(); assert_eq!(fs.getattr(id).await.unwrap().size, 3, "a new inode with the size now"); assert_eq!(fs.read(id, 0, 24).await.unwrap(), (b"new".to_vec(), true), "the end, not more to come"); + // A file with two names gets a new inode under both. + let pair = fs.lookup(ROOT_INO, &name("pair.md")).await.unwrap(); + fs.link(pair, ROOT_INO, &name("twin.md")).await.unwrap(); + std::fs::write(folder.path().join("pair.md"), "new").unwrap(); + assert!(fs.read(pair, 0, 24).await.is_err()); + assert!(fs.read(pair, 0, 24).await.is_err(), "no name keeps the old inode"); + let (one, two) = ( + fs.lookup(ROOT_INO, &name("pair.md")).await.unwrap(), + fs.lookup(ROOT_INO, &name("twin.md")).await.unwrap(), + ); + assert_eq!(one, two); + assert_ne!(one, pair); + assert_eq!(fs.getattr(one).await.unwrap().size, 3); // A read of part of a file that hasn't changed is not its end. std::fs::write(folder.path().join("notes.md"), "a longer file now").unwrap(); let id2 = fs.lookup(ROOT_INO, &name("notes.md")).await.unwrap(); From 9e2f38bc31204ba95445f29df3a0c4fd068d8cf2 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 28 Oct 2026 00:34:52 +0000 Subject: [PATCH 7/7] Update a rechecked file's listing and path entry under one lock 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 Claude-Session: https://claude.ai/code/session_013jkdGETmgGUb8hcSEpppQK --- crates/sandfs-core/src/workspace.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/crates/sandfs-core/src/workspace.rs b/crates/sandfs-core/src/workspace.rs index 2bc9265..07e49db 100644 --- a/crates/sandfs-core/src/workspace.rs +++ b/crates/sandfs-core/src/workspace.rs @@ -109,7 +109,9 @@ impl Cache { /// Put `entry` in the cached listing `key` under `name`, in place of the entry with /// the same source key, and say whether that changed it. The listing's own lock is /// held throughout, so two files of one directory changed at once both stay changed. - fn patch_listing(&self, key: &CacheKey, name: &str, entry: SourceEntry) -> bool { + /// `then` runs under that lock too, so what it caches from `entry` changes in the + /// same order as the listing. + fn patch_listing(&self, key: &CacheKey, name: &str, entry: SourceEntry, then: impl FnOnce(SourceEntry)) -> bool { let Some(slot) = self.dirs.lock().expect("cache lock").get(key).cloned() else { return false }; let mut listing = slot.lock().expect("cache slot lock"); let Some(current) = listing.as_ref() else { return false }; @@ -117,8 +119,9 @@ impl Cache { return false; } let mut fresh = (**current).clone(); - fresh.insert(name.to_owned(), entry); + fresh.insert(name.to_owned(), entry.clone()); *listing = Some(Arc::new(fresh)); + then(entry); true } @@ -469,11 +472,11 @@ impl Workspace { if let (Some(parent), Some(name)) = (path.parent(), path.name()) && let Some(dir) = self.base_dir(&parent, &mount)? && self.cached_listing(&mount, &dir)?.get(name).is_some_and(|entry| entry.key == key) - && self.cache.patch_listing(&(mount.id, dir), name, now.clone()) - { - if let Some((mount_path, _)) = self.mounts.iter().find(|(_, owner)| owner.id == mount.id) { + && let Some((mount_path, _)) = self.mounts.iter().find(|(_, owner)| owner.id == mount.id) + && self.cache.patch_listing(&(mount.id, dir), name, now, |now| { self.cache.remember_entry(mount.id, &path.as_str()[mount_path.as_str().len() + 1..], now); - } + }) + { changed = true; } if changed {