diff --git a/crates/pdfs-fuse/src/drain.rs b/crates/pdfs-fuse/src/drain.rs index f347672..cb94744 100644 --- a/crates/pdfs-fuse/src/drain.rs +++ b/crates/pdfs-fuse/src/drain.rs @@ -402,8 +402,12 @@ impl Core { // What held the name wakes the ops waiting for it as it lets go // (`Db::wake_ops_waiting_for`). Letting go between our check and // the record above woke nothing, and the op sat out its backoff. + // A create on the wire holds it where the database cannot see: + // woken here, the op spent its waits in milliseconds and landed + // under a conflict name (`docs/BUGS.md` B154). if held && let Some(name) = op.name.as_deref() + && !self.creating_named(&op, name) && let Err(e) = self.db.wake_ops_waiting_for(name) { debug!(uid = %op.uid, error = %e, "waking the ops waiting for a name failed"); @@ -1238,6 +1242,15 @@ impl Core { Ok(creating || self.db.has_pending_move_from(name)?) } + /// Whether a create other than `op` is on the wire under `name`, in any + /// folder. + fn creating_named(&self, op: &PendingOp, name: &str) -> bool { + self.creating + .lock() + .iter() + .any(|(id, (_, n))| *id != op.id && n == name) + } + /// Trash the node a create made after the file was deleted, or replaced by /// a rename, while its upload was on the wire (`docs/BUGS.md` B129). /// @@ -1657,6 +1670,7 @@ impl Core { &self, uid: &NodeUid, meta: &StagedWrite, + blob: &Path, ) -> Result, Box> { let Some(ref base) = meta.based_on else { return Ok(None); @@ -1687,6 +1701,21 @@ impl Core { "queued write chains onto our own sealed revision; not a conflict"); return Ok(None); } + // An earlier attempt at this write may have landed with its answer + // lost: the remote then holds these very bytes in a revision we never + // heard of. Forked, the write was a conflict copy of itself + // (docs/BUGS.md B155); sent again, it is a revision of the same bytes. + if meta.complete + && node_size(&node) == meta.len + && let NodeKind::File { + content_sha1: Some(remote), + .. + } = &node.kind + && remote.eq_ignore_ascii_case(&staged_sha1(blob)?) + { + debug!(%uid, ?remote_rev, "the remote already holds the queued write; not a conflict"); + return Ok(None); + } Ok(Some(reason)) } @@ -2014,7 +2043,7 @@ impl Core { let meta: StagedWrite = serde_json::from_str(op.meta_json.as_deref().unwrap_or(""))?; let uid = parse_node_uid(&meta.uid).ok_or("staged write has an unparseable uid")?; - if let Some(reason) = self.revision_conflict(&uid, &meta)? { + if let Some(reason) = self.revision_conflict(&uid, &meta, &blob)? { // Trashing the file from the mount drops its queued write, but not // from under a worker already holding it. That worker then finds the // node trashed — by us — and would upload the withdrawn bytes as a diff --git a/crates/pdfs-fuse/src/sim/daemon.rs b/crates/pdfs-fuse/src/sim/daemon.rs index d234343..6ae2eab 100644 --- a/crates/pdfs-fuse/src/sim/daemon.rs +++ b/crates/pdfs-fuse/src/sim/daemon.rs @@ -535,6 +535,84 @@ 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_made_under_the_name_of_a_create_on_the_wire_waits_for_it() { + // A file renamed while its create was on the wire, and a new one made + // under its old name. The new file's create found the name held by the + // first and woke itself again after every try: it spent its waits in + // milliseconds and landed under a conflict name, which the mount then + // showed instead of the user's (B154). + let drive = FakeDrive::new(); + let dir = scratch("create-waits-for-create"); + let daemon = Daemon::start(&dir, drive.client(1, Faults::lan())).unwrap(); + let mount = &daemon.mountpoint; + let file = |bytes: &[u8]| Some(Entry::File(Arc::new(bytes.to_vec()))); + + let held = daemon.client.hold_reply_to_next_create(); + std::fs::write(mount.join("c.bin"), b"first").unwrap(); + assert!(wait_until(Duration::from_secs(30), || held.reached())); + std::fs::rename(mount.join("c.bin"), mount.join("d")).unwrap(); + std::fs::write(mount.join("c.bin"), b"second").unwrap(); + assert!(wait_until(Duration::from_secs(30), || { + daemon + .pending() + .is_ok_and(|items| items.iter().any(|op| op.attempts > 0)) + })); + // Long enough for the waits to run out, were they not waits. + std::thread::sleep(Duration::from_millis(300)); + drop(held); + assert!( + wait_until(Duration::from_secs(30), || { + daemon.pending().is_ok_and(|items| items.is_empty()) + && drive.tree().get("d").cloned() == file(b"first") + && drive.tree().get("c.bin").cloned() == file(b"second") + }), + "{:?} {:?}", + daemon.pending(), + drive.tree() + ); + assert_eq!(drive.tree().len(), 2, "{:?}", drive.tree()); + assert_eq!(std::fs::read(mount.join("c.bin")).unwrap(), b"second"); + + 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_write_whose_answer_was_lost_lands_once() { + // Drive made the revision, but its answer was lost. The retry found + // the file at a revision it did not know and kept the write as a + // conflict copy of itself (B155). + let drive = FakeDrive::new(); + let dir = scratch("lost-revision-reply"); + let daemon = Daemon::start(&dir, drive.client(1, Faults::lan())).unwrap(); + let path = daemon.mountpoint.join("f.txt"); + let file = |bytes: &[u8]| Some(Entry::File(Arc::new(bytes.to_vec()))); + + std::fs::write(&path, b"first").unwrap(); + assert!(wait_until(Duration::from_secs(30), || { + daemon.pending().is_ok_and(|items| items.is_empty()) + && drive.tree().get("f.txt").cloned() == file(b"first") + })); + daemon.client.lose_reply_to_next_revision(); + std::fs::write(&path, b"second").unwrap(); + 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() + ); + assert_eq!(drive.tree().len(), 1, "{:?}", drive.tree()); + + 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_pdfs_rename_after_a_queued_rename_is_not_undone() { diff --git a/crates/pdfs-fuse/src/sim/fake_drive.rs b/crates/pdfs-fuse/src/sim/fake_drive.rs index 17feb05..9d70e6e 100644 --- a/crates/pdfs-fuse/src/sim/fake_drive.rs +++ b/crates/pdfs-fuse/src/sim/fake_drive.rs @@ -332,6 +332,8 @@ impl FakeDrive { hold: Mutex::new(None), drop_after_create: AtomicBool::new(false), lose_create_reply: AtomicBool::new(false), + hold_create_reply: Mutex::new(None), + lose_revision_reply: AtomicBool::new(false), })) } @@ -583,6 +585,10 @@ struct ClientInner { drop_after_create: AtomicBool, /// Set by [`FakeClient::lose_reply_to_next_create`]. lose_create_reply: AtomicBool, + /// The create [`FakeClient::hold_reply_to_next_create`] is waiting for. + hold_create_reply: Mutex>>, + /// Set by [`FakeClient::lose_reply_to_next_revision`]. + lose_revision_reply: AtomicBool, } struct HoldState { @@ -591,8 +597,17 @@ struct HoldState { released: AtomicBool, } -/// A read held on its way to Drive, from [`FakeClient::hold_next_read`]. It -/// goes on when this is dropped. +/// Wait while `state` holds a call. +async fn wait_until_released(state: &HoldState) { + state.reached.store(true, Ordering::SeqCst); + while !state.released.load(Ordering::SeqCst) { + tokio::time::sleep(Duration::from_millis(5)).await; + } +} + +/// A call held on its way to or from Drive, from [`FakeClient::hold_next_read`] +/// or [`FakeClient::hold_reply_to_next_create`]. It goes on when this is +/// dropped. pub(crate) struct Held(Arc); impl Held { @@ -646,6 +661,23 @@ impl FakeClient { 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 { + let state = Arc::new(HoldState { + uid: self.0.drive.root(), + reached: AtomicBool::new(false), + released: AtomicBool::new(false), + }); + *self.0.hold_create_reply.lock() = Some(state.clone()); + Held(state) + } + + /// Make the next revision of a file on Drive and lose its answer. + pub(crate) fn lose_reply_to_next_revision(&self) { + self.0.lose_revision_reply.store(true, Ordering::SeqCst); + } + /// Wait here if `uids` is the read a [`Held`] is for. async fn wait_if_held(&self, uids: &[NodeUid]) { let state = { @@ -656,10 +688,7 @@ impl FakeClient { } }; let Some(state) = state else { return }; - state.reached.store(true, Ordering::SeqCst); - while !state.released.load(Ordering::SeqCst) { - tokio::time::sleep(Duration::from_millis(5)).await; - } + wait_until_released(&state).await; } /// Requests made through this client so far. @@ -1073,6 +1102,12 @@ impl DriveApi for FakeClient { if made.is_ok() && self.0.lose_create_reply.swap(false, Ordering::SeqCst) { return Err(no_answer()); } + let held = self.0.hold_create_reply.lock().take(); + if made.is_ok() + && let Some(state) = held + { + wait_until_released(&state).await; + } made } @@ -1129,7 +1164,11 @@ impl DriveApi for FakeClient { server.put(file_uid, Some(remote)); Ok(()) }) - .await + .await?; + if self.0.lose_revision_reply.swap(false, Ordering::SeqCst) { + return Err(no_answer()); + } + Ok(()) } async fn open_revision(&self, uid: &NodeUid) -> Result> { diff --git a/docs/BUGS.md b/docs/BUGS.md index 039c503..8327295 100644 --- a/docs/BUGS.md +++ b/docs/BUGS.md @@ -12,6 +12,46 @@ Conventions: --- +## B155 — A write whose upload lost its answer lands as a conflict copy of itself + +**Status:** Fixed (unverified). +**Found:** 2026-10-04, by the simulation's Wi-Fi profile (`one_client_on_a_slow_link`, seed 2, +second replay on 7269cb5). `x/z/y/a.txt` was written after its create landed. The revision upload +reached Drive, but the answer was lost and the call timed out. On the retry, Drive held +`a (sync-conflict …).txt` beside the file, with the same bytes. + +**Where:** `Core::revision_conflict` in `crates/pdfs-fuse/src/drain.rs`. + +**Cause.** A queued write remembers the revision it was made against. The retry found the file one +revision further, sealed by the lost upload, which this daemon never heard of, so it was not +recorded as our own (`own_sealed_rev`). The write was taken for a change from elsewhere and kept as +a conflict copy. + +**Fix.** A complete write whose bytes the remote already holds, by size and SHA-1, is not a +conflict: it is sent again as a revision of the same bytes. Sim test +`a_write_whose_answer_was_lost_lands_once`, with the fake Drive's new +`lose_reply_to_next_revision`. + +## B154 — A file made under the name of a create on the wire lands as a conflict copy + +**Status:** Fixed (unverified). +**Found:** 2026-10-04, by CI on 7269cb5 (`one_client_on_a_good_link`, seed 9, every run). `c.bin` +was renamed to `d` while its create was on the wire, and a new `c.bin` was made. The new file landed +as `c (sync-conflict …).bin`, the mount showed that name, and `unlink c.bin` answered `ENOENT`. + +**Where:** the failure path of `Core::run_pending_drain` in `crates/pdfs-fuse/src/drain.rs`. + +**Cause.** A regression from the B153 fix. The new file's create found `c.bin` held by the first +create, which was still on the wire, and failed as a wait. The B153 fix then woke the ops waiting +for the name. The database cannot see a create on the wire, only queued trashes and renames, so +it found the name free and the op was due again at once. Four waits went in 70 ms, and the create +took a conflict name. + +**Fix.** No wake while another create on the wire holds the name. It frees the name when it +lands, or hands it to the rename queued behind it, which wakes the waiters when it lands. Sim +test `a_file_made_under_the_name_of_a_create_on_the_wire_waits_for_it`, with the fake Drive's +new `hold_reply_to_next_create`. + ## B153 — An op waiting for a name sat out its backoff after the name was free **Status:** Fixed (unverified). diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 6ccd4b5..634cd78 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -33,6 +33,10 @@ folder by local id too. for this release. ### Fixed +- **A file made under the name of a file whose upload is still going out keeps its name.** It + no longer lands as a conflict copy, and the mount no longer shows that copy's name (B154). +- **A change to a file no longer lands as a conflict copy of itself when the link loses Drive's + answer to its upload** (B155). - **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 aa26126..0ab14b1 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 B153. +- Fixes from the simulation and account runs since then: B127 and B141 to B155. - **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`. @@ -49,9 +49,11 @@ Tracks `docs/MILESTONE-3.0.0.md`. Update it with every commit that moves a phase - [ ] Simulation passes on the Wi-Fi profile: `one_client_on_a_slow_link` failed in CI on seeds 2 and 3. Two bugs (B127, B151) and a harness gap: a settle did not read the log, so it could not explain a known bug's damage. All three fixed; not yet confirmed in CI. - Replays then found two races on the LAN profile too (B152, B153), both fixed. One - slow-link run of seed 2 failed once with a rename answering `ENOENT` as a folder move - landed. Not reproduced in 8 runs since; it may be B152. + Replays then found two races on the LAN profile too (B152, B153), both fixed. CI on + 7269cb5 failed LAN seed 9, a regression from the B153 fix (B154), and Wi-Fi seed 2, + a write whose answer was lost (B155). Both fixed. One slow-link run of seed 2 failed + once with a rename answering `ENOENT` as a folder move landed. Not reproduced since; + it may be B152. - [ ] 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)