mirror of
https://github.com/narrrl/proton-drive-linux.git
synced 2026-11-03 16:41:26 +01:00
fix(fuse): keep the size of a file open for writing across re-listings
B109: an online create seals an empty file on Drive, and until the last close the written bytes live only in the open handle. A re-listing of the parent, a size upgrade or a create landing re-interned the node with Drive's empty size, the kernel capped reads at that size, and SQLite reported "database disk image is malformed". State::keep_open_write_size now carries the handle's size and local mtime over in all three places. The acceptance script waits for My files to mount after a daemon restart instead of failing at once.
This commit is contained in:
1 parent
a2992b3b9d
commit
f252b09757
6 files changed
+83
-9
No files matched your search
@@ -1060,9 +1060,11 @@ impl Core {
|
||||
let Some(&ino) = st.by_uid.get(real) else {
|
||||
return;
|
||||
};
|
||||
let mut landed = node.clone();
|
||||
st.keep_open_write_size(ino, &mut landed);
|
||||
if let Some(e) = st.entries.get_mut(&ino) {
|
||||
e.uid = real.clone();
|
||||
e.node = node.clone();
|
||||
e.node = landed;
|
||||
}
|
||||
// Where the op said the node goes and where it actually landed can
|
||||
// differ — a conflict re-homes it — so the tree follows the parent
|
||||
|
||||
@@ -2695,6 +2695,10 @@ impl Core {
|
||||
continue;
|
||||
};
|
||||
changed.push(ino);
|
||||
// An open write handle is ahead of the server too (B109).
|
||||
if st.active_writes.contains_key(&ino) {
|
||||
continue;
|
||||
}
|
||||
let NodeKind::File { claimed_size, .. } = &node.kind else {
|
||||
continue;
|
||||
};
|
||||
|
||||
@@ -800,12 +800,13 @@ impl State {
|
||||
self.intern_mem_with_access(parent, node, access)
|
||||
}
|
||||
|
||||
fn intern_mem_with_access(&mut self, parent: u64, node: Node, access: Access) -> u64 {
|
||||
fn intern_mem_with_access(&mut self, parent: u64, mut node: Node, access: Access) -> u64 {
|
||||
if let Some(&ino) = self.by_uid.get(&node.uid) {
|
||||
let changed = self
|
||||
.entries
|
||||
.get(&ino)
|
||||
.is_some_and(|entry| entry.access != access);
|
||||
self.keep_open_write_size(ino, &mut node);
|
||||
if let Some(e) = self.entries.get_mut(&ino) {
|
||||
e.node = node;
|
||||
e.parent = parent;
|
||||
@@ -850,6 +851,23 @@ impl State {
|
||||
|
||||
/// Allocate (or reuse) a stable inode for a node that came *from* the
|
||||
/// database, which is why nothing is written back.
|
||||
/// Carry the size and mtime of a file open for writing over onto `node`,
|
||||
/// which is about to replace its entry.
|
||||
///
|
||||
/// The open handle is ahead of anything Drive can report until release
|
||||
/// queues it. Taking the remote's size shrank a SQLite database to its empty
|
||||
/// create while SQLite held it open, and the kernel then capped every read
|
||||
/// at that size (docs/BUGS.md B109).
|
||||
pub(crate) fn keep_open_write_size(&self, ino: u64, node: &mut Node) {
|
||||
let (Some(aw), Some(entry)) = (self.active_writes.get(&ino), self.entries.get(&ino)) else {
|
||||
return;
|
||||
};
|
||||
if let NodeKind::File { claimed_size, .. } = &mut node.kind {
|
||||
*claimed_size = Some(aw.len as i64);
|
||||
node.modification_time = entry.node.modification_time;
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn intern_from_db(&mut self, parent: u64, node: Node) -> u64 {
|
||||
self.intern_mem(parent, node)
|
||||
}
|
||||
@@ -1900,6 +1918,27 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_listing_does_not_shrink_a_file_open_for_writing() {
|
||||
let (mut st, _dir) = state();
|
||||
let ino = st.intern(0, node("file", "root", "workload.db", false));
|
||||
let mut h = handle(16384, 0);
|
||||
h.ino = ino;
|
||||
st.active_writes.insert(ino, h);
|
||||
st.set_size(ino, 16384);
|
||||
st.touch_mtime(ino, 500);
|
||||
|
||||
// The remote still holds the empty file the create sealed.
|
||||
st.intern(0, node("file", "root", "workload.db", false));
|
||||
assert_eq!(crate::node_size(&st.entries[&ino].node), 16384);
|
||||
assert_eq!(st.entries[&ino].node.modification_time, 500);
|
||||
|
||||
// Once the handle is gone the remote is authoritative again.
|
||||
st.active_writes.remove(&ino);
|
||||
st.intern(0, node("file", "root", "workload.db", false));
|
||||
assert_eq!(crate::node_size(&st.entries[&ino].node), 0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_write_past_the_end_authors_the_hole_it_leaves() {
|
||||
// Opened over a 100-byte base, shrunk to 10, then written at 50: bytes
|
||||
|
||||
+23
-5
@@ -12,6 +12,29 @@ Conventions:
|
||||
|
||||
---
|
||||
|
||||
## B109 — A re-listing shrinks a file that is open for writing, and SQLite reads it as malformed
|
||||
|
||||
**Status:** Fixed (unverified).
|
||||
**Found:** 2026-09-29, by the acceptance case "application workloads" on My files. It failed
|
||||
several runs in a row with `database disk image is malformed` at the `SELECT COUNT(*)` right
|
||||
after a 500-row insert, and the daemon logged nothing. A diagnosis added to the case showed
|
||||
`workload.db` at 0 bytes while SQLite still held it open with 16 KiB written.
|
||||
|
||||
**Where:** `crates/pdfs-fuse/src/state.rs` (`State::keep_open_write_size`, called from
|
||||
`intern_mem_with_access`), the local-uid remap in `crates/pdfs-fuse/src/drain.rs`, and the size
|
||||
upgrade in `crates/pdfs-fuse/src/lib.rs`.
|
||||
|
||||
**Cause.** An online create seals an empty file on Drive. Until the last close, the written
|
||||
bytes live only in the open handle, and the entry's size is the handle's. Any re-listing of the
|
||||
parent re-interned the node from Drive, which replaced that size with 0. Such re-listings come
|
||||
from event echoes and sibling creates, and SQLite's journal is one. The kernel then got size 0
|
||||
from `getattr` and ended SQLite's reads at EOF without asking the daemon, so the database read
|
||||
back as empty pages. Queued writes were already protected (B11); open handles were not.
|
||||
|
||||
**Fix.** A node that has an open write handle keeps the handle's size and its local mtime when
|
||||
a listing, a size upgrade or a create landing replaces its entry. After the last close, the
|
||||
queued write takes over as before.
|
||||
|
||||
## B108 — A file deleted during a mirror pass puts the sync folder in `error`
|
||||
|
||||
**Status:** Fixed (unverified).
|
||||
@@ -31,11 +54,6 @@ ended in `error` and logged "item(s) failed; will retry" for files nobody wanted
|
||||
does not count as an error. The next pass sees the deletion and handles it like any other.
|
||||
Other open failures are still errors.
|
||||
|
||||
**Also open from the same run.** "application workloads" on My files failed with
|
||||
`database disk image is malformed` at the `SELECT` after the insert. The daemon logged nothing
|
||||
for it. The case now reports the step that failed, the database and journal files beside it,
|
||||
and which pages differ from a local replay of the same statements.
|
||||
|
||||
## B107 — One read on a stalled connection hangs the reading program for half an hour
|
||||
|
||||
**Status:** Fixed (unverified).
|
||||
|
||||
@@ -40,6 +40,9 @@ scratch (user data in `staging/` and `recovery/` is never touched by this).
|
||||
- **Editors can save over a file several times in a row.** The fix for B104 could leave the
|
||||
upload of a replaced file queued, and the next save onto the same name failed with
|
||||
`Input/output error` (B106).
|
||||
- **Databases and other files kept open while written no longer read back empty.** When the
|
||||
folder was listed again while a file was still open, its size dropped to what Drive held,
|
||||
often 0 bytes, and SQLite reported `database disk image is malformed` (B109).
|
||||
- **Deleting files during a mirror pass no longer marks the folder as failed.** A file removed
|
||||
after the pass planned its upload was counted as an error, so the folder showed `error` until
|
||||
a later pass (B108).
|
||||
|
||||
@@ -3488,9 +3488,17 @@ def run_account(reference: Run, args) -> None:
|
||||
"no running pdfs daemon: start proton-drive.service and sign in, "
|
||||
"or pass --offline-only for the account-free contract"
|
||||
)
|
||||
status = daemon.status()
|
||||
# A daemon that was just (re)started answers before it has mounted My
|
||||
# files; give it the sync timeout to get there rather than failing at once.
|
||||
deadline = time.monotonic() + timeout
|
||||
while True:
|
||||
status = daemon.status()
|
||||
mountpoint = (status.get("mount") or {}).get("mountpoint")
|
||||
ready = bool(mountpoint) and is_mountpoint(Path(mountpoint))
|
||||
if ready or not status.get("logged_in") or time.monotonic() >= deadline:
|
||||
break
|
||||
time.sleep(1)
|
||||
check(bool(status.get("logged_in")), "the daemon is not signed in; run `pdfs login` first")
|
||||
mountpoint = (status.get("mount") or {}).get("mountpoint")
|
||||
check(
|
||||
bool(mountpoint) and is_mountpoint(Path(mountpoint)),
|
||||
f"My files is not mounted (daemon reports {mountpoint!r})",
|
||||
|
||||
Reference in new issue
Block a user