From ce60e006ac7ebd9a6eee119ff0b45a54f6f74b34 Mon Sep 17 00:00:00 2001 From: Nils Pukropp Date: Sun, 4 Oct 2026 22:24:28 +0200 Subject: [PATCH] fix(fuse): a file replaced just as its create lands is trashed on Drive (B175) --- crates/pdfs-fuse/src/filesystem.rs | 2 +- crates/pdfs-fuse/src/lib.rs | 26 ++++++++++++---- crates/pdfs-fuse/src/sim/daemon.rs | 49 ++++++++++++++++++++++++++++++ docs/BUGS.md | 22 ++++++++++++++ docs/CHANGELOG.md | 2 ++ docs/MILESTONE-3.0.0-STATUS.md | 5 +-- 6 files changed, 97 insertions(+), 9 deletions(-) diff --git a/crates/pdfs-fuse/src/filesystem.rs b/crates/pdfs-fuse/src/filesystem.rs index 81f6eeb..d9e3ceb 100644 --- a/crates/pdfs-fuse/src/filesystem.rs +++ b/crates/pdfs-fuse/src/filesystem.rs @@ -1946,7 +1946,7 @@ impl ProtonFs { reply.error(error); return; } - if let Err(e) = self.core.remove_replaced(victim_uid, newname) { + if let Err(e) = self.core.remove_replaced(*victim_ino, victim_uid, newname) { reply.error(e); return; } diff --git a/crates/pdfs-fuse/src/lib.rs b/crates/pdfs-fuse/src/lib.rs index d6162b2..005d647 100644 --- a/crates/pdfs-fuse/src/lib.rs +++ b/crates/pdfs-fuse/src/lib.rs @@ -5007,15 +5007,18 @@ impl Core { /// /// A node whose own creation is still queued has never reached the server, so /// dropping its queued ops is the whole removal; nothing goes to the wire and - /// it works offline. + /// it works offline. A create that landed before its ops were dropped left + /// none: Drive has the node, so it is trashed there like any other + /// (`docs/BUGS.md` B175). /// /// A handle still open on the replaced node keeps reading its bytes, as /// `rename(2)` promises: the inode stays as an unlinked entry and its /// staged bytes, cache and reader go at the final close, exactly as for an /// `unlink` under an open handle. Its queued ops go now, or the drain would /// upload to the node after it was trashed (`docs/BUGS.md` B106). - fn remove_replaced(&self, uid: &NodeUid, name: &str) -> Result<(), Errno> { - if is_local_uid(uid) { + fn remove_replaced(&self, ino: u64, uid: &NodeUid, name: &str) -> Result<(), Errno> { + let landed; + let uid = if is_local_uid(uid) { if self.is_open_anywhere(uid) { self.withdraw_queued_ops(uid)?; } else { @@ -5024,9 +5027,20 @@ impl Core { self.for_each_state(|st| { st.forget_or_unlink(uid); }); - debug!(%uid, name, "replaced a node whose create was still queued"); - return Ok(()); - } + match self.landed_row_uid(ino) { + Some(real) => { + debug!(local = %uid, %real, name, "create landed under a replace"); + landed = real; + &landed + } + None => { + debug!(%uid, name, "replaced a node whose create was still queued"); + return Ok(()); + } + } + } else { + uid + }; // Local-first or offline, or the network goes away under the call: // queue the trash. The rename that follows queues as well, behind it. if !self.sends_inline() { diff --git a/crates/pdfs-fuse/src/sim/daemon.rs b/crates/pdfs-fuse/src/sim/daemon.rs index 5eb686a..99efa6e 100644 --- a/crates/pdfs-fuse/src/sim/daemon.rs +++ b/crates/pdfs-fuse/src/sim/daemon.rs @@ -560,6 +560,55 @@ mod tests { let _ = std::fs::remove_dir_all(&dir); } + #[test] + #[ignore = "mounts FUSE: run with `cargo test -p pdfs-fuse sim:: -- --ignored`"] + fn a_file_replaced_as_its_create_lands_is_trashed_on_drive() { + // The same race as B173, through a rename onto the file: the replace + // found no op left to drop and took the file for one Drive had never + // seen. Drive kept it under the name, and the next rename of that name + // moved it instead of the file that replaced it (B175). + let drive = FakeDrive::new(); + let dir = scratch("replaced-as-create-lands"); + let daemon = Daemon::start(&dir, drive.client(1, Faults::lan())).unwrap(); + let mnt = daemon.mountpoint.clone(); + std::fs::write(mnt.join("b.txt"), b"new").unwrap(); + assert!( + wait_until(Duration::from_secs(30), || { + daemon.pending().is_ok_and(|items| items.is_empty()) + && drive.lookup("b.txt").is_some() + }), + "{:?} {:?}", + daemon.pending(), + drive.tree() + ); + + let create = daemon.client.hold_reply_to_next_create(); + std::fs::write(mnt.join("a.txt"), b"old").unwrap(); + assert!(wait_until(Duration::from_secs(30), || create.reached())); + let uid = drive.lookup("a.txt").unwrap(); + let landing = daemon.client.hold_next_landing(&uid); + drop(create); + assert!(wait_until(Duration::from_secs(30), || landing.reached())); + std::fs::rename(mnt.join("b.txt"), mnt.join("a.txt")).unwrap(); + drop(landing); + std::fs::rename(mnt.join("a.txt"), mnt.join("d")).unwrap(); + assert!( + wait_until(Duration::from_secs(30), || { + daemon.pending().is_ok_and(|items| items.is_empty()) + && drive.lookup("a.txt").is_none() + && drive.tree().get("d") == Some(&Entry::File(Arc::new(b"new".to_vec()))) + }), + "{:?} {:?}", + daemon.pending(), + drive.tree() + ); + assert_eq!(std::fs::read(mnt.join("d")).unwrap(), b"new"); + assert!(!mnt.join("a.txt").exists()); + + assert!(matches!(daemon.stop(), Ok(MountOutcome::Shutdown))); + let _ = std::fs::remove_dir_all(&dir); + } + #[test] #[ignore = "mounts FUSE: run with `cargo test -p pdfs-fuse sim:: -- --ignored`"] fn a_file_moved_while_its_create_was_on_the_wire_keeps_its_new_name() { diff --git a/docs/BUGS.md b/docs/BUGS.md index 5a1249d..1d7ba2a 100644 --- a/docs/BUGS.md +++ b/docs/BUGS.md @@ -12,6 +12,28 @@ Conventions: --- +## B175 — A file replaced by a rename just as its create lands stays on Drive + +**Status:** Fixed (unverified). +**Found:** 2026-10-04, by `one_client_on_a_good_link` (seed 8) on 378667f, run three at a time +in a loop. `rename b.txt -> a.txt` replaced an `a.txt` whose create had landed a few +milliseconds before, and the next `rename a.txt -> d` moved the old `a.txt` on Drive. Drive +ended with the old bytes in `d` and the new ones still in `a.txt`, and the queue did not drain. + +**Where:** `Core::remove_replaced` in `crates/pdfs-fuse/src/lib.rs`. + +**Cause.** The same gap as B173, through `rename`. The replace found the old file under its +stand-in, found no op left to drop, and took it for a file Drive had never seen. So nothing +trashed it on Drive, and the tree took it back under its name once it got its uid. + +**Fix.** As in B173: after dropping a stand-in's ops, the replace checks whether the node's row +has a real uid. If it has, the file is trashed on Drive like any other replaced file. + +**Test:** `a_file_replaced_as_its_create_lands_is_trashed_on_drive` in +`crates/pdfs-fuse/src/sim/daemon.rs`. + +--- + ## B174 — A file renamed just as its create lands is refused with "Permission denied" **Status:** Fixed (unverified). diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index c709e42..8c0cdde 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -76,6 +76,8 @@ by its local id, in the queue, the stored tree and pins alike. there, and the next file with that name no longer became a conflict copy (B173). - **Renaming a file just as it reaches Drive no longer fails with "Permission denied"** (B174). +- **A file replaced by a rename just as it reached Drive is removed from Drive too.** It no + longer stayed there, and the next rename of its name no longer moved it instead (B175). - **A file renamed just as its upload finished keeps its new name.** The mount no longer showed the old name, and a second rename no longer failed with "No such file or directory" (B152, B172). diff --git a/docs/MILESTONE-3.0.0-STATUS.md b/docs/MILESTONE-3.0.0-STATUS.md index 3cd894d..031d26e 100644 --- a/docs/MILESTONE-3.0.0-STATUS.md +++ b/docs/MILESTONE-3.0.0-STATUS.md @@ -9,7 +9,7 @@ Tracks `docs/MILESTONE-3.0.0.md`. Update it with every commit that moves a phase - **Phase 2**: local ids, inode = local id, one tree for every mount (2.10.0). - **Phase 3, first part**: mkdir, create, rename, unlink and rmdir on the mount are recorded locally and sent from the queue; `"local_first": false` brings back the old path (5e0f30c). -- Fixes from the simulation and account runs since then: B127 and B141 to B174. +- Fixes from the simulation and account runs since then: B127 and B141 to B175. - **Sync issues, first cut** (§6.2): refusals sorted into issues, shown at once in the app, tray and CLI; Export button and `pdfs sync export`; `pdfs sync issues`. @@ -118,7 +118,8 @@ Tracks `docs/MILESTONE-3.0.0.md`. Update it with every commit that moves a phase while its upload was read back showed its old name again (B172, fixed). The loop on d850265 failed seed 12 once in nine runs: a file deleted just as its create landed stayed on Drive (B173, fixed). The suite on 378667f failed seed 6: a rename - in the same gap was refused with `EACCES` (B174, fixed). + in the same gap was refused with `EACCES` (B174, fixed). The loop on 378667f failed + seed 8: a file replaced by a rename in that gap stayed on Drive (B175, fixed). 5. **Release**: 2.10.0 is tagged; the 2.9.0 tag still waits for the user's go-ahead. ## After 3.0.0