mirror of
https://github.com/narrrl/proton-drive-linux.git
synced 2026-11-04 00:47:00 +01:00
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.
This commit is contained in:
1 parent
4820ea1236
commit
519ec1af07
2 files changed
+43
-5
No files matched your search
@@ -389,11 +389,11 @@ impl<R: Read> Read for CountingReader<'_, R> {
|
|||||||
&& cancel.load(Ordering::Relaxed)
|
&& cancel.load(Ordering::Relaxed)
|
||||||
{
|
{
|
||||||
// Not `Ok(0)`: a short read is how this reader says "end of file",
|
// Not `Ok(0)`: a short read is how this reader says "end of file",
|
||||||
// and the SDK would seal a truncated revision from it.
|
// and the SDK would seal a truncated revision from it. Not
|
||||||
return Err(std::io::Error::new(
|
// `Interrupted` either: every reader loop, the SDK's included,
|
||||||
std::io::ErrorKind::Interrupted,
|
// retries that kind, and this flag never clears, so the drain
|
||||||
"upload superseded by a newer write",
|
// 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)?;
|
let n = self.inner.read(buf)?;
|
||||||
self.guard.add(n as u64);
|
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_millis(500));
|
||||||
assert_eq!(limit.charge(1500, now), Duration::from_secs(2));
|
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);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
@@ -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/<pid>/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
|
## 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
|
**Status:** Fixed (unverified) — the daemon carrying the change has not been driven against a real
|
||||||
|
|||||||
Reference in new issue
Block a user