mirror of
https://github.com/narrrl/proton-drive-linux.git
synced 2026-11-04 00:47:00 +01:00
test(fuse): a listing from before a create commits keeps the landed file (B166)
The account run's 0-byte read of a new file was B166 too: Drive's listing had the node from before its create was committed, and the queue no longer held it when read. The fake Drive can now hold a folder's listing on its way back with one file listed unrevised.
This commit is contained in:
1 parent
25c46cfe0a
commit
ffa0fe698d
5 files changed
+137
-33
No files matched your search
@@ -595,6 +595,60 @@ 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_listing_taken_before_a_create_commits_keeps_the_landed_file() {
|
||||||
|
// Drive listed a new file before its create was committed, with no
|
||||||
|
// revision, and the answer came back after the create landed: the
|
||||||
|
// queue no longer held the file, so Drive's empty node was taken, and
|
||||||
|
// a 17-byte file read back as 0 bytes (B166). The size upgrade's read
|
||||||
|
// still finds it so, and the daemon's own listing is checked: the
|
||||||
|
// kernel still has the size the write left.
|
||||||
|
let drive = FakeDrive::new();
|
||||||
|
let dir = scratch("listed-before-commit");
|
||||||
|
let daemon = Daemon::start(&dir, drive.client(1, Faults::lan())).unwrap();
|
||||||
|
let mnt = daemon.mountpoint.clone();
|
||||||
|
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()
|
||||||
|
}));
|
||||||
|
|
||||||
|
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()));
|
||||||
|
// Long enough for the next event poll to bring the file's event, which
|
||||||
|
// drops the folder's listing.
|
||||||
|
std::thread::sleep(crate::POLL_INTERVAL + Duration::from_secs(3));
|
||||||
|
let folder = drive.lookup("x").unwrap();
|
||||||
|
let uid = drive.lookup("x/c.bin").unwrap();
|
||||||
|
let listing = daemon.client.hold_answer_to_next_listing(&folder, &uid);
|
||||||
|
let lister = {
|
||||||
|
let x = mnt.join("x");
|
||||||
|
std::thread::spawn(move || std::fs::read_dir(x).unwrap().count())
|
||||||
|
};
|
||||||
|
assert!(wait_until(Duration::from_secs(30), || listing.reached()));
|
||||||
|
drop(create);
|
||||||
|
assert!(wait_until(Duration::from_secs(30), || {
|
||||||
|
daemon.pending().is_ok_and(|items| items.is_empty())
|
||||||
|
}));
|
||||||
|
daemon.client.answer_reads_unrevised(&uid, 1);
|
||||||
|
drop(listing);
|
||||||
|
assert_eq!(lister.join().unwrap(), 1);
|
||||||
|
|
||||||
|
let path = mnt.join("x").to_string_lossy().into_owned();
|
||||||
|
let Ok(Response::Entries { entries }) = daemon.request(&Request::ListDir { path }) else {
|
||||||
|
panic!("the daemon did not list x");
|
||||||
|
};
|
||||||
|
let sizes: Vec<_> = entries
|
||||||
|
.iter()
|
||||||
|
.map(|entry| (entry.name.as_str(), entry.size))
|
||||||
|
.collect();
|
||||||
|
assert_eq!(sizes, [("c.bin", 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]
|
#[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_folder_made_again_while_the_old_one_is_trashed_is_a_new_folder() {
|
fn a_folder_made_again_while_the_old_one_is_trashed_is_a_new_folder() {
|
||||||
|
|||||||
@@ -36,7 +36,9 @@
|
|||||||
//! for the answer, or on its way back ([`FakeClient::hold_answer_to_next_read`]),
|
//! 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. It can
|
//! 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
|
//! also have a new file's next reads answered as Drive lists it before its
|
||||||
//! create is committed ([`FakeClient::answer_reads_unrevised`]; B163).
|
//! create is committed ([`FakeClient::answer_reads_unrevised`]; B163), and
|
||||||
|
//! hold a folder's listing on its way back with one of its files listed that
|
||||||
|
//! way ([`FakeClient::hold_answer_to_next_listing`]).
|
||||||
|
|
||||||
use std::collections::{BTreeMap, HashMap};
|
use std::collections::{BTreeMap, HashMap};
|
||||||
use std::io::{Read, Write};
|
use std::io::{Read, Write};
|
||||||
@@ -342,6 +344,7 @@ impl FakeDrive {
|
|||||||
lose_revision_read_back: AtomicBool::new(false),
|
lose_revision_read_back: AtomicBool::new(false),
|
||||||
lose_read: Mutex::new(None),
|
lose_read: Mutex::new(None),
|
||||||
unrevised_reads: Mutex::new(None),
|
unrevised_reads: Mutex::new(None),
|
||||||
|
hold_listing: Mutex::new(None),
|
||||||
}))
|
}))
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -608,6 +611,9 @@ struct ClientInner {
|
|||||||
/// Set by [`FakeClient::answer_reads_unrevised`]: the file and how many
|
/// Set by [`FakeClient::answer_reads_unrevised`]: the file and how many
|
||||||
/// more of its reads are answered without an active revision.
|
/// more of its reads are answered without an active revision.
|
||||||
unrevised_reads: Mutex<Option<(NodeUid, usize)>>,
|
unrevised_reads: Mutex<Option<(NodeUid, usize)>>,
|
||||||
|
/// The listing [`FakeClient::hold_answer_to_next_listing`] is waiting
|
||||||
|
/// for, and the file it lists without an active revision.
|
||||||
|
hold_listing: Mutex<Option<(Arc<HoldState>, NodeUid)>>,
|
||||||
}
|
}
|
||||||
|
|
||||||
struct HoldState {
|
struct HoldState {
|
||||||
@@ -617,6 +623,26 @@ struct HoldState {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/// Wait while `state` holds a call.
|
/// Wait while `state` holds a call.
|
||||||
|
/// `node` as Drive lists a file whose create it has not committed yet: no
|
||||||
|
/// active revision, no size.
|
||||||
|
fn unrevise(node: &mut Node) {
|
||||||
|
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;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
async fn wait_until_released(state: &HoldState) {
|
async fn wait_until_released(state: &HoldState) {
|
||||||
state.reached.store(true, Ordering::SeqCst);
|
state.reached.store(true, Ordering::SeqCst);
|
||||||
while !state.released.load(Ordering::SeqCst) {
|
while !state.released.load(Ordering::SeqCst) {
|
||||||
@@ -736,6 +762,23 @@ impl FakeClient {
|
|||||||
*self.0.unrevised_reads.lock() = Some((uid.clone(), reads));
|
*self.0.unrevised_reads.lock() = Some((uid.clone(), reads));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Answer the next listing of `folder`'s nodes with what Drive holds now,
|
||||||
|
/// `unrevised` in it as Drive lists a file whose create it has not
|
||||||
|
/// committed yet, then hold the answer until it is dropped.
|
||||||
|
pub(crate) fn hold_answer_to_next_listing(
|
||||||
|
&self,
|
||||||
|
folder: &NodeUid,
|
||||||
|
unrevised: &NodeUid,
|
||||||
|
) -> Held {
|
||||||
|
let state = Arc::new(HoldState {
|
||||||
|
uid: folder.clone(),
|
||||||
|
reached: AtomicBool::new(false),
|
||||||
|
released: AtomicBool::new(false),
|
||||||
|
});
|
||||||
|
*self.0.hold_listing.lock() = Some((state.clone(), unrevised.clone()));
|
||||||
|
Held(state)
|
||||||
|
}
|
||||||
|
|
||||||
/// Wait here if `uids` is the read a [`Held`] in `slot` is for.
|
/// 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]) {
|
async fn wait_if_held(&self, slot: &Mutex<Option<Arc<HoldState>>>, uids: &[NodeUid]) {
|
||||||
let state = {
|
let state = {
|
||||||
@@ -1006,23 +1049,7 @@ impl DriveApi for FakeClient {
|
|||||||
if *left == 0 {
|
if *left == 0 {
|
||||||
*unrevised = None;
|
*unrevised = None;
|
||||||
}
|
}
|
||||||
for node in &mut nodes {
|
nodes.iter_mut().for_each(unrevise);
|
||||||
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);
|
self.cache(&nodes);
|
||||||
@@ -1031,14 +1058,28 @@ impl DriveApi for FakeClient {
|
|||||||
|
|
||||||
async fn enumerate_nodes_light(&self, uids: &[NodeUid]) -> Result<Vec<Node>> {
|
async fn enumerate_nodes_light(&self, uids: &[NodeUid]) -> Result<Vec<Node>> {
|
||||||
let lag = self.lag();
|
let lag = self.lag();
|
||||||
self.request(false, |server| {
|
let mut nodes: Vec<Node> = self
|
||||||
let now = Instant::now();
|
.request(false, |server| {
|
||||||
Ok(uids
|
let now = Instant::now();
|
||||||
|
Ok(uids
|
||||||
|
.iter()
|
||||||
|
.filter_map(|uid| Some(to_node(uid, &server.visible(uid, lag, now)?, true)))
|
||||||
|
.collect())
|
||||||
|
})
|
||||||
|
.await?;
|
||||||
|
let held = self.0.hold_listing.lock().take_if(|(state, _)| {
|
||||||
|
nodes
|
||||||
.iter()
|
.iter()
|
||||||
.filter_map(|uid| Some(to_node(uid, &server.visible(uid, lag, now)?, true)))
|
.any(|node| node.parent_uid.as_ref() == Some(&state.uid))
|
||||||
.collect())
|
});
|
||||||
})
|
if let Some((state, unrevised)) = held {
|
||||||
.await
|
nodes
|
||||||
|
.iter_mut()
|
||||||
|
.filter(|node| node.uid == unrevised)
|
||||||
|
.for_each(unrevise);
|
||||||
|
wait_until_released(&state).await;
|
||||||
|
}
|
||||||
|
Ok(nodes)
|
||||||
}
|
}
|
||||||
|
|
||||||
async fn enumerate_folder_children_node_uids(
|
async fn enumerate_folder_children_node_uids(
|
||||||
|
|||||||
+12
-4
@@ -12,13 +12,15 @@ Conventions:
|
|||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## B166 — A folder drops a file whose create lands while it is listed
|
## B166 — A folder drops or empties a file whose create lands while it is listed
|
||||||
|
|
||||||
**Status:** Fixed.
|
**Status:** Fixed.
|
||||||
**Found:** 2026-10-04, by the account run on the working tree after B165 (a wide directory, on
|
**Found:** 2026-10-04, by the account run on the working tree after B165 (a wide directory, on
|
||||||
My files and on on-demand pairing 1): "concurrent unlinks left the wrong entries". Every
|
My files and on on-demand pairing 1): "concurrent unlinks left the wrong entries". Every
|
||||||
create and all 64 trashes landed as they should, so the drain was right and the listing was
|
create and all 64 trashes landed as they should, so the drain was right and the listing was
|
||||||
wrong. The run logs no names. The simulation lost files that were never unlinked.
|
wrong. The run logs no names. The simulation lost files that were never unlinked. The same
|
||||||
|
run read `-starts-with-dash`, 17 bytes, back as 0 bytes on on-demand pairing 1, just as its
|
||||||
|
create landed, so B163's fix does not cover it.
|
||||||
|
|
||||||
**Where:** `list_children` in `crates/pdfs-fuse/src/lib.rs`.
|
**Where:** `list_children` in `crates/pdfs-fuse/src/lib.rs`.
|
||||||
|
|
||||||
@@ -27,7 +29,10 @@ the queue still holds for it (B132). The queue was read after Drive answered. A
|
|||||||
landed in between was in neither: Drive had taken the listing before the node existed, and
|
landed in between was in neither: Drive had taken the listing before the node existed, and
|
||||||
the queue had let go of it by the time it was read. The file was missing until the folder
|
the queue had let go of it by the time it was read. The file was missing until the folder
|
||||||
was listed again. On Wi-Fi, with 16 workers landing creates while the event feed drops the
|
was listed again. On Wi-Fi, with 16 workers landing creates while the event feed drops the
|
||||||
folder's listing, this happened in almost every run of the wide-directory case.
|
folder's listing, this happened in almost every run of the wide-directory case. When
|
||||||
|
Drive's listing already had the node, but from before its create was committed, the node had
|
||||||
|
no revision, and the file was listed at 0 bytes until its size was read again (the B163 state,
|
||||||
|
reached through a listing instead of the read-back).
|
||||||
|
|
||||||
**Fix.** The queued children are read before Drive is asked as well as after. A node queued
|
**Fix.** The queued children are read before Drive is asked as well as after. A node queued
|
||||||
before and gone from the queue after has landed meanwhile, and is listed as the DB has it
|
before and gone from the queue after has landed meanwhile, and is listed as the DB has it
|
||||||
@@ -35,7 +40,10 @@ now, under its name. One unlinked or moved away meanwhile is no longer a child o
|
|||||||
in the DB, so it stays out.
|
in the DB, so it stays out.
|
||||||
|
|
||||||
**Test:** `half_of_a_wide_folder_unlinked_at_once_leaves_the_other_half` in `sim/daemon.rs`
|
**Test:** `half_of_a_wide_folder_unlinked_at_once_leaves_the_other_half` in `sim/daemon.rs`
|
||||||
(failed 4 of 4 runs before the fix, passed 8 of 8 after).
|
(failed 4 of 4 runs before the fix, passed 8 of 8 after), and
|
||||||
|
`a_listing_taken_before_a_create_commits_keeps_the_landed_file`, which holds Drive's answer to
|
||||||
|
the listing until the create has landed (the daemon listed the file at 0 bytes before the
|
||||||
|
fix).
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
|||||||
+3
-2
@@ -59,8 +59,9 @@ by its local id, in the queue, the stored tree and pins alike.
|
|||||||
upload landed showed it both as Drive had it and as the queue did (B164).
|
upload landed showed it both as Drive had it and as the queue did (B164).
|
||||||
- **Stopping the daemon no longer hangs until systemd kills it.** A stop waits up to 10 s for
|
- **Stopping the daemon no longer hangs until systemd kills it.** A stop waits up to 10 s for
|
||||||
work in progress, then exits and logs what was still running (B165).
|
work in progress, then exits and logs what was still running (B165).
|
||||||
- **A folder no longer drops a new file while others are uploaded.** A file whose upload
|
- **A folder no longer drops or empties a new file while others are uploaded.** A file whose
|
||||||
landed while the folder was being listed was missing from that listing (B166).
|
upload landed while the folder was being listed was missing from that listing, or listed as
|
||||||
|
empty (B166).
|
||||||
- **A file renamed just as its upload finished keeps its new name.** The mount no longer showed
|
- **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).
|
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
|
- **A change that waits for a name to be freed no longer waits seconds after it is.** A rename or
|
||||||
|
|||||||
@@ -103,8 +103,8 @@ Tracks `docs/MILESTONE-3.0.0.md`. Update it with every commit that moves a phase
|
|||||||
while the daemon was restarting: the stop waited on a busy thread until systemd killed
|
while the daemon was restarting: the stop waited on a busy thread until systemd killed
|
||||||
it (B165, bounded). The next run on the working tree (2026-10-04: 259 passed, 4
|
it (B165, bounded). The next run on the working tree (2026-10-04: 259 passed, 4
|
||||||
failed) found a wide folder dropping files whose create landed while it was listed
|
failed) found a wide folder dropping files whose create landed while it was listed
|
||||||
(B166, fixed). Still open from it: a new file that read back empty again, and a
|
(B166, fixed), or listing one empty: the new file that read back empty again
|
||||||
cleanup that found no control socket.
|
(B166 too). Still open from it: a cleanup that found no control socket.
|
||||||
5. **Release**: 2.10.0 is tagged; the 2.9.0 tag still waits for the user's go-ahead.
|
5. **Release**: 2.10.0 is tagged; the 2.9.0 tag still waits for the user's go-ahead.
|
||||||
|
|
||||||
## After 3.0.0
|
## After 3.0.0
|
||||||
|
|||||||
Reference in new issue
Block a user