diff --git a/crates/pdfs-fuse/src/drain.rs b/crates/pdfs-fuse/src/drain.rs index 573173b..b8d1700 100644 --- a/crates/pdfs-fuse/src/drain.rs +++ b/crates/pdfs-fuse/src/drain.rs @@ -2294,6 +2294,14 @@ impl Core { blob: &Path, adopt: impl FnOnce(i64, u64), ) -> Option { + // Asked before the read-back: the op went before it, so a rename queued + // behind the upload can land while Drive answers, and the answer may + // still have the old name (`docs/BUGS.md` B172). + let placed = self.placement(uid); + let renaming = self + .db + .has_pending_op(&uid.to_string(), OP_RENAME) + .unwrap_or(false); let fetched = self.fetch_node_remote(uid); if fetched.is_ok() { self.sealed_unread.lock().remove(uid); @@ -2376,37 +2384,42 @@ impl Core { self.release_pending(uid, blob); #[cfg(test)] self.drive.landing(uid); - self.place_landed(uid, &node); + self.place_landed(uid, &node, placed, renaming); self.landed.lock().remove(uid); Some(node) } /// Put a node whose upload just landed into the tree, unless a newer write - /// is queued for it. - fn place_landed(&self, uid: &NodeUid, node: &Node) { + /// is queued for it. `before` is where the tree had the node and + /// `renaming` whether a rename of it was queued, both from before the + /// read-back. + fn place_landed( + &self, + uid: &NodeUid, + node: &Node, + before: Option<(String, Option)>, + renaming: bool, + ) { // Ordered so that a write queued *during* the fetch above is still // caught: it took its baseline from the node's optimistic stamp, and // this overwrites it with the revision the server actually holds. if self.rebaseline_pending(uid, node) { return; } - // A rename queued behind the upload has not reached Drive yet, and - // until it lands the tree keeps the name and folder the user gave the - // file, as `adopt_real_uid` does. Taking Drive's put the file back - // under its old name. A rename queued after the check below has - // already moved the entry when the tree is updated, so a name that - // changed in between is the user's too (docs/BUGS.md B146). - let placed = |st: &crate::state::State| { - let entry = st.by_uid.get(uid).and_then(|ino| st.entries.get(ino))?; - Some((entry.node.name.clone(), entry.node.parent_uid.clone())) - }; - let before = placed(&self.state()); - let renaming = self - .db - .has_pending_op(&uid.to_string(), OP_RENAME) - .unwrap_or(false); + // A rename queued behind the upload has not reached Drive yet, or + // landed after Drive read the node back, and the tree keeps the name + // and folder the user gave the file, as `adopt_real_uid` does. Taking + // Drive's put the file back under its old name. A rename queued since + // the read-back began has already moved the entry when the tree is + // updated, so a name that changed in between is the user's too + // (docs/BUGS.md B146, B172). + let renaming = renaming + || self + .db + .has_pending_op(&uid.to_string(), OP_RENAME) + .unwrap_or(false); self.for_each_state(|st| { - let moved = placed(st) != before; + let moved = placed_in(st, uid) != before; let Some(entry) = st.by_uid.get(uid).and_then(|ino| st.entries.get(ino)) else { return; }; @@ -2420,6 +2433,11 @@ impl Core { }); } + /// Where the tree has `uid`: its name and parent. + fn placement(&self, uid: &NodeUid) -> Option<(String, Option)> { + placed_in(&self.state(), uid) + } + /// Rebase every write handle open on `uid` onto `node`, a revision this /// daemon has just put there itself (`WriteHandle::rebase_onto`). fn rebase_open_writes(&self, uid: &NodeUid, node: &Node) { @@ -2675,6 +2693,12 @@ fn keeps_landed_upload(pinned: bool, len: u64, budget: u64) -> bool { pinned || budget == 0 || len <= budget / LANDED_KEEP_SHARE } +/// Where `st` has `uid`: its name and parent. +fn placed_in(st: &crate::state::State, uid: &NodeUid) -> Option<(String, Option)> { + let entry = st.by_uid.get(uid).and_then(|ino| st.entries.get(ino))?; + Some((entry.node.name.clone(), entry.node.parent_uid.clone())) +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/pdfs-fuse/src/sim/daemon.rs b/crates/pdfs-fuse/src/sim/daemon.rs index a0e1321..4ce626b 100644 --- a/crates/pdfs-fuse/src/sim/daemon.rs +++ b/crates/pdfs-fuse/src/sim/daemon.rs @@ -463,6 +463,68 @@ 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_renamed_while_its_upload_is_read_back_keeps_its_new_name() { + // The rename landed while the drain read the uploaded file back. The + // answer still had the old name, and the drain found no rename queued, + // so the mount showed the file under its old name and the new one + // answered ENOENT (B172). + let drive = FakeDrive::new(); + let dir = scratch("renamed-while-read-back"); + let daemon = Daemon::start(&dir, drive.client(1, Faults::lan())).unwrap(); + let mnt = daemon.mountpoint.clone(); + let file = |bytes: &[u8]| Some(Entry::File(Arc::new(bytes.to_vec()))); + + std::fs::write(mnt.join("a.txt"), b"first").unwrap(); + assert!(wait_until(Duration::from_secs(30), || { + daemon.pending().is_ok_and(|items| items.is_empty()) + && drive.tree().get("a.txt").cloned() == file(b"first") + })); + let uid = drive.lookup("a.txt").unwrap(); + let mut held = daemon.client.hold_answer_to_next_read(&uid); + std::fs::write(mnt.join("a.txt"), b"second").unwrap(); + assert!(wait_until(Duration::from_secs(30), || held.reached())); + std::fs::rename(mnt.join("a.txt"), mnt.join("f.txt")).unwrap(); + // Each read is answered in turn until the upload's op is retired; the + // answer to the read-back after it is held until the rename has landed. + let reading_back = loop { + if daemon + .pending() + .is_ok_and(|items| items.iter().all(|op| op.kind != "revision")) + { + break held; + } + let next = daemon.client.hold_answer_to_next_read(&uid); + drop(held); + assert!(wait_until(Duration::from_secs(30), || next.reached())); + held = next; + }; + assert!( + wait_until(Duration::from_secs(30), || drive.lookup("f.txt").is_some()), + "{:?} {:?}", + daemon.pending(), + drive.tree() + ); + drop(reading_back); + assert!( + wait_until(Duration::from_secs(30), || { + daemon.pending().is_ok_and(|items| items.is_empty()) + && drive.tree().get("f.txt").cloned() == file(b"second") + }), + "{:?} {:?}", + daemon.pending(), + drive.tree() + ); + // Long enough for the drain to have put the read-back into the tree. + std::thread::sleep(Duration::from_millis(200)); + assert_eq!(std::fs::read(mnt.join("f.txt")).unwrap(), b"second"); + 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 1eb057d..16dcaf2 100644 --- a/docs/BUGS.md +++ b/docs/BUGS.md @@ -12,6 +12,31 @@ Conventions: --- +## B172 — A file renamed while its upload is read back shows its old name + +**Status:** Fixed (unverified). +**Found:** 2026-10-04, by `one_client_on_a_good_link` (seed 8) on 2502af6, run three at a time +in a loop: two of nine runs failed. In one, a write to `f.txt` answered `ENOENT`; in the other, +a rename of `f.txt`. The mount showed the file as `a.txt` again. + +**Where:** `refresh_after_upload` and `place_landed` in `crates/pdfs-fuse/src/drain.rs`. + +**Cause.** `a.txt` was renamed to `f.txt` while its create was on the wire, with a write behind +it. The create landed, and its op became a revision with a rename queued after it. The upload +retires its op before it reads the node back, so the rename went out while Drive answered the +read-back, and that answer still had the old name. The drain asked whether a rename was queued +only after the answer came, found none, and put `a.txt` back into the tree. The create's +read-back already asks first (B161); the upload's did not. + +**Fix.** The upload's read-back notes whether a rename is queued, and where the tree has the +file, before it asks Drive. The tree keeps the name and folder the user gave the file if a +rename was queued then or is now, or if the file moved meanwhile. + +**Test:** `a_file_renamed_while_its_upload_is_read_back_keeps_its_new_name` in +`crates/pdfs-fuse/src/sim/daemon.rs`. + +--- + ## B171 — A file deleted while its upload fails stays on Drive for the upload's backoff **Status:** Fixed (unverified). diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 2981429..91eb949 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -73,7 +73,8 @@ by its local id, in the queue, the stored tree and pins alike. - **A file deleted while its upload was retrying is removed from Drive right away.** It no longer waits for the upload's next retry (B171). - **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). + the old name, and a second rename no longer failed with "No such file or directory" (B152, + B172). - **A change that waits for a name to be freed no longer waits seconds after it is.** A rename or upload over a file being deleted goes out as soon as the delete lands (B153). - **A file deleted right after it was made no longer stays on Drive when the link lost Drive's diff --git a/docs/MILESTONE-3.0.0-STATUS.md b/docs/MILESTONE-3.0.0-STATUS.md index abf4457..437d6f9 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 B171. +- Fixes from the simulation and account runs since then: B127 and B141 to B172. - **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`. @@ -114,7 +114,8 @@ Tracks `docs/MILESTONE-3.0.0.md`. Update it with every commit that moves a phase `one_client_on_a_good_link`: a write opened as an upload landed took the revision it replaced as its base, and the next write failed with `EIO` (B170, fixed). The suite's seed 9 found a deleted file's trash waiting out its create's backoff (B171, - fixed). + fixed). Run three at a time on 2502af6, seed 8 failed two of nine runs: a file renamed + while its upload was read back showed its old name again (B172, 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