mirror of
https://github.com/narrrl/proton-drive-linux.git
synced 2026-11-03 16:41:26 +01:00
fix(fuse): keep a move that lands while its create is read back (B161)
adopt_real_uid asked whether a move was queued only after reading the node back. A move that landed in between left no op, and the answer still had the old name, so the mount showed the file under it. Ask first. The fake Drive can hold a read's answer (hold_answer_to_next_read) so the test can land the move while the answer is on its way.
This commit is contained in:
1 parent
d6dd41407f
commit
cd18029944
6 files changed
+155
-12
No files matched your search
@@ -1587,15 +1587,17 @@ impl Core {
|
||||
real: &NodeUid,
|
||||
adopt: impl FnOnce(&Node),
|
||||
) -> Result<(), Box<dyn std::error::Error>> {
|
||||
// A rename made while the create was on the wire is queued against the
|
||||
// real uid (`Db::finish_create`). Until it lands, the tree keeps the
|
||||
// name and folder the user gave it. Asked before the read-back: a
|
||||
// rename that lands while Drive answers leaves no op behind, and the
|
||||
// answer may still have the old name (`docs/BUGS.md` B161).
|
||||
let real_key = real.to_string();
|
||||
let renaming = self.db.has_pending_op(&real_key, OP_RENAME)?;
|
||||
// Drive answers a create before it lists the new node.
|
||||
let mut node = self
|
||||
.read_back(real)
|
||||
.map_err(|e| self.errno_error(e, "fetch node"))?;
|
||||
// A rename made while the create was on the wire is queued against the
|
||||
// real uid (`Db::finish_create`). Until it lands, the tree keeps the
|
||||
// name and folder the user gave it.
|
||||
let real_key = real.to_string();
|
||||
let renaming = self.db.has_pending_op(&real_key, OP_RENAME)?;
|
||||
// Before open handles are rebased onto the node, so one released in
|
||||
// between finds what `adopt` keeps (`docs/BUGS.md` B123).
|
||||
adopt(&node);
|
||||
|
||||
@@ -422,6 +422,78 @@ 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_moved_while_its_create_was_on_the_wire_keeps_its_new_name() {
|
||||
// The move queued behind the create landed while the drain read the
|
||||
// new node back. The answer still had the old name, and the drain no
|
||||
// longer found the move queued, so the mount showed the old name in
|
||||
// the new folder (B161).
|
||||
let drive = FakeDrive::new();
|
||||
let dir = scratch("moved-while-created");
|
||||
let daemon = Daemon::start(&dir, drive.client(1, Faults::lan())).unwrap();
|
||||
let mnt = daemon.mountpoint.clone();
|
||||
std::fs::create_dir(mnt.join("x")).unwrap();
|
||||
std::fs::create_dir(mnt.join("y")).unwrap();
|
||||
assert!(
|
||||
wait_until(Duration::from_secs(30), || {
|
||||
daemon.pending().is_ok_and(|items| items.is_empty()) && drive.lookup("y").is_some()
|
||||
}),
|
||||
"{:?} {:?}",
|
||||
daemon.pending(),
|
||||
drive.tree()
|
||||
);
|
||||
|
||||
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()));
|
||||
std::fs::rename(mnt.join("x/c.bin"), mnt.join("y/d")).unwrap();
|
||||
let uid = drive.lookup("x/c.bin").unwrap();
|
||||
// Each read-back is answered in turn until the create is retired; the
|
||||
// answer to the next one is held until the move has landed. A new
|
||||
// folder wakes the drain for the move.
|
||||
let mut held = create;
|
||||
let adopting = loop {
|
||||
let next = daemon.client.hold_answer_to_next_read(&uid);
|
||||
drop(held);
|
||||
assert!(wait_until(Duration::from_secs(30), || next.reached()));
|
||||
if daemon
|
||||
.pending()
|
||||
.is_ok_and(|items| items.iter().all(|op| op.kind != "create"))
|
||||
{
|
||||
break next;
|
||||
}
|
||||
held = next;
|
||||
};
|
||||
std::fs::create_dir(mnt.join("z")).unwrap();
|
||||
assert!(
|
||||
wait_until(Duration::from_secs(10), || drive.lookup("y/d").is_some()),
|
||||
"{:?} {:?}",
|
||||
daemon.pending(),
|
||||
drive.tree()
|
||||
);
|
||||
drop(adopting);
|
||||
|
||||
let file = Some(Entry::File(Arc::new(b"abc".to_vec())));
|
||||
assert!(
|
||||
wait_until(Duration::from_secs(30), || {
|
||||
daemon.pending().is_ok_and(|items| items.is_empty())
|
||||
&& drive.tree().get("y/d").cloned() == file
|
||||
}),
|
||||
"{:?} {:?}",
|
||||
daemon.pending(),
|
||||
drive.tree()
|
||||
);
|
||||
let names: Vec<_> = std::fs::read_dir(mnt.join("y"))
|
||||
.unwrap()
|
||||
.map(|e| e.unwrap().file_name().into_string().unwrap())
|
||||
.collect();
|
||||
assert_eq!(names, ["d"]);
|
||||
|
||||
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 the_echo_of_our_own_rename_keeps_the_cached_bytes() {
|
||||
|
||||
@@ -33,7 +33,8 @@
|
||||
//!
|
||||
//! 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.
|
||||
//! 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.
|
||||
|
||||
use std::collections::{BTreeMap, HashMap};
|
||||
use std::io::{Read, Write};
|
||||
@@ -330,6 +331,7 @@ impl FakeDrive {
|
||||
online: AtomicBool::new(true),
|
||||
requests: AtomicU64::new(0),
|
||||
hold: Mutex::new(None),
|
||||
hold_answer: Mutex::new(None),
|
||||
drop_after_create: AtomicBool::new(false),
|
||||
lose_create_reply: AtomicBool::new(false),
|
||||
hold_create_reply: Mutex::new(None),
|
||||
@@ -583,6 +585,8 @@ struct ClientInner {
|
||||
requests: AtomicU64,
|
||||
/// The read [`FakeClient::hold_next_read`] is waiting for.
|
||||
hold: Mutex<Option<Arc<HoldState>>>,
|
||||
/// The read [`FakeClient::hold_answer_to_next_read`] is waiting for.
|
||||
hold_answer: Mutex<Option<Arc<HoldState>>>,
|
||||
/// Set by [`FakeClient::drop_link_after_next_create`].
|
||||
drop_after_create: AtomicBool,
|
||||
/// Set by [`FakeClient::lose_reply_to_next_create`].
|
||||
@@ -667,6 +671,19 @@ impl FakeClient {
|
||||
Held(state)
|
||||
}
|
||||
|
||||
/// Answer the next read of `uid` alone with what Drive holds now, then
|
||||
/// hold the answer until it is dropped, as a slow link does. Only one
|
||||
/// answer is held at a time.
|
||||
pub(crate) fn hold_answer_to_next_read(&self, uid: &NodeUid) -> Held {
|
||||
let state = Arc::new(HoldState {
|
||||
uid: uid.clone(),
|
||||
reached: AtomicBool::new(false),
|
||||
released: AtomicBool::new(false),
|
||||
});
|
||||
*self.0.hold_answer.lock() = Some(state.clone());
|
||||
Held(state)
|
||||
}
|
||||
|
||||
/// Make the next file create on Drive, then hold its answer until the
|
||||
/// answer is dropped, as a slow link does.
|
||||
pub(crate) fn hold_reply_to_next_create(&self) -> Held {
|
||||
@@ -691,10 +708,10 @@ impl FakeClient {
|
||||
self.0.lose_revision_read_back.store(true, Ordering::SeqCst);
|
||||
}
|
||||
|
||||
/// Wait here if `uids` is the read a [`Held`] is for.
|
||||
async fn wait_if_held(&self, uids: &[NodeUid]) {
|
||||
/// 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 = {
|
||||
let mut hold = self.0.hold.lock();
|
||||
let mut hold = slot.lock();
|
||||
match hold.as_ref() {
|
||||
Some(state) if uids == std::slice::from_ref(&state.uid) => hold.take(),
|
||||
_ => None,
|
||||
@@ -928,7 +945,7 @@ impl DriveApi for FakeClient {
|
||||
}
|
||||
|
||||
async fn enumerate_nodes(&self, uids: &[NodeUid]) -> Result<Vec<Node>> {
|
||||
self.wait_if_held(uids).await;
|
||||
self.wait_if_held(&self.0.hold, uids).await;
|
||||
{
|
||||
let mut lose = self.0.lose_read.lock();
|
||||
if lose
|
||||
@@ -949,6 +966,7 @@ impl DriveApi for FakeClient {
|
||||
.collect())
|
||||
})
|
||||
.await?;
|
||||
self.wait_if_held(&self.0.hold_answer, uids).await;
|
||||
self.cache(&nodes);
|
||||
Ok(nodes)
|
||||
}
|
||||
@@ -1645,6 +1663,29 @@ mod tests {
|
||||
});
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_held_answer_is_what_drive_held_when_read() {
|
||||
rt().block_on(async {
|
||||
let drive = FakeDrive::new();
|
||||
let client = drive.client(1, Faults::none());
|
||||
let root = drive.root();
|
||||
let file = client.upload_file(&root, "a", "", b"1").await.unwrap();
|
||||
let held = client.hold_answer_to_next_read(&file);
|
||||
let read = tokio::spawn({
|
||||
let client = client.clone();
|
||||
let file = file.clone();
|
||||
async move { client.enumerate_nodes(std::slice::from_ref(&file)).await }
|
||||
});
|
||||
tokio::time::sleep(Duration::from_millis(50)).await;
|
||||
assert!(held.reached());
|
||||
assert!(!read.is_finished());
|
||||
drive.device().trash("a");
|
||||
drop(held);
|
||||
let nodes = read.await.unwrap().unwrap();
|
||||
assert!(!nodes[0].trashed);
|
||||
});
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_rename_lags_only_by_the_update_lag() {
|
||||
rt().block_on(async {
|
||||
|
||||
@@ -12,6 +12,31 @@ Conventions:
|
||||
|
||||
---
|
||||
|
||||
## B161 — A file moved while it was being created shows up under its old name
|
||||
|
||||
**Status:** Fixed.
|
||||
**Found:** 2026-10-04, by the simulation's slow-link profile (`one_client_on_a_slow_link`,
|
||||
seed 3). A file written and then moved at once ended up on Drive as `y/z/d`, but the mount
|
||||
showed it as `y/z/c.bin`, its name before the move, and `d` was missing.
|
||||
|
||||
**Where:** `Core::adopt_real_uid` in `crates/pdfs-fuse/src/drain.rs`.
|
||||
|
||||
**Cause.** A move made while a file's create is on the wire waits in the queue until the create
|
||||
lands. Once the create is retired, the drain reads the new node back from Drive and adopts it.
|
||||
The tree keeps the name the user gave only while the move is still queued. The drain asked
|
||||
about that after the read-back. On a slow link the move could land in between: the answer
|
||||
still had the old name, and the move was no longer queued, so the old name won.
|
||||
|
||||
**Fix.** The drain asks whether a move is queued before it reads the node back. It still asks
|
||||
again under the tree's lock (B152).
|
||||
|
||||
**Test:** simulation test `a_file_moved_while_its_create_was_on_the_wire_keeps_its_new_name`
|
||||
(failed before the fix: the new folder listed nothing). The fake Drive can now hold a read's
|
||||
answer, taken when the read arrives, until the test releases it
|
||||
(`FakeClient::hold_answer_to_next_read`).
|
||||
|
||||
---
|
||||
|
||||
## B160 — A new folder reaches Drive only half a minute later
|
||||
|
||||
**Status:** Fixed.
|
||||
|
||||
@@ -49,6 +49,8 @@ by its local id, in the queue, the stored tree and pins alike.
|
||||
- **Opening a file right after the daemon restarts no longer fails with "Resource temporarily
|
||||
unavailable"** (B159).
|
||||
- **A new folder reaches Drive at once**, not up to half a minute later (B160).
|
||||
- **A file moved while it was still being uploaded keeps its new name in the mount** on a slow
|
||||
link (B161).
|
||||
- **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 B160.
|
||||
- Fixes from the simulation and account runs since then: B127 and B141 to B161.
|
||||
- **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`.
|
||||
|
||||
@@ -85,7 +85,8 @@ Tracks `docs/MILESTONE-3.0.0.md`. Update it with every commit that moves a phase
|
||||
landing out of order (B158, fixed). Flaky-link seed 4 failed one of ten runs on a
|
||||
lookup that answered `EAGAIN` while the root was listed again after a restart (B159,
|
||||
fixed). Slow-link seed 4 also had folders reach Drive 30 s late, after the
|
||||
drain's idle poll (B160, fixed).
|
||||
drain's idle poll (B160, fixed). Slow-link seed 3 failed on a file moved while its
|
||||
create was on the wire, which the mount showed under its old name (B161, fixed).
|
||||
- [ ] Known-bug list in `sim/run.rs` is empty: B125 and B130 open; B127, B129 and B132 fixed
|
||||
but unconfirmed.
|
||||
- [ ] A clean account run. The one on a6f36d5 (2026-10-04, LAN: 427 passed, 12 failed)
|
||||
|
||||
Reference in new issue
Block a user