mirror of
https://github.com/narrrl/proton-drive-linux.git
synced 2026-11-03 16:41:26 +01:00
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EuyxByBpK3fSWtyPkugeHz
This commit is contained in:
4 files changed
+87
-6
No files matched your search
@@ -1267,8 +1267,12 @@ impl ProtonFs {
|
|||||||
//
|
//
|
||||||
// Complete pending blobs are the common case and can be copied without
|
// Complete pending blobs are the common case and can be copied without
|
||||||
// any network access. An incomplete blob still has gaps referring to
|
// any network access. An incomplete blob still has gaps referring to
|
||||||
// the remote base; stacking another write on it cannot be represented
|
// the remote base, and stacking another write on it cannot be
|
||||||
// safely by WriteHandle, so fail the open rather than risk corruption.
|
// 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
|
// 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
|
// 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();
|
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() {
|
let (file, path) = match self.core.cache.create_scratch() {
|
||||||
Ok(x) => x,
|
Ok(x) => x,
|
||||||
Err(e) => {
|
Err(e) => {
|
||||||
@@ -1325,6 +1325,25 @@ impl ProtonFs {
|
|||||||
"copy queued revision into write scratch failed");
|
"copy queued revision into write scratch failed");
|
||||||
break 'install Err(Errno::EIO);
|
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);
|
initial_written.add(0, pending.meta.len);
|
||||||
}
|
}
|
||||||
// Sampled *before* the state lock, never under it: no site in
|
// Sampled *before* the state lock, never under it: no site in
|
||||||
|
|||||||
@@ -451,6 +451,36 @@ mod tests {
|
|||||||
let _ = std::fs::remove_dir_all(&dir);
|
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]
|
#[test]
|
||||||
#[ignore = "mounts FUSE: run with `cargo test -p pdfs-fuse sim:: -- --ignored`"]
|
#[ignore = "mounts FUSE: run with `cargo test -p pdfs-fuse sim:: -- --ignored`"]
|
||||||
fn a_file_deleted_offline_while_open_still_reads_and_takes_writes() {
|
fn a_file_deleted_offline_while_open_still_reads_and_takes_writes() {
|
||||||
|
|||||||
@@ -100,6 +100,7 @@ a live account since.
|
|||||||
|
|
||||||
| Entry | Title |
|
| 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 |
|
| [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 |
|
| [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 |
|
| [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.
|
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
|
### B203 — `pdfs unlock` leaves a running daemon a spent refresh token
|
||||||
|
|
||||||
**Status:** Fixed, not verified live
|
**Status:** Fixed, not verified live
|
||||||
|
|||||||
@@ -12,6 +12,10 @@ scratch (user data in `staging/` and `recovery/` is never touched by this).
|
|||||||
## [Unreleased]
|
## [Unreleased]
|
||||||
|
|
||||||
### Fixed
|
### 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
|
- **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
|
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.
|
once you are done. `pdfs login` now handles a CAPTCHA the same way instead of failing.
|
||||||
|
|||||||
Reference in new issue
Block a user