mirror of
https://github.com/narrrl/proton-drive-linux.git
synced 2026-11-03 16:41:26 +01:00
fix(fuse): a file landing while its folder is pinned stays available offline (B208)
A folder pin reads each file's modification time and size before it downloads the file. If the file's create landed during the download, the landing set Drive's time, and the pin then stored the blob under the old time. The file showed as kept offline but read as stale. After each download, the pin now compares the file's time and size with the tree's. If they changed, it downloads the file again under the new values, at most three times. The simulation gains FakeClient::hold_next_download to force that order.
This commit is contained in:
1 parent
3e4886f965
commit
005f7ded1f
4 files changed
+146
-1
No files matched your search
@@ -4290,7 +4290,7 @@ impl Core {
|
||||
return;
|
||||
}
|
||||
job.detail(file.clone());
|
||||
if let Err(e) = self.fetch_content(&uid, &file, mtime, size) {
|
||||
if let Err(e) = self.fetch_pinned(&uid, &file, mtime, size) {
|
||||
warn!(%uid, error = %e, "pin folder: download failed");
|
||||
failed += 1;
|
||||
}
|
||||
@@ -4302,6 +4302,30 @@ impl Core {
|
||||
}
|
||||
}
|
||||
|
||||
/// Cache the file `uid` for a folder pin. Its `mtime` and `size` were read
|
||||
/// before the download, and a create landing or a remote edit can change
|
||||
/// them while it runs. The blob is then stored under the old ones and reads
|
||||
/// as stale, so the file is listed as kept but not available offline. Fetch
|
||||
/// it again under the new ones, a few times at most.
|
||||
fn fetch_pinned(&self, uid: &NodeUid, name: &str, mtime: i64, size: u64) -> CoreResult<()> {
|
||||
let mut fetched = (mtime, size);
|
||||
for _ in 0..3 {
|
||||
self.fetch_content(uid, name, fetched.0, fetched.1)?;
|
||||
let current = {
|
||||
let st = self.state();
|
||||
st.by_uid
|
||||
.get(uid)
|
||||
.and_then(|ino| st.entries.get(ino))
|
||||
.map(|e| (e.node.modification_time, node_size(&e.node)))
|
||||
};
|
||||
match current {
|
||||
Some(now) if now != fetched => fetched = now,
|
||||
_ => break,
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Every file in the subtree rooted at folder `ino`.
|
||||
fn pin_subtree_files(&self, ino: u64) -> CoreResult<Vec<(NodeUid, String, i64, u64)>> {
|
||||
let mut files: Vec<(NodeUid, String, i64, u64)> = Vec::new();
|
||||
|
||||
@@ -2186,6 +2186,81 @@ mod tests {
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
||||
#[test]
|
||||
#[ignore = "mounts FUSE: run with `cargo test -p pdfs-fuse sim:: -- --ignored`"]
|
||||
fn a_folder_pinned_while_a_create_in_it_lands_keeps_that_file() {
|
||||
// The pin's download of a file started before the file's landing set
|
||||
// its modification time from Drive, and ended after it: the blob was
|
||||
// stored under the old time, and the file never read as cached (B208).
|
||||
let drive = FakeDrive::new();
|
||||
let dir = scratch("pin-folder-landing");
|
||||
let daemon = Daemon::start(&dir, drive.client(1, Faults::lan())).unwrap();
|
||||
std::fs::create_dir(daemon.mountpoint.join("Docs")).unwrap();
|
||||
assert!(wait_until(Duration::from_secs(30), || {
|
||||
daemon.pending().is_ok_and(|items| items.is_empty())
|
||||
}));
|
||||
let entry = || match daemon.request(&Request::ListDir {
|
||||
path: "Docs".into(),
|
||||
}) {
|
||||
Ok(Response::Entries { entries }) => entries.into_iter().find(|e| e.name == "a.txt"),
|
||||
_ => None,
|
||||
};
|
||||
|
||||
// Drive stamps the file with the time it landed, a second after it
|
||||
// was written, so the landing moves its modification time.
|
||||
let paused = |paused| Request::SetSyncPaused {
|
||||
paused,
|
||||
until: None,
|
||||
};
|
||||
daemon.request(&paused(true)).unwrap();
|
||||
std::fs::write(daemon.mountpoint.join("Docs/a.txt"), b"alpha").unwrap();
|
||||
std::thread::sleep(Duration::from_millis(1100));
|
||||
let landing = daemon.client.hold_next_landing_of_any();
|
||||
daemon.request(&paused(false)).unwrap();
|
||||
assert!(wait_until(Duration::from_secs(30), || landing.reached()));
|
||||
let real = drive.lookup("Docs/a.txt").unwrap();
|
||||
let read_back = daemon.client.hold_next_read(&real);
|
||||
drop(landing);
|
||||
assert!(wait_until(Duration::from_secs(30), || read_back.reached()));
|
||||
let written = entry().unwrap().modified;
|
||||
|
||||
// Without the blob the landing kept, the pin downloads the file.
|
||||
daemon.request(&Request::PurgeCache).unwrap();
|
||||
|
||||
let download = daemon.client.hold_next_download(&real);
|
||||
take_logged();
|
||||
daemon
|
||||
.request(&Request::Pin {
|
||||
path: "Docs".into(),
|
||||
})
|
||||
.unwrap();
|
||||
assert!(wait_until(Duration::from_secs(30), || download.reached()));
|
||||
drop(read_back);
|
||||
assert!(wait_until(Duration::from_secs(30), || {
|
||||
entry().is_some_and(|e| e.modified != written)
|
||||
}));
|
||||
drop(download);
|
||||
let mut logged = Vec::new();
|
||||
assert!(
|
||||
wait_until(Duration::from_secs(30), || {
|
||||
logged.extend(take_logged());
|
||||
logged
|
||||
.iter()
|
||||
.any(|line| line.starts_with("pin folder: done"))
|
||||
}),
|
||||
"{logged:?}"
|
||||
);
|
||||
|
||||
assert!(
|
||||
entry().is_some_and(|e| e.cached),
|
||||
"the file kept through its folder is available offline: {:?}",
|
||||
entry()
|
||||
);
|
||||
|
||||
assert!(matches!(daemon.stop(), Ok(MountOutcome::Shutdown)));
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
||||
/// Register `local` as a synced folder in `mode` and wait until the daemon
|
||||
/// lists it that way, returning its id.
|
||||
fn sync_folder(daemon: &Daemon, local: &Path, mode: &str) -> i64 {
|
||||
|
||||
@@ -362,6 +362,7 @@ impl FakeDrive {
|
||||
hold: Mutex::new(None),
|
||||
hold_answer: Mutex::new(None),
|
||||
hold_trash: Mutex::new(None),
|
||||
hold_download: Mutex::new(None),
|
||||
hold_any_landing: AtomicBool::new(false),
|
||||
hold_landing: Mutex::new(None),
|
||||
drop_after_create: AtomicBool::new(false),
|
||||
@@ -641,6 +642,8 @@ struct ClientInner {
|
||||
hold_answer: Mutex<Option<Arc<HoldState>>>,
|
||||
/// The trash [`FakeClient::hold_next_trash`] is waiting for.
|
||||
hold_trash: Mutex<Option<Arc<HoldState>>>,
|
||||
/// The download [`FakeClient::hold_next_download`] is waiting for.
|
||||
hold_download: Mutex<Option<Arc<HoldState>>>,
|
||||
/// The landing [`FakeClient::hold_next_landing`] is waiting for.
|
||||
hold_landing: Mutex<Option<Arc<HoldState>>>,
|
||||
/// Set by [`FakeClient::hold_next_landing_of_any`]: hold whichever node
|
||||
@@ -783,6 +786,18 @@ impl FakeClient {
|
||||
Held(state)
|
||||
}
|
||||
|
||||
/// Hold the next download of `uid`'s content once Drive has answered it,
|
||||
/// before the bytes reach the daemon, until the answer is dropped.
|
||||
pub(crate) fn hold_next_download(&self, uid: &NodeUid) -> Held {
|
||||
let state = Arc::new(HoldState {
|
||||
uid: uid.clone(),
|
||||
reached: AtomicBool::new(false),
|
||||
released: AtomicBool::new(false),
|
||||
});
|
||||
*self.0.hold_download.lock() = Some(state.clone());
|
||||
Held(state)
|
||||
}
|
||||
|
||||
/// Hold the drain once the next upload to `uid` has landed and its blob
|
||||
/// is let go of, before the tree has the node (`DriveApi::landing`), until
|
||||
/// the answer is dropped.
|
||||
@@ -1432,6 +1447,8 @@ impl DriveApi for FakeClient {
|
||||
.ok_or_else(|| api(ResponseCode::DoesNotExist, "file not found"))
|
||||
})
|
||||
.await?;
|
||||
self.wait_if_held(&self.0.hold_download, std::slice::from_ref(uid))
|
||||
.await;
|
||||
output.write_all(&content).map_err(io_error)
|
||||
}
|
||||
|
||||
|
||||
@@ -100,6 +100,7 @@ a live account since.
|
||||
|
||||
| Entry | Title |
|
||||
|---|---|
|
||||
| [B208](#b208--a-file-landing-while-its-folder-is-pinned-is-kept-but-not-available-offline) | A file landing while its folder is pinned is kept but not available offline |
|
||||
| [B207](#b207--a-move-between-locations-goes-ahead-under-a-file-opened-just-after-its-create-landed) | A move between locations goes ahead under a file opened just after its create landed |
|
||||
| [B206](#b206--an-upgrade-leaves-the-old-daemon-running-and-nothing-says-so) | An upgrade leaves the old daemon running, and nothing says so |
|
||||
| [B205](#b205--a-photo-upload-the-sdk-refuses-says-youre-offline) | A photo upload the SDK refuses says "You're offline" |
|
||||
@@ -310,6 +311,34 @@ a live account since.
|
||||
|
||||
Newest first.
|
||||
|
||||
### B208 — A file landing while its folder is pinned is kept but not available offline
|
||||
|
||||
**Status:** Fixed, not verified live
|
||||
**Found:** 2026-10-11, CI simulation run of `3e4886f`.
|
||||
`sim::daemon::tests::a_pinned_folder_keeps_every_file_under_it_until_it_is_unpinned` waited
|
||||
30 s for `Docs/sub/b.txt` to read as cached and gave up. It failed about once in 25 runs under load
|
||||
and never alone.
|
||||
**Where:** `Core::pin_subtree` in `crates/pdfs-fuse/src/lib.rs`
|
||||
|
||||
**Repro:** Pin a folder while a create in it lands, after the drain has repointed the tree at
|
||||
the real uid and before it has read the node back. The file's cached blob must be gone, as after
|
||||
`pdfs cache clear`, so the pin downloads it.
|
||||
|
||||
**Cause.** The pin walks the folder first and downloads each file under the modification time
|
||||
and size it read then. The landing's read-back then sets the time Drive gave the file, and keeps
|
||||
the uploaded bytes under it. The pin's download ended after that and stored the blob again under
|
||||
the old time. The cache records one time per blob, so the file read as stale: shown as kept
|
||||
offline, but downloaded again on the next read, and unreadable offline.
|
||||
|
||||
**Fix.** After each download the pin compares the file's time and size with the tree's. If they
|
||||
moved, it fetches the file again under the new ones, at most three times.
|
||||
|
||||
**Test:** `sim::daemon::tests::a_folder_pinned_while_a_create_in_it_lands_keeps_that_file` holds
|
||||
the read-back and the pin's download with `FakeClient::hold_next_download`, new for this, and
|
||||
lets them go in that order.
|
||||
|
||||
---
|
||||
|
||||
### B207 — A move between locations goes ahead under a file opened just after its create landed
|
||||
|
||||
**Status:** Fixed, not verified live
|
||||
|
||||
Reference in new issue
Block a user