From 519ec1af077085993d8d522f9d5377bfd91ff367 Mon Sep 17 00:00:00 2001 From: Nils Pukropp Date: Wed, 23 Sep 2026 17:58:06 +0200 Subject: [PATCH] fix(fuse): stop a superseded upload from spinning its drain thread A write that lands while a revision is on the wire cancels that upload through the flag CountingReader checks before each read. The cancel was reported as ErrorKind::Interrupted, which every read loop retries, including the SDK's read_full_block. The flag never clears, so the drain thread spun at 100% CPU forever and the transfer stayed listed as uploading at 0 bytes until the daemon restarted. A growing browser download (.crdownload) triggers it naturally. Report the cancel as ErrorKind::Other so the SDK fails the upload; drain_revision already treats a cancelled upload as abandoned, not failed. Tracked as B98. --- crates/pdfs-fuse/src/transfers.rs | 23 ++++++++++++++++++----- docs/BUGS.md | 25 +++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/crates/pdfs-fuse/src/transfers.rs b/crates/pdfs-fuse/src/transfers.rs index 2f483ad..078f020 100644 --- a/crates/pdfs-fuse/src/transfers.rs +++ b/crates/pdfs-fuse/src/transfers.rs @@ -389,11 +389,11 @@ impl Read for CountingReader<'_, R> { && cancel.load(Ordering::Relaxed) { // Not `Ok(0)`: a short read is how this reader says "end of file", - // and the SDK would seal a truncated revision from it. - return Err(std::io::Error::new( - std::io::ErrorKind::Interrupted, - "upload superseded by a newer write", - )); + // and the SDK would seal a truncated revision from it. Not + // `Interrupted` either: every reader loop, the SDK's included, + // retries that kind, and this flag never clears, so the drain + // thread spun forever with the transfer stuck at 0 bytes (B98). + return Err(std::io::Error::other("upload superseded by a newer write")); } let n = self.inner.read(buf)?; self.guard.add(n as u64); @@ -433,4 +433,17 @@ mod tests { assert_eq!(limit.charge(1500, now), Duration::from_millis(500)); assert_eq!(limit.charge(1500, now), Duration::from_secs(2)); } + + #[test] + fn a_cancelled_upload_fails_its_read_instead_of_asking_for_a_retry() { + let reg = TransferRegistry::new(); + let guard = reg.begin("f", "", TransferDirection::Upload, 4); + let cancel = Arc::new(AtomicBool::new(true)); + let mut reader = CountingReader::new(&b"data"[..], &guard).with_cancel(cancel); + // `read_exact` retries `Interrupted` the way the SDK's block loop does, + // so this hangs rather than fails if the cancel asks for a retry. + let err = reader.read_exact(&mut [0; 4]).unwrap_err(); + assert_ne!(err.kind(), std::io::ErrorKind::Interrupted); + assert_eq!(guard.entry.done.load(Ordering::Relaxed), 0); + } } diff --git a/docs/BUGS.md b/docs/BUGS.md index 7c0520f..5bc7129 100644 --- a/docs/BUGS.md +++ b/docs/BUGS.md @@ -12,6 +12,31 @@ Conventions: --- +## B98 — A superseded upload spins its drain thread forever and shows as "Uploading" at 0 bytes + +**Status:** Fixed (unverified) — the daemon carrying the change has not been driven against a +superseded upload yet. +**Found:** 2026-09-23. The Sync page showed `Unconfirmed 654050.crdownload — 0 bytes of 23.8 MiB` +for hours after Brave had finished and renamed the download. `pdfs transfers` listed the same +entry, `pdfs diagnostics` showed every worker idle, and `/proc//task` showed `pdfs-drain-1` +in state `R` at 100% CPU, holding a deleted staging blob open. + +**Where:** `crates/pdfs-fuse/src/transfers.rs` (`CountingReader::read`). + +**Cause.** A write that lands while a revision is on the wire cancels that upload through the +flag `CountingReader` checks before every read. The reader reported the cancel as +`ErrorKind::Interrupted`. Every read loop treats that kind as "try again", including the SDK's +`read_full_block`, and the flag never clears, so the loop never ended. The `TransferGuard` was +never dropped, so the transfer stayed in the registry, and the drain thread was lost until the +daemon restarted. A browser download is the natural trigger: the `.crdownload` file keeps growing, +so each new write supersedes the upload of the previous one. + +**Fix.** The cancel is reported as `ErrorKind::Other`, which the SDK turns into an error. +`drain_revision` already recognises a cancelled upload and returns without counting a failure. +Regression test: `a_cancelled_upload_fails_its_read_instead_of_asking_for_a_retry`. + +--- + ## B97 — Photo refreshes stack up on the control socket, and shutdown turns the resolve pass into a burst of "undecryptable photo" warnings **Status:** Fixed (unverified) — the daemon carrying the change has not been driven against a real