From ea10a432166c08b816f54fc82c0dcb165e4b5ed6 Mon Sep 17 00:00:00 2001 From: Nils Pukropp Date: Mon, 26 Oct 2026 17:45:14 +0100 Subject: [PATCH] fix(fuse): a copy right after a save waits for the edit (B256) close(2) returns before the kernel sends release, and release takes the write handle out of the open writes a few milliseconds before the revision is in the queue. A `pdfs copy` checked in that gap found the edit nowhere and copied the version Drive had. A folder copy looked only at the queue, so it also missed files below it still open with unsaved writes. release now keeps the inode in State::releasing until the revision is queued. A copy refuses a file or folder with unsaved writes, or a release still queueing, at it or below it, and a move checks the same set. --- crates/pdfs-fuse/src/filesystem.rs | 11 +++++++++-- crates/pdfs-fuse/src/lib.rs | 10 +++++++++- crates/pdfs-fuse/src/relocate.rs | 3 ++- crates/pdfs-fuse/src/state.rs | 6 ++++++ docs/BUGS.md | 25 +++++++++++++++++++++++++ 5 files changed, 51 insertions(+), 4 deletions(-) diff --git a/crates/pdfs-fuse/src/filesystem.rs b/crates/pdfs-fuse/src/filesystem.rs index ebbb6a1..5d3e443 100644 --- a/crates/pdfs-fuse/src/filesystem.rs +++ b/crates/pdfs-fuse/src/filesystem.rs @@ -671,7 +671,7 @@ impl Filesystem for ProtonFs { reply: ReplyEmpty, ) { let ino = self.tree(ino); - let (handle, unlinked_uid) = { + let (handle, unlinked_uid, releasing) = { let mut st = self.core.state(); let unlinked_uid = release_unlinked_entry(&mut st, ino.0); let ino = st.handles.remove(&fh.0); @@ -684,7 +684,11 @@ impl Filesystem for ProtonFs { None } }); - (h, unlinked_uid) + let releasing = ino.filter(|_| h.is_some()); + if let Some(ino) = releasing { + st.releasing.insert(ino); + } + (h, unlinked_uid, releasing) }; // Deliberately **not** handed to a worker, unlike the other slow // handlers here. The kernel does not wait for a `release` reply before @@ -701,6 +705,9 @@ impl Filesystem for ProtonFs { // finish it (audit F1). What is left is a rename, two fsyncs, a database // insert, and at most a bounded copy out of the local cache. self.finish_release(handle, unlinked_uid, reply); + if let Some(ino) = releasing { + self.core.state().releasing.remove(&ino); + } } fn unlink(&self, _req: &Request, parent: INodeNo, name: &OsStr, reply: ReplyEmpty) { diff --git a/crates/pdfs-fuse/src/lib.rs b/crates/pdfs-fuse/src/lib.rs index ba77d2a..4e9fdc1 100644 --- a/crates/pdfs-fuse/src/lib.rs +++ b/crates/pdfs-fuse/src/lib.rs @@ -5230,7 +5230,15 @@ impl Core { .entries .get(&ino) .ok_or_else(|| CoreError::not_found(format!("{} is gone", rel.display())))?; - let unsaved = st.active_writes.get(&ino).is_some_and(|w| w.dirty); + // Unsaved writes, or a release still queueing them, to the file or, + // for a folder, to any file below it. + let unsaved = st + .active_writes + .iter() + .filter(|(_, w)| w.dirty) + .map(|(&open, _)| open) + .chain(st.releasing.iter().copied()) + .any(|open| relocate::ino_is_under(&st, open, ino)); (entry.node.name.clone(), entry.node.is_folder(), unsaved) }; // Unlike a move, a copy cannot wait in the queue: it needs the node as diff --git a/crates/pdfs-fuse/src/relocate.rs b/crates/pdfs-fuse/src/relocate.rs index f3bfb28..945d85e 100644 --- a/crates/pdfs-fuse/src/relocate.rs +++ b/crates/pdfs-fuse/src/relocate.rs @@ -133,7 +133,7 @@ fn rename_noreplace(from: &Path, to: &Path) -> std::io::Result<()> { /// Whether inode `ino` is `root` or lies under it in the tree. Bounded, so a /// parent cycle in corrupt data cannot hang the caller. -fn ino_is_under(st: &State, mut ino: u64, root: u64) -> bool { +pub(crate) fn ino_is_under(st: &State, mut ino: u64, root: u64) -> bool { for _ in 0..MAX_TREE_DEPTH { if ino == root { return true; @@ -456,6 +456,7 @@ impl Core { writing = st .active_writes .keys() + .chain(&st.releasing) .any(|&open| ino_is_under(st, open, ino)); }); Ok(writing) diff --git a/crates/pdfs-fuse/src/state.rs b/crates/pdfs-fuse/src/state.rs index aaf7996..9a66e0a 100644 --- a/crates/pdfs-fuse/src/state.rs +++ b/crates/pdfs-fuse/src/state.rs @@ -266,6 +266,11 @@ pub(crate) struct State { pub(crate) next_ino: u64, /// Shared write state keyed by inode. Concurrent writers share the scratch file. pub(crate) active_writes: HashMap, + /// Inodes whose last write handle `release` has taken out of + /// [`State::active_writes`] and is still queueing. `close(2)` returns + /// before that, and in between the edit is in neither the handles nor the + /// queue, so a check for changes not on Drive yet looks here too. + pub(crate) releasing: HashSet, /// Maps file handle id (fh) to inode (ino). Read-only opens use fh 0 and /// have no entry here. pub(crate) handles: HashMap, @@ -428,6 +433,7 @@ impl State { roots: HashSet::new(), next_ino: FALLBACK_INO, active_writes: HashMap::new(), + releasing: HashSet::new(), handles: HashMap::new(), next_fh: 1, dir_snapshots: HashMap::new(), diff --git a/docs/BUGS.md b/docs/BUGS.md index bf8a4ce..dd2fe73 100644 --- a/docs/BUGS.md +++ b/docs/BUGS.md @@ -100,6 +100,7 @@ a live account since. | Entry | Title | |---|---| +| [B256](#b256--pdfs-copy-right-after-a-save-copies-the-old-version) | `pdfs copy` right after a save copies the old version | | [B254](#b254--an-edit-in-the-same-second-as-a-new-upload-never-arrives) | An edit in the same second as a new upload never arrives | | [B253](#b253--the-app-shows-the-sign-in-page-when-the-keyring-is-locked) | The app shows the sign-in page when the keyring is locked | | [B252](#b252--computers-sharing-a-device-overwrite-each-others-profile) | Computers sharing a device overwrite each other's profile | @@ -357,6 +358,30 @@ a live account since. Newest first. +### B256 — `pdfs copy` right after a save copies the old version + +**Status:** Fixed, not verified live +**Found:** 2026-10-26, the simulation `a_pdfs_copy_copies_on_drive_and_waits_for_unsynced_changes` +failing on most runs. The feature was not released yet. +**Where:** `release` in `crates/pdfs-fuse/src/filesystem.rs`; `copy_to` in +`crates/pdfs-fuse/src/lib.rs`; `has_pending_work_under` in `crates/pdfs-fuse/src/relocate.rs` + +**Repro:** Save a file in the mount and copy it, or its folder, with `pdfs copy` straight away. +The copy goes ahead and Drive copies the version before the save. + +**Cause.** `close(2)` returns before the kernel sends `release`. `release` takes the write handle +out of the open writes and then queues the revision, which takes a few milliseconds. A copy +checked in between found the edit in neither place. A folder copy looked only at the queue, so +it also missed a file still open with unsaved writes. + +**Fix.** `release` keeps the inode in `State::releasing` until the revision is queued. A copy +refuses a file or folder with unsaved writes, or a release still queueing, at it or below it. +A move checks the same set. + +**Test:** The simulation above passed 6 of 6 runs after the fix. + +--- + ### B254 — An edit in the same second as a new upload never arrives **Status:** Fixed, not verified live