mirror of
https://github.com/narrrl/proton-drive-linux.git
synced 2026-11-04 08:56:59 +01:00
fix(fuse): a file replaced just as its create lands is trashed on Drive (B175)
This commit is contained in:
1 parent
1612a9d07a
commit
ce60e006ac
6 files changed
+97
-9
No files matched your search
@@ -1946,7 +1946,7 @@ impl ProtonFs {
|
|||||||
reply.error(error);
|
reply.error(error);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
if let Err(e) = self.core.remove_replaced(victim_uid, newname) {
|
if let Err(e) = self.core.remove_replaced(*victim_ino, victim_uid, newname) {
|
||||||
reply.error(e);
|
reply.error(e);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -5007,15 +5007,18 @@ impl Core {
|
|||||||
///
|
///
|
||||||
/// A node whose own creation is still queued has never reached the server, so
|
/// A node whose own creation is still queued has never reached the server, so
|
||||||
/// dropping its queued ops is the whole removal; nothing goes to the wire and
|
/// dropping its queued ops is the whole removal; nothing goes to the wire and
|
||||||
/// it works offline.
|
/// it works offline. A create that landed before its ops were dropped left
|
||||||
|
/// none: Drive has the node, so it is trashed there like any other
|
||||||
|
/// (`docs/BUGS.md` B175).
|
||||||
///
|
///
|
||||||
/// A handle still open on the replaced node keeps reading its bytes, as
|
/// A handle still open on the replaced node keeps reading its bytes, as
|
||||||
/// `rename(2)` promises: the inode stays as an unlinked entry and its
|
/// `rename(2)` promises: the inode stays as an unlinked entry and its
|
||||||
/// staged bytes, cache and reader go at the final close, exactly as for an
|
/// staged bytes, cache and reader go at the final close, exactly as for an
|
||||||
/// `unlink` under an open handle. Its queued ops go now, or the drain would
|
/// `unlink` under an open handle. Its queued ops go now, or the drain would
|
||||||
/// upload to the node after it was trashed (`docs/BUGS.md` B106).
|
/// upload to the node after it was trashed (`docs/BUGS.md` B106).
|
||||||
fn remove_replaced(&self, uid: &NodeUid, name: &str) -> Result<(), Errno> {
|
fn remove_replaced(&self, ino: u64, uid: &NodeUid, name: &str) -> Result<(), Errno> {
|
||||||
if is_local_uid(uid) {
|
let landed;
|
||||||
|
let uid = if is_local_uid(uid) {
|
||||||
if self.is_open_anywhere(uid) {
|
if self.is_open_anywhere(uid) {
|
||||||
self.withdraw_queued_ops(uid)?;
|
self.withdraw_queued_ops(uid)?;
|
||||||
} else {
|
} else {
|
||||||
@@ -5024,9 +5027,20 @@ impl Core {
|
|||||||
self.for_each_state(|st| {
|
self.for_each_state(|st| {
|
||||||
st.forget_or_unlink(uid);
|
st.forget_or_unlink(uid);
|
||||||
});
|
});
|
||||||
debug!(%uid, name, "replaced a node whose create was still queued");
|
match self.landed_row_uid(ino) {
|
||||||
return Ok(());
|
Some(real) => {
|
||||||
}
|
debug!(local = %uid, %real, name, "create landed under a replace");
|
||||||
|
landed = real;
|
||||||
|
&landed
|
||||||
|
}
|
||||||
|
None => {
|
||||||
|
debug!(%uid, name, "replaced a node whose create was still queued");
|
||||||
|
return Ok(());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
} else {
|
||||||
|
uid
|
||||||
|
};
|
||||||
// Local-first or offline, or the network goes away under the call:
|
// Local-first or offline, or the network goes away under the call:
|
||||||
// queue the trash. The rename that follows queues as well, behind it.
|
// queue the trash. The rename that follows queues as well, behind it.
|
||||||
if !self.sends_inline() {
|
if !self.sends_inline() {
|
||||||
|
|||||||
@@ -560,6 +560,55 @@ 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_file_replaced_as_its_create_lands_is_trashed_on_drive() {
|
||||||
|
// The same race as B173, through a rename onto the file: the replace
|
||||||
|
// found no op left to drop and took the file for one Drive had never
|
||||||
|
// seen. Drive kept it under the name, and the next rename of that name
|
||||||
|
// moved it instead of the file that replaced it (B175).
|
||||||
|
let drive = FakeDrive::new();
|
||||||
|
let dir = scratch("replaced-as-create-lands");
|
||||||
|
let daemon = Daemon::start(&dir, drive.client(1, Faults::lan())).unwrap();
|
||||||
|
let mnt = daemon.mountpoint.clone();
|
||||||
|
std::fs::write(mnt.join("b.txt"), b"new").unwrap();
|
||||||
|
assert!(
|
||||||
|
wait_until(Duration::from_secs(30), || {
|
||||||
|
daemon.pending().is_ok_and(|items| items.is_empty())
|
||||||
|
&& drive.lookup("b.txt").is_some()
|
||||||
|
}),
|
||||||
|
"{:?} {:?}",
|
||||||
|
daemon.pending(),
|
||||||
|
drive.tree()
|
||||||
|
);
|
||||||
|
|
||||||
|
let create = daemon.client.hold_reply_to_next_create();
|
||||||
|
std::fs::write(mnt.join("a.txt"), b"old").unwrap();
|
||||||
|
assert!(wait_until(Duration::from_secs(30), || create.reached()));
|
||||||
|
let uid = drive.lookup("a.txt").unwrap();
|
||||||
|
let landing = daemon.client.hold_next_landing(&uid);
|
||||||
|
drop(create);
|
||||||
|
assert!(wait_until(Duration::from_secs(30), || landing.reached()));
|
||||||
|
std::fs::rename(mnt.join("b.txt"), mnt.join("a.txt")).unwrap();
|
||||||
|
drop(landing);
|
||||||
|
std::fs::rename(mnt.join("a.txt"), mnt.join("d")).unwrap();
|
||||||
|
assert!(
|
||||||
|
wait_until(Duration::from_secs(30), || {
|
||||||
|
daemon.pending().is_ok_and(|items| items.is_empty())
|
||||||
|
&& drive.lookup("a.txt").is_none()
|
||||||
|
&& drive.tree().get("d") == Some(&Entry::File(Arc::new(b"new".to_vec())))
|
||||||
|
}),
|
||||||
|
"{:?} {:?}",
|
||||||
|
daemon.pending(),
|
||||||
|
drive.tree()
|
||||||
|
);
|
||||||
|
assert_eq!(std::fs::read(mnt.join("d")).unwrap(), b"new");
|
||||||
|
assert!(!mnt.join("a.txt").exists());
|
||||||
|
|
||||||
|
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_file_moved_while_its_create_was_on_the_wire_keeps_its_new_name() {
|
fn a_file_moved_while_its_create_was_on_the_wire_keeps_its_new_name() {
|
||||||
|
|||||||
@@ -12,6 +12,28 @@ Conventions:
|
|||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
## B175 — A file replaced by a rename just as its create lands stays on Drive
|
||||||
|
|
||||||
|
**Status:** Fixed (unverified).
|
||||||
|
**Found:** 2026-10-04, by `one_client_on_a_good_link` (seed 8) on 378667f, run three at a time
|
||||||
|
in a loop. `rename b.txt -> a.txt` replaced an `a.txt` whose create had landed a few
|
||||||
|
milliseconds before, and the next `rename a.txt -> d` moved the old `a.txt` on Drive. Drive
|
||||||
|
ended with the old bytes in `d` and the new ones still in `a.txt`, and the queue did not drain.
|
||||||
|
|
||||||
|
**Where:** `Core::remove_replaced` in `crates/pdfs-fuse/src/lib.rs`.
|
||||||
|
|
||||||
|
**Cause.** The same gap as B173, through `rename`. The replace found the old file under its
|
||||||
|
stand-in, found no op left to drop, and took it for a file Drive had never seen. So nothing
|
||||||
|
trashed it on Drive, and the tree took it back under its name once it got its uid.
|
||||||
|
|
||||||
|
**Fix.** As in B173: after dropping a stand-in's ops, the replace checks whether the node's row
|
||||||
|
has a real uid. If it has, the file is trashed on Drive like any other replaced file.
|
||||||
|
|
||||||
|
**Test:** `a_file_replaced_as_its_create_lands_is_trashed_on_drive` in
|
||||||
|
`crates/pdfs-fuse/src/sim/daemon.rs`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
## B174 — A file renamed just as its create lands is refused with "Permission denied"
|
## B174 — A file renamed just as its create lands is refused with "Permission denied"
|
||||||
|
|
||||||
**Status:** Fixed (unverified).
|
**Status:** Fixed (unverified).
|
||||||
|
|||||||
@@ -76,6 +76,8 @@ by its local id, in the queue, the stored tree and pins alike.
|
|||||||
there, and the next file with that name no longer became a conflict copy (B173).
|
there, and the next file with that name no longer became a conflict copy (B173).
|
||||||
- **Renaming a file just as it reaches Drive no longer fails with "Permission denied"**
|
- **Renaming a file just as it reaches Drive no longer fails with "Permission denied"**
|
||||||
(B174).
|
(B174).
|
||||||
|
- **A file replaced by a rename just as it reached Drive is removed from Drive too.** It no
|
||||||
|
longer stayed there, and the next rename of its name no longer moved it instead (B175).
|
||||||
- **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,
|
||||||
B172).
|
B172).
|
||||||
|
|||||||
@@ -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 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
|
- **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).
|
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 B174.
|
- Fixes from the simulation and account runs since then: B127 and B141 to B175.
|
||||||
- **Sync issues, first cut** (§6.2): refusals sorted into issues, shown at once in the app,
|
- **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`.
|
tray and CLI; Export button and `pdfs sync export`; `pdfs sync issues`.
|
||||||
|
|
||||||
@@ -118,7 +118,8 @@ Tracks `docs/MILESTONE-3.0.0.md`. Update it with every commit that moves a phase
|
|||||||
while its upload was read back showed its old name again (B172, fixed). The loop
|
while its upload was read back showed its old name again (B172, fixed). The loop
|
||||||
on d850265 failed seed 12 once in nine runs: a file deleted just as its create
|
on d850265 failed seed 12 once in nine runs: a file deleted just as its create
|
||||||
landed stayed on Drive (B173, fixed). The suite on 378667f failed seed 6: a rename
|
landed stayed on Drive (B173, fixed). The suite on 378667f failed seed 6: a rename
|
||||||
in the same gap was refused with `EACCES` (B174, fixed).
|
in the same gap was refused with `EACCES` (B174, fixed). The loop on 378667f failed
|
||||||
|
seed 8: a file replaced by a rename in that gap stayed on Drive (B175, fixed).
|
||||||
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