mirror of
https://github.com/narrrl/proton-drive-linux.git
synced 2026-11-03 16:41:26 +01:00
fix(fuse): never adopt a node we trashed as an unanswered create's twin (B162)
A folder removed and made again met the old folder while its trash was on the wire and adopted it, so the new folder's contents went into the old one and were trashed with it. The twin search now skips hidden nodes, as listings do. The fake Drive can hold a trash (hold_next_trash).
This commit is contained in:
1 parent
cd18029944
commit
18587124ac
6 files changed
+103
-2
No files matched your search
@@ -1337,9 +1337,14 @@ impl Core {
|
||||
}
|
||||
let uids = self.block_on_bounded(self.drive.enumerate_folder_children_node_uids(parent))?;
|
||||
let children = self.block_on_bounded(self.drive.enumerate_nodes_light(&uids))?;
|
||||
// A node this mount trashed is not one an unanswered create made, even
|
||||
// while its trash is on the wire or a listing still shows it. Taking a
|
||||
// folder removed and made again put the new one's contents into the
|
||||
// old one, and its trash took them along (docs/BUGS.md B162).
|
||||
let hidden = self.hidden.lock().clone();
|
||||
let Some(twin) = children
|
||||
.into_iter()
|
||||
.find(|node| node.name == name && !node.trashed)
|
||||
.find(|node| node.name == name && !node.trashed && !hidden.contains(&node.uid))
|
||||
else {
|
||||
return Ok(None);
|
||||
};
|
||||
|
||||
@@ -494,6 +494,56 @@ 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_made_again_while_the_old_one_is_trashed_is_a_new_folder() {
|
||||
// A folder removed and made again at once: the new one's create met
|
||||
// the old folder, its trash still on the wire, and took it for the
|
||||
// node an unanswered create of its own had made. What went into the
|
||||
// new folder went into the old one, and the trash took it all along
|
||||
// (B162).
|
||||
let drive = FakeDrive::new();
|
||||
let dir = scratch("remade-while-trashed");
|
||||
let daemon = Daemon::start(&dir, drive.client(1, Faults::lan())).unwrap();
|
||||
let mnt = daemon.mountpoint.clone();
|
||||
std::fs::create_dir(mnt.join("z")).unwrap();
|
||||
assert!(wait_until(Duration::from_secs(30), || {
|
||||
daemon.pending().is_ok_and(|items| items.is_empty()) && drive.lookup("z").is_some()
|
||||
}));
|
||||
let old = drive.lookup("z").unwrap();
|
||||
|
||||
let trash = daemon.client.hold_next_trash(&old);
|
||||
std::fs::remove_dir(mnt.join("z")).unwrap();
|
||||
assert!(wait_until(Duration::from_secs(30), || trash.reached()));
|
||||
std::fs::create_dir(mnt.join("z")).unwrap();
|
||||
std::fs::write(mnt.join("z/f"), b"abc").unwrap();
|
||||
// Sent, and either waiting for the name or landed.
|
||||
assert!(wait_until(Duration::from_secs(30), || {
|
||||
daemon.pending().is_ok_and(|items| {
|
||||
!items
|
||||
.iter()
|
||||
.any(|op| op.kind == "mkdir" && op.attempts == 0)
|
||||
})
|
||||
}));
|
||||
drop(trash);
|
||||
|
||||
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("z/f").cloned() == file
|
||||
}),
|
||||
"{:?} {:?}",
|
||||
daemon.pending(),
|
||||
drive.tree()
|
||||
);
|
||||
assert_ne!(drive.lookup("z"), Some(old));
|
||||
assert_eq!(std::fs::read(mnt.join("z/f")).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 the_echo_of_our_own_rename_keeps_the_cached_bytes() {
|
||||
|
||||
@@ -332,6 +332,7 @@ impl FakeDrive {
|
||||
requests: AtomicU64::new(0),
|
||||
hold: Mutex::new(None),
|
||||
hold_answer: Mutex::new(None),
|
||||
hold_trash: Mutex::new(None),
|
||||
drop_after_create: AtomicBool::new(false),
|
||||
lose_create_reply: AtomicBool::new(false),
|
||||
hold_create_reply: Mutex::new(None),
|
||||
@@ -587,6 +588,8 @@ struct ClientInner {
|
||||
hold: Mutex<Option<Arc<HoldState>>>,
|
||||
/// The read [`FakeClient::hold_answer_to_next_read`] is waiting for.
|
||||
hold_answer: Mutex<Option<Arc<HoldState>>>,
|
||||
/// The trash [`FakeClient::hold_next_trash`] is waiting for.
|
||||
hold_trash: Mutex<Option<Arc<HoldState>>>,
|
||||
/// Set by [`FakeClient::drop_link_after_next_create`].
|
||||
drop_after_create: AtomicBool,
|
||||
/// Set by [`FakeClient::lose_reply_to_next_create`].
|
||||
@@ -684,6 +687,18 @@ impl FakeClient {
|
||||
Held(state)
|
||||
}
|
||||
|
||||
/// Hold the next trash of `uid` alone, before it reaches Drive, until the
|
||||
/// answer is dropped.
|
||||
pub(crate) fn hold_next_trash(&self, uid: &NodeUid) -> Held {
|
||||
let state = Arc::new(HoldState {
|
||||
uid: uid.clone(),
|
||||
reached: AtomicBool::new(false),
|
||||
released: AtomicBool::new(false),
|
||||
});
|
||||
*self.0.hold_trash.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 {
|
||||
@@ -1313,6 +1328,7 @@ impl DriveApi for FakeClient {
|
||||
}
|
||||
|
||||
async fn trash_nodes(&self, uids: &[NodeUid]) -> Result<NodeOutcomes> {
|
||||
self.wait_if_held(&self.0.hold_trash, uids).await;
|
||||
self.batch(uids, trash_one).await
|
||||
}
|
||||
|
||||
|
||||
@@ -12,6 +12,32 @@ Conventions:
|
||||
|
||||
---
|
||||
|
||||
## B162 — A folder removed and made again at once loses what goes into it
|
||||
|
||||
**Status:** Fixed.
|
||||
**Found:** 2026-10-04, by the simulation's slow-link profile (`one_client_on_a_slow_link`,
|
||||
seed 4). The client removed folder `z` and made `z` again, with `z/x` and `z/x/y` inside. All
|
||||
three stayed in the mount but never reached Drive. The log showed the new `z` "adopting" the old
|
||||
one.
|
||||
|
||||
**Where:** `Core::adoptable_twin` in `crates/pdfs-fuse/src/drain.rs`.
|
||||
|
||||
**Cause.** A create that finds its name taken looks for a node an earlier, unanswered attempt
|
||||
of its own made (B127), and adopts it. The old folder's trash was still on the wire, so Drive
|
||||
refused the new folder's name, and the listing showed the old folder, not in the tree any more
|
||||
and made a moment ago. The new folder took it for its own twin. What went into the new folder
|
||||
was created in the old one, and the trash then took it all along.
|
||||
|
||||
**Fix.** The twin search skips nodes this mount trashed (its `hidden` set), as listings already
|
||||
do. The create then waits for the trash to free the name (`name_is_held`), or takes the name
|
||||
at once if the trash has landed (B144).
|
||||
|
||||
**Test:** simulation test `a_folder_made_again_while_the_old_one_is_trashed_is_a_new_folder`
|
||||
(failed before the fix: Drive ended up empty). The fake Drive can now hold a trash
|
||||
(`FakeClient::hold_next_trash`).
|
||||
|
||||
---
|
||||
|
||||
## B161 — A file moved while it was being created shows up under its old name
|
||||
|
||||
**Status:** Fixed.
|
||||
|
||||
@@ -51,6 +51,8 @@ by its local id, in the queue, the stored tree and pins alike.
|
||||
- **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 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 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 B161.
|
||||
- Fixes from the simulation and account runs since then: B127 and B141 to B162.
|
||||
- **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`.
|
||||
|
||||
@@ -87,6 +87,8 @@ Tracks `docs/MILESTONE-3.0.0.md`. Update it with every commit that moves a phase
|
||||
fixed). Slow-link seed 4 also had folders reach Drive 30 s late, after the
|
||||
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).
|
||||
Seed 4 failed on a folder removed and made again, whose create adopted the removed
|
||||
folder (B162, 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