mirror of
https://github.com/narrrl/proton-drive-linux.git
synced 2026-11-03 16:41:26 +01:00
fix(fuse): do not take a new file Drive lists before its commit as empty (B163)
This commit is contained in:
1 parent
18587124ac
commit
b37d5e7928
6 files changed
+166
-4
No files matched your search
@@ -3130,10 +3130,19 @@ impl Core {
|
||||
|
||||
/// [`Core::fetch_node`] for a node Drive has just made, asked again for a
|
||||
/// moment while Drive does not list it yet.
|
||||
///
|
||||
/// A file Drive lists without an active revision is not listed yet either:
|
||||
/// the create commits one before it answers, so this answer is from before
|
||||
/// the commit, with no size. Taken, it emptied the new file in the tree,
|
||||
/// and a read of it right after found nothing (`docs/BUGS.md` B163).
|
||||
pub(crate) fn read_back(&self, uid: &NodeUid) -> Result<Node, Errno> {
|
||||
let mut delays = MINTED_READ_BACK_DELAYS.iter();
|
||||
loop {
|
||||
match self.fetch_node(uid) {
|
||||
let fetched = match self.fetch_node(uid) {
|
||||
Ok(node) if is_unrevised(&node) => Err(Errno::ENOENT),
|
||||
fetched => fetched,
|
||||
};
|
||||
match fetched {
|
||||
Err(e)
|
||||
if e == Errno::ENOENT
|
||||
&& let Some(delay) = delays.next() =>
|
||||
@@ -6259,6 +6268,18 @@ fn check_replaceable(src_dir: bool, dst_dir: bool, dst_empty: bool) -> Result<()
|
||||
}
|
||||
}
|
||||
|
||||
/// Whether `node` is a file Drive lists without an active revision, as it does
|
||||
/// one whose create has not been committed.
|
||||
fn is_unrevised(node: &Node) -> bool {
|
||||
matches!(
|
||||
node.kind,
|
||||
NodeKind::File {
|
||||
active_revision_id: None,
|
||||
..
|
||||
}
|
||||
)
|
||||
}
|
||||
|
||||
fn node_size(node: &Node) -> u64 {
|
||||
match &node.kind {
|
||||
NodeKind::Folder => 0,
|
||||
|
||||
@@ -494,6 +494,66 @@ mod tests {
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
||||
#[test]
|
||||
#[ignore = "mounts FUSE: run with `cargo test -p pdfs-fuse sim:: -- --ignored`"]
|
||||
fn a_file_read_back_before_drive_committed_it_keeps_its_bytes() {
|
||||
// Drive listed the new file a moment before its create was committed,
|
||||
// with no revision and no size. The drain took that answer for the
|
||||
// file, and a read right after the create landed found it empty
|
||||
// (B163).
|
||||
let drive = FakeDrive::new();
|
||||
let dir = scratch("read-back-unrevised");
|
||||
// The feed reports the file as it is long after the read.
|
||||
let faults = Faults {
|
||||
event_delay: Duration::from_secs(60),
|
||||
..Faults::lan()
|
||||
};
|
||||
let daemon = Daemon::start(&dir, drive.client(1, faults)).unwrap();
|
||||
let mnt = daemon.mountpoint.clone();
|
||||
// A folder made here is listed from the tree, not from Drive.
|
||||
std::fs::create_dir(mnt.join("x")).unwrap();
|
||||
assert!(
|
||||
wait_until(Duration::from_secs(30), || {
|
||||
daemon.pending().is_ok_and(|items| items.is_empty()) && drive.lookup("x").is_some()
|
||||
}),
|
||||
"{:?} {:?}",
|
||||
daemon.pending(),
|
||||
drive.tree()
|
||||
);
|
||||
take_logged();
|
||||
|
||||
let create = daemon.client.hold_reply_to_next_create();
|
||||
std::fs::write(mnt.join("x/c.bin"), b"abc").unwrap();
|
||||
assert!(wait_until(Duration::from_secs(30), || create.reached()));
|
||||
let uid = drive.lookup("x/c.bin").unwrap();
|
||||
// The read-back after the create, the one adopting it, and the size
|
||||
// fetch a listing makes.
|
||||
daemon.client.answer_reads_unrevised(&uid, 3);
|
||||
drop(create);
|
||||
let mut logged = Vec::new();
|
||||
assert!(
|
||||
wait_until(Duration::from_secs(30), || {
|
||||
logged.extend(take_logged());
|
||||
logged
|
||||
.iter()
|
||||
.any(|line| line.starts_with("pending create landed") && line.contains("c.bin"))
|
||||
}),
|
||||
"{logged:?}"
|
||||
);
|
||||
|
||||
// A listing refreshes the size the kernel holds, as the account run's
|
||||
// did before it read the file.
|
||||
let sizes: Vec<_> = std::fs::read_dir(mnt.join("x"))
|
||||
.unwrap()
|
||||
.map(|e| e.unwrap().metadata().unwrap().len())
|
||||
.collect();
|
||||
assert_eq!(sizes, [3]);
|
||||
assert_eq!(std::fs::read(mnt.join("x/c.bin")).unwrap(), b"abc");
|
||||
|
||||
assert!(matches!(daemon.stop(), Ok(MountOutcome::Shutdown)));
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
||||
#[test]
|
||||
#[ignore = "mounts FUSE: run with `cargo test -p pdfs-fuse sim:: -- --ignored`"]
|
||||
fn a_folder_made_again_while_the_old_one_is_trashed_is_a_new_folder() {
|
||||
|
||||
@@ -34,7 +34,9 @@
|
||||
//! A test can also hold the next read of one node on its way to Drive
|
||||
//! ([`FakeClient::hold_next_read`]), to make a change while the daemon waits
|
||||
//! for the answer, or on its way back ([`FakeClient::hold_answer_to_next_read`]),
|
||||
//! so the daemon gets an answer a change made meanwhile has overtaken.
|
||||
//! so the daemon gets an answer a change made meanwhile has overtaken. It can
|
||||
//! also have a new file's next reads answered as Drive lists it before its
|
||||
//! create is committed ([`FakeClient::answer_reads_unrevised`]; B163).
|
||||
|
||||
use std::collections::{BTreeMap, HashMap};
|
||||
use std::io::{Read, Write};
|
||||
@@ -339,6 +341,7 @@ impl FakeDrive {
|
||||
lose_revision_reply: AtomicBool::new(false),
|
||||
lose_revision_read_back: AtomicBool::new(false),
|
||||
lose_read: Mutex::new(None),
|
||||
unrevised_reads: Mutex::new(None),
|
||||
}))
|
||||
}
|
||||
|
||||
@@ -602,6 +605,9 @@ struct ClientInner {
|
||||
lose_revision_read_back: AtomicBool,
|
||||
/// The file whose next read goes unanswered, once its revision is made.
|
||||
lose_read: Mutex<Option<NodeUid>>,
|
||||
/// Set by [`FakeClient::answer_reads_unrevised`]: the file and how many
|
||||
/// more of its reads are answered without an active revision.
|
||||
unrevised_reads: Mutex<Option<(NodeUid, usize)>>,
|
||||
}
|
||||
|
||||
struct HoldState {
|
||||
@@ -723,6 +729,13 @@ impl FakeClient {
|
||||
self.0.lose_revision_read_back.store(true, Ordering::SeqCst);
|
||||
}
|
||||
|
||||
/// Answer the next `reads` reads of file `uid` alone that find it as Drive
|
||||
/// lists a file whose create it has not committed yet: no active revision,
|
||||
/// no size.
|
||||
pub(crate) fn answer_reads_unrevised(&self, uid: &NodeUid, reads: usize) {
|
||||
*self.0.unrevised_reads.lock() = Some((uid.clone(), reads));
|
||||
}
|
||||
|
||||
/// Wait here if `uids` is the read a [`Held`] in `slot` is for.
|
||||
async fn wait_if_held(&self, slot: &Mutex<Option<Arc<HoldState>>>, uids: &[NodeUid]) {
|
||||
let state = {
|
||||
@@ -982,6 +995,36 @@ impl DriveApi for FakeClient {
|
||||
})
|
||||
.await?;
|
||||
self.wait_if_held(&self.0.hold_answer, uids).await;
|
||||
let mut nodes = nodes;
|
||||
{
|
||||
let mut unrevised = self.0.unrevised_reads.lock();
|
||||
if let Some((uid, left)) = unrevised.as_mut()
|
||||
&& uids == std::slice::from_ref(uid)
|
||||
&& !nodes.is_empty()
|
||||
{
|
||||
*left -= 1;
|
||||
if *left == 0 {
|
||||
*unrevised = None;
|
||||
}
|
||||
for node in &mut nodes {
|
||||
if let NodeKind::File {
|
||||
total_size_on_storage,
|
||||
active_revision_state,
|
||||
active_revision_id,
|
||||
claimed_size,
|
||||
content_sha1,
|
||||
..
|
||||
} = &mut node.kind
|
||||
{
|
||||
*total_size_on_storage = 0;
|
||||
*active_revision_state = None;
|
||||
*active_revision_id = None;
|
||||
*claimed_size = None;
|
||||
*content_sha1 = None;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
self.cache(&nodes);
|
||||
Ok(nodes)
|
||||
}
|
||||
|
||||
@@ -12,6 +12,39 @@ Conventions:
|
||||
|
||||
---
|
||||
|
||||
## B163 — A file read right after its create landed is empty
|
||||
|
||||
**Status:** Fixed.
|
||||
**Found:** 2026-10-04, by the account run on 2.10.0 ("unusual but legal names", on-demand
|
||||
pairing). The case wrote some twenty files into a new folder, listed it and read each back.
|
||||
`-starts-with-dash` read 0 bytes for 17. Its create had landed with the bytes a moment before.
|
||||
No revision was queued for it and nothing was logged.
|
||||
|
||||
**Where:** `Core::read_back` in `crates/pdfs-fuse/src/lib.rs`.
|
||||
|
||||
**Cause.** After a create, the drain reads the new node back and adopts that answer into the
|
||||
tree. Drive can answer a read before it lists a node it has just made, so an empty answer is
|
||||
asked again (B143). It can also list the node before the create's revision is committed, as a
|
||||
file with no active revision: no claimed size and nothing on storage, so its size reads as 0.
|
||||
The adoption took that answer, and the tree showed the new file as empty. The folder was made
|
||||
here, so its listing came from the tree. The size fetch the listing starts met the same answer
|
||||
and left the 0. The kernel capped the read at it.
|
||||
|
||||
The run logged nothing for the file, so the stale answer is inferred, not observed. It is the
|
||||
one path found that leaves a landed file empty with no op queued.
|
||||
|
||||
**Fix.** `read_back` treats a file without an active revision like a node Drive does not list
|
||||
yet: it asks again after 250 ms and 1 s. If both answers are still without a revision, it
|
||||
reports the node as not found, and the drain adopts it later, as it does for a node Drive
|
||||
never lists in that time.
|
||||
|
||||
**Test:** simulation test `a_file_read_back_before_drive_committed_it_keeps_its_bytes` (failed
|
||||
before the fix: the listing showed the file with 0 bytes). The fake Drive can now answer a
|
||||
file's next reads as Drive lists a file that is not committed yet
|
||||
(`FakeClient::answer_reads_unrevised`).
|
||||
|
||||
---
|
||||
|
||||
## B162 — A folder removed and made again at once loses what goes into it
|
||||
|
||||
**Status:** Fixed.
|
||||
|
||||
@@ -53,6 +53,8 @@ by its local id, in the queue, the stored tree and pins alike.
|
||||
link (B161).
|
||||
- **A folder removed and made again at once keeps what goes into it.** It used to be created
|
||||
inside the removed folder and was trashed along with it (B162).
|
||||
- **A new file no longer reads as empty just after it is uploaded.** Drive can list it a moment
|
||||
before its contents are committed, and the mount took that answer (B163).
|
||||
- **A file renamed just as its upload finished keeps its new name.** The mount no longer showed
|
||||
the old name, and a second rename no longer failed with "No such file or directory" (B152).
|
||||
- **A change that waits for a name to be freed no longer waits seconds after it is.** A rename or
|
||||
|
||||
@@ -9,7 +9,7 @@ Tracks `docs/MILESTONE-3.0.0.md`. Update it with every commit that moves a phase
|
||||
- **Phase 2**: local ids, inode = local id, one tree for every mount (2.10.0).
|
||||
- **Phase 3, first part**: mkdir, create, rename, unlink and rmdir on the mount are recorded
|
||||
locally and sent from the queue; `"local_first": false` brings back the old path (5e0f30c).
|
||||
- Fixes from the simulation and account runs since then: B127 and B141 to B162.
|
||||
- Fixes from the simulation and account runs since then: B127 and B141 to B163.
|
||||
- **Sync issues, first cut** (§6.2): refusals sorted into issues, shown at once in the app,
|
||||
tray and CLI; Export button and `pdfs sync export`; `pdfs sync issues`.
|
||||
|
||||
@@ -97,7 +97,10 @@ Tracks `docs/MILESTONE-3.0.0.md`. Update it with every commit that moves a phase
|
||||
to reach Drive at once and now accepts queued ones. The move cases were the harness:
|
||||
it retried every source after one was busy, and expected a refusal where a mirror to
|
||||
mirror move carries the local copy along. The journal showed a conflict copy retrying
|
||||
into a deleted folder (B150, fixed). Not confirmed yet.
|
||||
into a deleted folder (B150, fixed). Not confirmed yet. The run on 2.10.0
|
||||
(2026-10-04: 260 passed, 3 failed) failed a new file that read back empty (B163,
|
||||
fixed), a 128-entry folder that listed 129, and its cleanup, which ran while the
|
||||
daemon was restarting.
|
||||
5. **Release**: 2.10.0 is tagged; the 2.9.0 tag still waits for the user's go-ahead.
|
||||
|
||||
## After 3.0.0
|
||||
|
||||
Reference in new issue
Block a user