diff --git a/crates/pdfs-fuse/src/drain.rs b/crates/pdfs-fuse/src/drain.rs index 2ef2dae..92d1155 100644 --- a/crates/pdfs-fuse/src/drain.rs +++ b/crates/pdfs-fuse/src/drain.rs @@ -1587,15 +1587,17 @@ impl Core { real: &NodeUid, adopt: impl FnOnce(&Node), ) -> Result<(), Box> { + // A rename made while the create was on the wire is queued against the + // real uid (`Db::finish_create`). Until it lands, the tree keeps the + // name and folder the user gave it. Asked before the read-back: a + // rename that lands while Drive answers leaves no op behind, and the + // answer may still have the old name (`docs/BUGS.md` B161). + let real_key = real.to_string(); + let renaming = self.db.has_pending_op(&real_key, OP_RENAME)?; // Drive answers a create before it lists the new node. let mut node = self .read_back(real) .map_err(|e| self.errno_error(e, "fetch node"))?; - // A rename made while the create was on the wire is queued against the - // real uid (`Db::finish_create`). Until it lands, the tree keeps the - // name and folder the user gave it. - let real_key = real.to_string(); - let renaming = self.db.has_pending_op(&real_key, OP_RENAME)?; // Before open handles are rebased onto the node, so one released in // between finds what `adopt` keeps (`docs/BUGS.md` B123). adopt(&node); diff --git a/crates/pdfs-fuse/src/sim/daemon.rs b/crates/pdfs-fuse/src/sim/daemon.rs index bec3447..d33af53 100644 --- a/crates/pdfs-fuse/src/sim/daemon.rs +++ b/crates/pdfs-fuse/src/sim/daemon.rs @@ -422,6 +422,78 @@ 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_moved_while_its_create_was_on_the_wire_keeps_its_new_name() { + // The move queued behind the create landed while the drain read the + // new node back. The answer still had the old name, and the drain no + // longer found the move queued, so the mount showed the old name in + // the new folder (B161). + let drive = FakeDrive::new(); + let dir = scratch("moved-while-created"); + let daemon = Daemon::start(&dir, drive.client(1, Faults::lan())).unwrap(); + let mnt = daemon.mountpoint.clone(); + std::fs::create_dir(mnt.join("x")).unwrap(); + std::fs::create_dir(mnt.join("y")).unwrap(); + assert!( + wait_until(Duration::from_secs(30), || { + daemon.pending().is_ok_and(|items| items.is_empty()) && drive.lookup("y").is_some() + }), + "{:?} {:?}", + daemon.pending(), + drive.tree() + ); + + let create = daemon.client.hold_reply_to_next_create(); + std::fs::write(mnt.join("x/c.bin"), b"abc").unwrap(); + assert!(wait_until(Duration::from_secs(30), || create.reached())); + std::fs::rename(mnt.join("x/c.bin"), mnt.join("y/d")).unwrap(); + let uid = drive.lookup("x/c.bin").unwrap(); + // Each read-back is answered in turn until the create is retired; the + // answer to the next one is held until the move has landed. A new + // folder wakes the drain for the move. + let mut held = create; + let adopting = loop { + let next = daemon.client.hold_answer_to_next_read(&uid); + drop(held); + assert!(wait_until(Duration::from_secs(30), || next.reached())); + if daemon + .pending() + .is_ok_and(|items| items.iter().all(|op| op.kind != "create")) + { + break next; + } + held = next; + }; + std::fs::create_dir(mnt.join("z")).unwrap(); + assert!( + wait_until(Duration::from_secs(10), || drive.lookup("y/d").is_some()), + "{:?} {:?}", + daemon.pending(), + drive.tree() + ); + drop(adopting); + + let file = Some(Entry::File(Arc::new(b"abc".to_vec()))); + assert!( + wait_until(Duration::from_secs(30), || { + daemon.pending().is_ok_and(|items| items.is_empty()) + && drive.tree().get("y/d").cloned() == file + }), + "{:?} {:?}", + daemon.pending(), + drive.tree() + ); + let names: Vec<_> = std::fs::read_dir(mnt.join("y")) + .unwrap() + .map(|e| e.unwrap().file_name().into_string().unwrap()) + .collect(); + assert_eq!(names, ["d"]); + + 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 the_echo_of_our_own_rename_keeps_the_cached_bytes() { diff --git a/crates/pdfs-fuse/src/sim/fake_drive.rs b/crates/pdfs-fuse/src/sim/fake_drive.rs index 5fc5a9e..409fc44 100644 --- a/crates/pdfs-fuse/src/sim/fake_drive.rs +++ b/crates/pdfs-fuse/src/sim/fake_drive.rs @@ -33,7 +33,8 @@ //! //! A test can also hold the next read of one node on its way to Drive //! ([`FakeClient::hold_next_read`]), to make a change while the daemon waits -//! for the answer. +//! for the answer, or on its way back ([`FakeClient::hold_answer_to_next_read`]), +//! so the daemon gets an answer a change made meanwhile has overtaken. use std::collections::{BTreeMap, HashMap}; use std::io::{Read, Write}; @@ -330,6 +331,7 @@ impl FakeDrive { online: AtomicBool::new(true), requests: AtomicU64::new(0), hold: Mutex::new(None), + hold_answer: Mutex::new(None), drop_after_create: AtomicBool::new(false), lose_create_reply: AtomicBool::new(false), hold_create_reply: Mutex::new(None), @@ -583,6 +585,8 @@ struct ClientInner { requests: AtomicU64, /// The read [`FakeClient::hold_next_read`] is waiting for. hold: Mutex>>, + /// The read [`FakeClient::hold_answer_to_next_read`] is waiting for. + hold_answer: Mutex>>, /// Set by [`FakeClient::drop_link_after_next_create`]. drop_after_create: AtomicBool, /// Set by [`FakeClient::lose_reply_to_next_create`]. @@ -667,6 +671,19 @@ impl FakeClient { Held(state) } + /// Answer the next read of `uid` alone with what Drive holds now, then + /// hold the answer until it is dropped, as a slow link does. Only one + /// answer is held at a time. + pub(crate) fn hold_answer_to_next_read(&self, uid: &NodeUid) -> Held { + let state = Arc::new(HoldState { + uid: uid.clone(), + reached: AtomicBool::new(false), + released: AtomicBool::new(false), + }); + *self.0.hold_answer.lock() = Some(state.clone()); + Held(state) + } + /// Make the next file create on Drive, then hold its answer until the /// answer is dropped, as a slow link does. pub(crate) fn hold_reply_to_next_create(&self) -> Held { @@ -691,10 +708,10 @@ impl FakeClient { self.0.lose_revision_read_back.store(true, Ordering::SeqCst); } - /// Wait here if `uids` is the read a [`Held`] is for. - async fn wait_if_held(&self, uids: &[NodeUid]) { + /// Wait here if `uids` is the read a [`Held`] in `slot` is for. + async fn wait_if_held(&self, slot: &Mutex>>, uids: &[NodeUid]) { let state = { - let mut hold = self.0.hold.lock(); + let mut hold = slot.lock(); match hold.as_ref() { Some(state) if uids == std::slice::from_ref(&state.uid) => hold.take(), _ => None, @@ -928,7 +945,7 @@ impl DriveApi for FakeClient { } async fn enumerate_nodes(&self, uids: &[NodeUid]) -> Result> { - self.wait_if_held(uids).await; + self.wait_if_held(&self.0.hold, uids).await; { let mut lose = self.0.lose_read.lock(); if lose @@ -949,6 +966,7 @@ impl DriveApi for FakeClient { .collect()) }) .await?; + self.wait_if_held(&self.0.hold_answer, uids).await; self.cache(&nodes); Ok(nodes) } @@ -1645,6 +1663,29 @@ mod tests { }); } + #[test] + fn a_held_answer_is_what_drive_held_when_read() { + rt().block_on(async { + let drive = FakeDrive::new(); + let client = drive.client(1, Faults::none()); + let root = drive.root(); + let file = client.upload_file(&root, "a", "", b"1").await.unwrap(); + let held = client.hold_answer_to_next_read(&file); + let read = tokio::spawn({ + let client = client.clone(); + let file = file.clone(); + async move { client.enumerate_nodes(std::slice::from_ref(&file)).await } + }); + tokio::time::sleep(Duration::from_millis(50)).await; + assert!(held.reached()); + assert!(!read.is_finished()); + drive.device().trash("a"); + drop(held); + let nodes = read.await.unwrap().unwrap(); + assert!(!nodes[0].trashed); + }); + } + #[test] fn a_rename_lags_only_by_the_update_lag() { rt().block_on(async { diff --git a/docs/BUGS.md b/docs/BUGS.md index 79eaf8d..b7b9763 100644 --- a/docs/BUGS.md +++ b/docs/BUGS.md @@ -12,6 +12,31 @@ Conventions: --- +## B161 — A file moved while it was being created shows up under its old name + +**Status:** Fixed. +**Found:** 2026-10-04, by the simulation's slow-link profile (`one_client_on_a_slow_link`, +seed 3). A file written and then moved at once ended up on Drive as `y/z/d`, but the mount +showed it as `y/z/c.bin`, its name before the move, and `d` was missing. + +**Where:** `Core::adopt_real_uid` in `crates/pdfs-fuse/src/drain.rs`. + +**Cause.** A move made while a file's create is on the wire waits in the queue until the create +lands. Once the create is retired, the drain reads the new node back from Drive and adopts it. +The tree keeps the name the user gave only while the move is still queued. The drain asked +about that after the read-back. On a slow link the move could land in between: the answer +still had the old name, and the move was no longer queued, so the old name won. + +**Fix.** The drain asks whether a move is queued before it reads the node back. It still asks +again under the tree's lock (B152). + +**Test:** simulation test `a_file_moved_while_its_create_was_on_the_wire_keeps_its_new_name` +(failed before the fix: the new folder listed nothing). The fake Drive can now hold a read's +answer, taken when the read arrives, until the test releases it +(`FakeClient::hold_answer_to_next_read`). + +--- + ## B160 — A new folder reaches Drive only half a minute later **Status:** Fixed. diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 3531ace..9acb2ad 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -49,6 +49,8 @@ by its local id, in the queue, the stored tree and pins alike. - **Opening a file right after the daemon restarts no longer fails with "Resource temporarily unavailable"** (B159). - **A new folder reaches Drive at once**, not up to half a minute later (B160). +- **A file moved while it was still being uploaded keeps its new name in the mount** on a slow + link (B161). - **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). - **A change that waits for a name to be freed no longer waits seconds after it is.** A rename or diff --git a/docs/MILESTONE-3.0.0-STATUS.md b/docs/MILESTONE-3.0.0-STATUS.md index 29ad533..13ee5d3 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 B160. +- Fixes from the simulation and account runs since then: B127 and B141 to B161. - **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`. @@ -85,7 +85,8 @@ Tracks `docs/MILESTONE-3.0.0.md`. Update it with every commit that moves a phase landing out of order (B158, fixed). Flaky-link seed 4 failed one of ten runs on a lookup that answered `EAGAIN` while the root was listed again after a restart (B159, fixed). Slow-link seed 4 also had folders reach Drive 30 s late, after the - drain's idle poll (B160, fixed). + drain's idle poll (B160, fixed). Slow-link seed 3 failed on a file moved while its + create was on the wire, which the mount showed under its old name (B161, fixed). - [ ] Known-bug list in `sim/run.rs` is empty: B125 and B130 open; B127, B129 and B132 fixed but unconfirmed. - [ ] A clean account run. The one on a6f36d5 (2026-10-04, LAN: 427 passed, 12 failed)