From 3e0db8cf4635676b751f875ffbbbb0d0cab58d74 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 9 Oct 2026 11:36:46 +0000 Subject: [PATCH] fix(fuse): fill an incomplete queued revision's gaps on a write open A partial write whose base was not in the content cache queued an incomplete revision, and every write open over it was refused with EIO until the drain uploaded it. CI's one-client-lan simulation hit this on seed 1: a create over a file whose partial edit had not drained yet. The write open now copies the blob and fills its gaps from the remote base with Core::fill_gaps, as the drain does, and fails only when that fill fails (B204). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01EuyxByBpK3fSWtyPkugeHz --- crates/pdfs-fuse/src/filesystem.rs | 31 ++++++++++++++++++++++++------ crates/pdfs-fuse/src/sim/daemon.rs | 30 +++++++++++++++++++++++++++++ docs/BUGS.md | 28 +++++++++++++++++++++++++++ docs/CHANGELOG.md | 4 ++++ 4 files changed, 87 insertions(+), 6 deletions(-) diff --git a/crates/pdfs-fuse/src/filesystem.rs b/crates/pdfs-fuse/src/filesystem.rs index 1b66c9e..9dd60cb 100644 --- a/crates/pdfs-fuse/src/filesystem.rs +++ b/crates/pdfs-fuse/src/filesystem.rs @@ -1267,8 +1267,12 @@ impl ProtonFs { // // Complete pending blobs are the common case and can be copied without // any network access. An incomplete blob still has gaps referring to - // the remote base; stacking another write on it cannot be represented - // safely by WriteHandle, so fail the open rather than risk corruption. + // the remote base, and stacking another write on it cannot be + // represented by WriteHandle, so the copy's gaps are filled from that + // base first, as the drain fills them before an upload. Only when that + // fails (offline, or the base is gone) does the open fail: refusing + // every such open failed a create over a file whose partial edit had + // not drained yet (`docs/BUGS.md` B204). // // Reading that blob and installing the handle cannot be done under one // lock — the copy is the size of the file — so the base is re-checked at @@ -1302,10 +1306,6 @@ impl ProtonFs { } } let pending_base = self.core.pending.lock().get(&uid).cloned(); - if pending_base.as_ref().is_some_and(|p| !p.meta.complete) { - error!(%uid, "refusing write over incomplete queued revision"); - break 'install Err(Errno::EIO); - } let (file, path) = match self.core.cache.create_scratch() { Ok(x) => x, Err(e) => { @@ -1325,6 +1325,25 @@ impl ProtonFs { "copy queued revision into write scratch failed"); break 'install Err(Errno::EIO); } + if !pending.meta.complete { + let mut authored = Intervals::default(); + for &(s, e) in &pending.meta.authored { + authored.add(s, e); + } + if let Err(error) = self.core.fill_gaps( + &uid, + &file, + pending.meta.len, + pending.meta.base_mtime, + pending.meta.base_size, + &authored, + ) { + let _ = std::fs::remove_file(&path); + error!(%uid, ?error, "cannot complete the queued revision a write open is based on"); + break 'install Err(error); + } + debug!(%uid, "write open completed an incomplete queued revision"); + } initial_written.add(0, pending.meta.len); } // Sampled *before* the state lock, never under it: no site in diff --git a/crates/pdfs-fuse/src/sim/daemon.rs b/crates/pdfs-fuse/src/sim/daemon.rs index 1cd63d5..3e7bc4c 100644 --- a/crates/pdfs-fuse/src/sim/daemon.rs +++ b/crates/pdfs-fuse/src/sim/daemon.rs @@ -451,6 +451,36 @@ mod tests { let _ = std::fs::remove_dir_all(&dir); } + #[test] + #[ignore = "mounts FUSE: run with `cargo test -p pdfs-fuse sim:: -- --ignored`"] + fn a_partial_write_over_bytes_never_read_takes_another_write_at_once() { + // Nothing here ever read the file, so the release of a partial write + // had no cached base to fill its gaps from and queued an incomplete + // revision. The next write open on it failed with EIO (B204). + let drive = FakeDrive::new(); + drive.device().write("f.txt", b"0123456789"); + let dir = scratch("partial-write-unread-base"); + 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()))); + + write_at(&path, 2, b"ab").unwrap(); + write_at(&path, 6, b"cd").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"01ab45cd89") + }), + "{:?} {:?}", + 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_file_deleted_offline_while_open_still_reads_and_takes_writes() { diff --git a/docs/BUGS.md b/docs/BUGS.md index 10d0c81..f18e3ea 100644 --- a/docs/BUGS.md +++ b/docs/BUGS.md @@ -100,6 +100,7 @@ a live account since. | Entry | Title | |---|---| +| [B204](#b204--a-write-over-a-partial-edit-whose-base-was-never-read-fails-with-eio) | A write over a partial edit whose base was never read fails with `EIO` | | [B203](#b203--pdfs-unlock-leaves-a-running-daemon-a-spent-refresh-token) | `pdfs unlock` leaves a running daemon a spent refresh token | | [B202](#b202--a-refused-session-shows-as-offline-forever-and-nothing-says-to-sign-in-again) | A refused session shows as offline forever, and nothing says to sign in again | | [B201](#b201--after-signing-in-again-the-old-daemon-puts-its-spent-tokens-back) | After signing in again, the old daemon puts its spent tokens back | @@ -306,6 +307,33 @@ a live account since. Newest first. +### B204 — A write over a partial edit whose base was never read fails with `EIO` + +**Status:** Fixed, not verified live +**Found:** 2026-10-09, by CI on 2a86830 (`one_client_on_a_good_link`, seed 1, which did not replay +locally). A partial write to `x/f.txt`, a file renamed twice before, was renamed over `z/d`. A +create over `z/d` a moment later failed with `EIO`, and the daemon logged "refusing write over +incomplete queued revision". +**Where:** `ProtonFs::serve_open` in `crates/pdfs-fuse/src/filesystem.rs` + +**Repro:** Write at an offset into a file this client never read, then open it for writing again +before the drain uploads the first write. + +**Cause.** The release of a partial write fills its gaps from the content cache only. When the +cache does not hold the base, the queued revision stays incomplete until the drain fills it from +Drive, and a write open over an incomplete revision was refused. B123, B157, B169 and B170 each +closed one way the cache could miss the base. Any other miss, such as a file never read here, +still failed the next write open for the length of the drain debounce. + +**Fix.** A write open over an incomplete queued revision copies the blob and fills its gaps from +the remote base with `Core::fill_gaps`, as the drain does before an upload. The open fails only +when that fill fails, for example offline. + +**Test:** `a_partial_write_over_bytes_never_read_takes_another_write_at_once` in `sim/daemon.rs`. +Without the fix the second write fails with `EIO`. + +--- + ### B203 — `pdfs unlock` leaves a running daemon a spent refresh token **Status:** Fixed, not verified live diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 3ecdd36..366eddc 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -12,6 +12,10 @@ scratch (user data in `staging/` and `recovery/` is never touched by this). ## [Unreleased] ### Fixed +- **Writing to a file again soon after a partial write could fail with an I/O error.** When the + first write changed only part of a file this client had not read, a second write, or saving + over the file, failed until the first one reached Drive. It now fetches the rest of the file + and goes ahead. - **A CAPTCHA could not be solved on hardened systems.** When the verification page cannot open in the app, as on secureblue, the app now offers it in your browser and continues the sign-in once you are done. `pdfs login` now handles a CAPTCHA the same way instead of failing.