mirror of
https://github.com/narrrl/proton-drive-linux.git
synced 2026-11-03 16:41:26 +01:00
fixed fs bugs + compleance test for fuse fs
This commit is contained in:
1 parent
d0b49d3c8c
commit
a76f36bdff
8 files changed
+459
-60
No files matched your search
@@ -5,7 +5,13 @@ use super::*;
|
||||
impl Filesystem for ProtonFs {
|
||||
fn lookup(&self, _req: &Request, parent: INodeNo, name: &OsStr, reply: ReplyEntry) {
|
||||
let parent = parent.0;
|
||||
let name = name.to_string_lossy().into_owned();
|
||||
let name = match fuse_name(name) {
|
||||
Ok(name) => name,
|
||||
Err(error) => {
|
||||
reply.error(error);
|
||||
return;
|
||||
}
|
||||
};
|
||||
// A folder that has not been listed yet is enumerated from the remote,
|
||||
// so serve it from a worker rather than stalling the dispatch loop. A
|
||||
// listed folder — the common case — is a map hit, and answering it
|
||||
@@ -307,7 +313,13 @@ impl Filesystem for ProtonFs {
|
||||
reply.error(e);
|
||||
return;
|
||||
}
|
||||
let name = name.to_string_lossy().into_owned();
|
||||
let name = match fuse_name(name) {
|
||||
Ok(name) => name,
|
||||
Err(error) => {
|
||||
reply.error(error);
|
||||
return;
|
||||
}
|
||||
};
|
||||
let parent_uid = {
|
||||
let st = self.core.state.lock();
|
||||
match st.entries.get(&parent) {
|
||||
@@ -761,7 +773,13 @@ impl Filesystem for ProtonFs {
|
||||
}
|
||||
|
||||
fn unlink(&self, _req: &Request, parent: INodeNo, name: &OsStr, reply: ReplyEmpty) {
|
||||
let name_str = name.to_string_lossy();
|
||||
let name_str = match fuse_name(name) {
|
||||
Ok(name) => name,
|
||||
Err(error) => {
|
||||
reply.error(error);
|
||||
return;
|
||||
}
|
||||
};
|
||||
if let Ok((ino, _)) = self.core.lookup_child(parent.0, &name_str) {
|
||||
let st = self.core.state.lock();
|
||||
if let Some(entry) = st.entries.get(&ino)
|
||||
@@ -775,7 +793,13 @@ impl Filesystem for ProtonFs {
|
||||
}
|
||||
|
||||
fn rmdir(&self, _req: &Request, parent: INodeNo, name: &OsStr, reply: ReplyEmpty) {
|
||||
let name_str = name.to_string_lossy();
|
||||
let name_str = match fuse_name(name) {
|
||||
Ok(name) => name,
|
||||
Err(error) => {
|
||||
reply.error(error);
|
||||
return;
|
||||
}
|
||||
};
|
||||
if let Ok((ino, _)) = self.core.lookup_child(parent.0, &name_str) {
|
||||
{
|
||||
let st = self.core.state.lock();
|
||||
@@ -817,7 +841,13 @@ impl Filesystem for ProtonFs {
|
||||
reply.error(e);
|
||||
return;
|
||||
}
|
||||
let name = name.to_string_lossy().into_owned();
|
||||
let name = match fuse_name(name) {
|
||||
Ok(name) => name,
|
||||
Err(error) => {
|
||||
reply.error(error);
|
||||
return;
|
||||
}
|
||||
};
|
||||
let parent_uid = {
|
||||
let st = self.core.state.lock();
|
||||
match st.entries.get(&parent) {
|
||||
@@ -899,8 +929,20 @@ impl Filesystem for ProtonFs {
|
||||
) {
|
||||
let parent = parent.0;
|
||||
let newparent = newparent.0;
|
||||
let name = name.to_string_lossy().into_owned();
|
||||
let newname = newname.to_string_lossy().into_owned();
|
||||
let name = match fuse_name(name) {
|
||||
Ok(name) => name,
|
||||
Err(error) => {
|
||||
reply.error(error);
|
||||
return;
|
||||
}
|
||||
};
|
||||
let newname = match fuse_name(newname) {
|
||||
Ok(name) => name,
|
||||
Err(error) => {
|
||||
reply.error(error);
|
||||
return;
|
||||
}
|
||||
};
|
||||
let (ino, uid) = match self.core.lookup_child(parent, &name) {
|
||||
Ok(x) => x,
|
||||
Err(e) => {
|
||||
@@ -1050,12 +1092,31 @@ impl Filesystem for ProtonFs {
|
||||
// take the synchronous path, so a genuine API refusal (permissions, a
|
||||
// name clash) surfaces to the caller instead of becoming a queued op
|
||||
// that can only ever conflict.
|
||||
if !self.core.online.load(Ordering::Relaxed) || is_local_uid(&new_parent_uid) {
|
||||
if rename_needs_queue(
|
||||
self.core.online.load(Ordering::Relaxed),
|
||||
is_local_uid(&new_parent_uid),
|
||||
newparent != parent,
|
||||
newname != name,
|
||||
) {
|
||||
match self
|
||||
.core
|
||||
.queue_rename(ino, &uid, newparent, &new_parent_uid, &newname)
|
||||
{
|
||||
Ok(()) => reply.ok(),
|
||||
Ok(()) => {
|
||||
// Send the rename response first. Kernel-side rename cache
|
||||
// updates happen while processing that response; notifying
|
||||
// before it lets those updates overwrite our invalidation
|
||||
// and retain an empty destination readdir page.
|
||||
reply.ok();
|
||||
if let Some(notifier) = self.core.notifier.get() {
|
||||
let _ = notifier.inval_entry(INodeNo(parent), OsStr::new(&name));
|
||||
let _ = notifier.inval_entry(INodeNo(newparent), OsStr::new(&newname));
|
||||
let _ = notifier.inval_inode(INodeNo(parent), 0, 0);
|
||||
if newparent != parent {
|
||||
let _ = notifier.inval_inode(INodeNo(newparent), 0, 0);
|
||||
}
|
||||
}
|
||||
}
|
||||
Err(e) => {
|
||||
self.core.restore_replaced(victim.as_ref(), &newname);
|
||||
reply.error(e);
|
||||
@@ -1063,54 +1124,58 @@ impl Filesystem for ProtonFs {
|
||||
}
|
||||
return;
|
||||
}
|
||||
// Move first if the parent changed, then rename if the name changed.
|
||||
// Rename first if both halves change. Moving first makes the encrypted
|
||||
// name requirements stale and repeatedly failed with InvalidRequirements.
|
||||
// A failure past this point has already trashed any node the rename was
|
||||
// replacing, so put it back rather than leaving the caller with neither
|
||||
// the source moved nor the destination intact.
|
||||
if newparent != parent
|
||||
&& let Err(e) = self
|
||||
if newname != name {
|
||||
if let Err(e) = self
|
||||
.core
|
||||
.rt
|
||||
.block_on(self.core.client.move_node(&uid, &new_parent_uid))
|
||||
{
|
||||
error!(%uid, error = %e, "move failed");
|
||||
self.core.restore_replaced(victim.as_ref(), &newname);
|
||||
reply.error(Errno::EIO);
|
||||
return;
|
||||
.block_on(self.core.client.rename_node(&uid, &newname, None))
|
||||
{
|
||||
error!(%uid, error = %e, "rename failed");
|
||||
self.core.restore_replaced(victim.as_ref(), &newname);
|
||||
reply.error(Errno::EIO);
|
||||
return;
|
||||
}
|
||||
}
|
||||
if newname != name {
|
||||
// A move changes the node's encrypted name requirements. Proton's
|
||||
// link-details read can remain briefly stale immediately after the
|
||||
// move, making the following rename fail with InvalidRequirements
|
||||
// ("out of date"). Re-read/retry that narrow eventual-consistency
|
||||
// error; other refusals are stable and must surface immediately.
|
||||
if newparent != parent {
|
||||
let mut attempts = 0u32;
|
||||
let renamed = loop {
|
||||
let moved = loop {
|
||||
match self
|
||||
.core
|
||||
.rt
|
||||
.block_on(self.core.client.rename_node(&uid, &newname, None))
|
||||
.block_on(self.core.client.move_node(&uid, &new_parent_uid))
|
||||
{
|
||||
Ok(()) => break Ok(()),
|
||||
Err(e)
|
||||
if newparent != parent
|
||||
if newname != name
|
||||
&& api_code(&e) == Some(ResponseCode::InvalidRequirements)
|
||||
&& attempts < 4 =>
|
||||
&& attempts < 20 =>
|
||||
{
|
||||
attempts += 1;
|
||||
std::thread::sleep(Duration::from_millis(100 * (1 << attempts)));
|
||||
// Requirement propagation routinely takes longer than
|
||||
// three seconds. Keep a bounded ten-second window; a
|
||||
// successful POSIX reply must mean both remote halves
|
||||
// landed, not merely that the second half was queued.
|
||||
std::thread::sleep(Duration::from_millis(500));
|
||||
}
|
||||
Err(e) => break Err(e),
|
||||
}
|
||||
};
|
||||
if let Err(e) = renamed {
|
||||
error!(%uid, attempts, error = %e, "rename after move failed");
|
||||
// The move half already landed. Keep our inode model honest
|
||||
// even though the combined POSIX operation has to return EIO.
|
||||
self.core
|
||||
.state
|
||||
.lock()
|
||||
.relocate(ino, parent, newparent, &new_parent_uid, &name);
|
||||
if let Err(e) = moved {
|
||||
error!(%uid, attempts, error = %e, "move after rename failed");
|
||||
// The rename half landed in the source directory.
|
||||
let mut state = self.core.state.lock();
|
||||
let old_parent_uid = state
|
||||
.entries
|
||||
.get(&parent)
|
||||
.map(|entry| entry.uid.clone())
|
||||
.unwrap_or_else(|| uid.clone());
|
||||
state.rename_in_place(ino, parent, &old_parent_uid, &newname);
|
||||
drop(state);
|
||||
self.core.restore_replaced(victim.as_ref(), &newname);
|
||||
reply.error(Errno::EIO);
|
||||
return;
|
||||
@@ -1385,6 +1450,7 @@ impl ProtonFs {
|
||||
}
|
||||
// Only remove the namespace entry after Drive accepted the mutation.
|
||||
// Otherwise an EIO response still made the path disappear locally.
|
||||
self.core.hidden.lock().insert(uid.clone());
|
||||
self.core.state.lock().forget_or_unlink(&uid);
|
||||
if !open_now {
|
||||
self.core.discard_queued_ops(&uid);
|
||||
|
||||
+107
-3
@@ -36,6 +36,7 @@ use std::collections::{HashMap, HashSet, VecDeque};
|
||||
use std::ffi::OsStr;
|
||||
use std::fs::File;
|
||||
use std::io::Write;
|
||||
use std::os::unix::ffi::OsStrExt;
|
||||
use std::os::unix::fs::FileExt;
|
||||
use std::os::unix::net::UnixListener;
|
||||
use std::path::{Component, Path, PathBuf};
|
||||
@@ -225,6 +226,9 @@ struct Core {
|
||||
/// staged blob *is* the file's content and the remote still holds the old
|
||||
/// revision; and the drain worker, which performs the uploads.
|
||||
pending: Arc<Mutex<HashMap<NodeUid, PendingRevision>>>,
|
||||
/// Nodes removed through this daemon which a briefly stale remote listing
|
||||
/// may still return. Uids are not reused; an explicit restore clears one.
|
||||
hidden: Arc<Mutex<HashSet<NodeUid>>>,
|
||||
/// Nudges the drain worker: set true and notify to have it re-examine the
|
||||
/// queue instead of waiting out its backoff.
|
||||
drain_wake: Arc<(Mutex<bool>, Condvar)>,
|
||||
@@ -654,6 +658,7 @@ impl Core {
|
||||
// Before the lock: a DB row carries the size the server last
|
||||
// sealed, which a queued write is ahead of (B11).
|
||||
self.stamp_pending_sizes(&mut nodes);
|
||||
let hidden = self.hidden.lock().clone();
|
||||
let mut st = self.state.lock();
|
||||
if st.children.contains_key(&ino) {
|
||||
return Ok(());
|
||||
@@ -661,7 +666,7 @@ impl Core {
|
||||
let mut child_inos = Vec::with_capacity(nodes.len());
|
||||
let mut needs_size = Vec::new();
|
||||
for node in nodes {
|
||||
if node.trashed || node.uid == folder_uid {
|
||||
if !node_visible(&node, &folder_uid, &hidden) {
|
||||
continue;
|
||||
}
|
||||
if matches!(
|
||||
@@ -712,6 +717,7 @@ impl Core {
|
||||
// Same as the DB path above: the remote's size for a file with a write
|
||||
// still queued is the pre-write one (B11).
|
||||
self.stamp_pending_sizes(&mut nodes);
|
||||
let hidden = self.hidden.lock().clone();
|
||||
|
||||
let mut st = self.state.lock();
|
||||
// Lost the race? Another thread already populated it.
|
||||
@@ -721,7 +727,7 @@ impl Core {
|
||||
let mut child_inos = Vec::with_capacity(nodes.len());
|
||||
let filtered_nodes: Vec<Node> = nodes
|
||||
.into_iter()
|
||||
.filter(|node| !node.trashed && node.uid != folder_uid)
|
||||
.filter(|node| node_visible(node, &folder_uid, &hidden))
|
||||
.collect();
|
||||
// Files whose real size the cheap enumeration could not read. Collected
|
||||
// before interning so the upgrade below has the uids without re-walking.
|
||||
@@ -1488,6 +1494,7 @@ impl Core {
|
||||
error!(%uid, error = %e, "queueing trash failed");
|
||||
Errno::EIO
|
||||
})?;
|
||||
self.hidden.lock().insert(uid.clone());
|
||||
self.state.lock().forget(uid);
|
||||
self.cache.evict(uid);
|
||||
self.evict_reader(uid);
|
||||
@@ -2334,6 +2341,12 @@ impl Core {
|
||||
self.rt
|
||||
.block_on(self.client.restore_nodes(&parsed))
|
||||
.map_err(|e| CoreError::from_api(&e, "restore"))?;
|
||||
{
|
||||
let mut hidden = self.hidden.lock();
|
||||
for uid in &parsed {
|
||||
hidden.remove(uid);
|
||||
}
|
||||
}
|
||||
{
|
||||
let mut st = self.state.lock();
|
||||
for parent in parents {
|
||||
@@ -2923,6 +2936,38 @@ fn copy_pending_for_truncate(pending: &PendingRevision, destination: &File) -> s
|
||||
destination.set_len(offset)
|
||||
}
|
||||
|
||||
/// Whether a rename must be represented as a durable desired end state.
|
||||
/// Offline operations and operations targeting a not-yet-uploaded directory
|
||||
/// cannot use the API immediately. Online remote-to-remote operations stay
|
||||
/// synchronous so a successful kernel reply means the remote namespace has
|
||||
/// already reached the same end state.
|
||||
fn rename_needs_queue(
|
||||
online: bool,
|
||||
destination_is_local: bool,
|
||||
_parent_changed: bool,
|
||||
_name_changed: bool,
|
||||
) -> bool {
|
||||
!online || destination_is_local
|
||||
}
|
||||
|
||||
/// Convert one kernel pathname component into the UTF-8 name accepted by Drive.
|
||||
/// Linux filesystems conventionally cap a component at `NAME_MAX` (255 bytes),
|
||||
/// but the FUSE kernel path can pass a longer component through to userspace.
|
||||
fn fuse_name(name: &OsStr) -> Result<String, Errno> {
|
||||
let bytes = name.as_bytes();
|
||||
if bytes.len() > 255 {
|
||||
return Err(Errno::ENAMETOOLONG);
|
||||
}
|
||||
if bytes.is_empty() || bytes == b"." || bytes == b".." {
|
||||
return Err(Errno::EINVAL);
|
||||
}
|
||||
name.to_str().map(str::to_owned).ok_or(Errno::EILSEQ)
|
||||
}
|
||||
|
||||
fn node_visible(node: &Node, folder_uid: &NodeUid, hidden: &HashSet<NodeUid>) -> bool {
|
||||
!node.trashed && node.uid != *folder_uid && !hidden.contains(&node.uid)
|
||||
}
|
||||
|
||||
/// Bytes rendered with a binary unit and one decimal place (e.g. `"1.2 GB"`),
|
||||
/// for the activity log. Uses 1024-based steps but the shorter SI labels.
|
||||
fn human_bytes(bytes: u64) -> String {
|
||||
@@ -3287,7 +3332,8 @@ mod pending_size_tests {
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::{
|
||||
Intervals, PendingRevision, conflict_name, copy_pending_for_truncate, is_stale_mount,
|
||||
Intervals, PendingRevision, conflict_name, copy_pending_for_truncate, fuse_name,
|
||||
is_stale_mount, node_visible, rename_needs_queue,
|
||||
};
|
||||
use pdfs_core::cache::{Baseline, StagedWrite};
|
||||
|
||||
@@ -3456,6 +3502,64 @@ mod tests {
|
||||
assert_ne!(pending.meta.authored, vec![(0, pending.meta.len)]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn online_combined_cross_directory_rename_stays_synchronous() {
|
||||
assert!(!rename_needs_queue(true, false, true, true));
|
||||
assert!(rename_needs_queue(false, false, false, true));
|
||||
assert!(rename_needs_queue(true, true, true, false));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn one_step_online_namespace_changes_stay_synchronous() {
|
||||
assert!(!rename_needs_queue(true, false, false, true));
|
||||
assert!(!rename_needs_queue(true, false, true, false));
|
||||
assert!(!rename_needs_queue(true, false, false, false));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn fuse_names_enforce_linux_component_limits() {
|
||||
use std::ffi::OsStr;
|
||||
|
||||
assert_eq!(
|
||||
fuse_name(OsStr::new("x".repeat(255).as_str()))
|
||||
.unwrap()
|
||||
.len(),
|
||||
255
|
||||
);
|
||||
assert_eq!(
|
||||
fuse_name(OsStr::new("x".repeat(256).as_str()))
|
||||
.expect_err("256-byte component must be refused")
|
||||
.code(),
|
||||
libc::ENAMETOOLONG
|
||||
);
|
||||
assert_eq!(
|
||||
fuse_name(OsStr::new("."))
|
||||
.expect_err("dot is not an ordinary component")
|
||||
.code(),
|
||||
libc::EINVAL
|
||||
);
|
||||
assert_eq!(
|
||||
fuse_name(OsStr::new("unicodé-文件")).unwrap(),
|
||||
"unicodé-文件"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn local_trash_tombstones_hide_stale_remote_entries() {
|
||||
let folder_uid = NodeUid::new(VolumeId::from("vol"), LinkId::from("folder"));
|
||||
let node = node_helper("child", "folder", "child", false);
|
||||
assert!(node_visible(
|
||||
&node,
|
||||
&folder_uid,
|
||||
&std::collections::HashSet::new()
|
||||
));
|
||||
assert!(!node_visible(
|
||||
&node,
|
||||
&folder_uid,
|
||||
&std::collections::HashSet::from([node.uid.clone()])
|
||||
));
|
||||
}
|
||||
|
||||
use proton_drive_rs::proton_sdk::ids::{LinkId, NodeUid, VolumeId};
|
||||
use proton_drive_rs::{Node, NodeKind};
|
||||
|
||||
|
||||
@@ -192,6 +192,7 @@ pub fn mount(
|
||||
db,
|
||||
online: Arc::new(AtomicBool::new(online)),
|
||||
pending: Arc::new(Mutex::new(HashMap::new())),
|
||||
hidden: Arc::new(Mutex::new(HashSet::new())),
|
||||
drain_wake: Arc::new((Mutex::new(false), Condvar::new())),
|
||||
timeline_refreshing: Arc::new(AtomicBool::new(false)),
|
||||
trash_refreshing: Arc::new(AtomicBool::new(false)),
|
||||
|
||||
@@ -219,10 +219,12 @@ impl State {
|
||||
|
||||
/// Check if a directory inode has any child nodes in memory or in the database.
|
||||
pub(crate) fn has_children(&self, parent: u64) -> bool {
|
||||
if let Some(kids) = self.children.get(&parent)
|
||||
&& !kids.is_empty()
|
||||
{
|
||||
return true;
|
||||
// A resident listing is authoritative even when empty. Falling through
|
||||
// from `Some([])` to SQLite lets an obsolete child row contradict the
|
||||
// listing `ensure_children` just established, so readdir says empty but
|
||||
// rmdir returns ENOTEMPTY forever.
|
||||
if let Some(kids) = self.children.get(&parent) {
|
||||
return !kids.is_empty();
|
||||
}
|
||||
if let Some(entry) = self.entries.get(&parent)
|
||||
&& let Ok(has_kids) = self.db.has_children(&entry.uid)
|
||||
@@ -607,6 +609,11 @@ mod tests {
|
||||
st.db
|
||||
.upsert_node(&node("file2_uid", "dir_uid", "file2.txt", false))
|
||||
.unwrap();
|
||||
assert!(
|
||||
!st.has_children(folder),
|
||||
"a resident empty listing overrides stale DB children"
|
||||
);
|
||||
st.children.remove(&folder);
|
||||
assert!(st.has_children(folder), "has child in db");
|
||||
}
|
||||
}
|
||||
+152
-9
@@ -1587,16 +1587,23 @@ counts to baseline and observe no post-unmount DB work.
|
||||
|
||||
---
|
||||
|
||||
## B45 — Truncate After a Queued Rewrite Replaces the Prefix With Zeros
|
||||
## B45 — Truncate After a Queued Rewrite Corrupts or Returns EIO
|
||||
|
||||
**Status:** Fixed and live-verified (2026-07-22)
|
||||
**Found by:** `scripts/fuse-acceptance.sh`
|
||||
**Where:** `crates/pdfs-fuse/src/filesystem.rs`, write-open scratch setup
|
||||
**Where:** `crates/pdfs-fuse/src/filesystem.rs`, write-open scratch setup;
|
||||
`crates/pdfs-fuse/src/lib.rs`, `Core::queue_truncate`
|
||||
|
||||
**Repro:** Write a file, immediately replace its contents, then run
|
||||
`truncate -s 4 file` before the queued replacement drains. Reading it returned
|
||||
four NUL bytes instead of the first four replacement bytes.
|
||||
|
||||
The expanded managed suite found the path-based variant: create and close a
|
||||
10-byte file, then immediately call `truncate(path, 4)`. The close's complete
|
||||
revision was still inside the two-second drain debounce. `queue_truncate`
|
||||
described the shrink as an incomplete edit over the remote base, noticed the
|
||||
pending edit, preserved an orphan staging file, and returned `EIO`.
|
||||
|
||||
**Cause:** A new write handle always treated the server revision as its base.
|
||||
When a newer complete revision was still queued locally, the handle's scratch
|
||||
file began sparse and zero-filled. Shrinking it preserved those scratch zeros,
|
||||
@@ -1604,8 +1611,10 @@ not the prefix of the locally authoritative queued revision.
|
||||
|
||||
**Fix:** A write opened over a complete queued revision seeds its scratch file
|
||||
from that staged blob and marks the copied range authored. Stacking a write over
|
||||
an incomplete queued revision is refused rather than guessed. Live acceptance
|
||||
verified the exact rewrite-then-truncate sequence returns `b"a mu"`.
|
||||
an incomplete queued revision is refused rather than guessed. Path-based
|
||||
truncate now composes the same way: it copies a complete pending blob, applies
|
||||
the new length, marks the result complete, and inherits the last real remote
|
||||
baseline. Live acceptance verified shrink, zero-filled growth, and sparse I/O.
|
||||
|
||||
---
|
||||
|
||||
@@ -1624,8 +1633,142 @@ date"), surfaced as `EIO`.
|
||||
link-details read can briefly observe the pre-move state, so the new name is
|
||||
signed against stale requirements.
|
||||
|
||||
**Fix:** Retry only `InvalidRequirements` from the rename-after-move step with a
|
||||
short bounded backoff. Stable API failures are not retried. If every retry
|
||||
fails, adopt the already-completed move in inode state before returning `EIO`,
|
||||
so the local model never claims the node remains in its old parent. The full
|
||||
live suite subsequently passed the replacement and combined move/rename cases.
|
||||
**Fix history:** A bounded retry of `InvalidRequirements` reduced the window but
|
||||
did not close it; the managed matrix reproduced the same failure after all four
|
||||
retries. Combined cross-directory move+rename now enters the durable rename
|
||||
queue as a desired end state. Its drain re-fetches the node on every attempt,
|
||||
skips either half that already landed, renames before moving, and resolves name
|
||||
collisions without losing the source. Simple rename-only and move-only calls
|
||||
remain synchronous. The managed live suite subsequently passed the replacement
|
||||
and combined move/rename cases.
|
||||
|
||||
---
|
||||
|
||||
## B47 — FUSE Accepts Path Components Longer Than NAME_MAX
|
||||
|
||||
**Status:** Fixed (unverified)
|
||||
**Found:** 2026-07-22, managed FUSE acceptance suite
|
||||
**Where:** `crates/pdfs-fuse/src/filesystem.rs`, name-taking callbacks
|
||||
|
||||
**Repro:** `open(<mount>/<256 ASCII bytes>, O_CREAT)` succeeded. A conventional
|
||||
Linux filesystem must reject a pathname component longer than 255 bytes with
|
||||
`ENAMETOOLONG`.
|
||||
|
||||
**Cause:** Every callback converted `OsStr` with `to_string_lossy` and passed it
|
||||
straight to Drive. This imposed neither Linux's component limit nor Drive's
|
||||
UTF-8 requirement, and could silently change non-UTF-8 names.
|
||||
|
||||
**Fix:** Route lookup, create, mkdir, unlink, rmdir, and both rename components
|
||||
through one validator. It enforces the 255-byte limit, rejects reserved/empty
|
||||
components, rejects invalid UTF-8 with `EILSEQ`, and preserves valid Unicode.
|
||||
The managed live suite is the verification gate.
|
||||
|
||||
---
|
||||
|
||||
## T1 — Managed Acceptance Raced Daemon Startup
|
||||
|
||||
**Status:** Fixed (2026-07-22)
|
||||
**Found:** repeated install/restart/acceptance loop
|
||||
**Where:** `scripts/fuse-acceptance.py`, managed setup
|
||||
|
||||
**Repro:** Run `systemctl --user restart proton-drive.service` and immediately
|
||||
start `--managed-live`. The offline reference completed before the daemon had
|
||||
recreated its control socket, so the first `pdfs --json sync list` failed with
|
||||
`ENOENT` and setup aborted.
|
||||
|
||||
**Fix:** Managed validation polls the supported `sync list` API until the daemon
|
||||
is ready or `PDFS_ACCEPTANCE_SYNC_TIMEOUT` expires. This is tracked as a test
|
||||
harness defect rather than an application bug.
|
||||
|
||||
---
|
||||
|
||||
## T2 — Acceptance Cleanup Raced Queued Namespace Replay
|
||||
|
||||
**Status:** Fixed (2026-07-22)
|
||||
**Found:** managed on-demand/on-demand matrix after every functional test passed
|
||||
**Where:** `scripts/fuse-acceptance.py`, per-contract cleanup
|
||||
|
||||
**Repro:** A combined move+rename was correctly accepted into the durable queue.
|
||||
Cleanup started recursively deleting its test tree while the rename drain was
|
||||
still landing. The moved entry appeared in `dir-b` after `rmtree` enumerated it
|
||||
but before `rmdir(dir-b)`, producing `ENOTEMPTY`.
|
||||
|
||||
**Fix:** Before cleanup, poll `pdfs --json status` until both pending counters
|
||||
are zero. Retry `rmtree` only for `ENOTEMPTY` inside the uniquely owned test
|
||||
root, then wait for the queued deletions before changing modes. Other errors are
|
||||
still immediate failures.
|
||||
|
||||
---
|
||||
|
||||
## T3 — Mode-Matrix Preservation Check Raced Mirror Restoration
|
||||
|
||||
**Status:** Fixed (2026-07-22)
|
||||
**Found:** managed on-demand/on-demand → on-demand/mirror transition
|
||||
**Where:** `scripts/fuse-acceptance.py`, managed mode switching
|
||||
|
||||
**Repro:** Switch a populated managed folder from `ondemand` to `mirror`, wait
|
||||
until `sync list` reports mirror/idle, and immediately read a preservation file.
|
||||
The file can still be absent even though the subsequent restore pass downloads
|
||||
it correctly.
|
||||
|
||||
**Cause:** The application commits the new mode before scheduling reconciliation
|
||||
and leaves the previous idle state and sync timestamp in the row. Those values
|
||||
describe the completed on-demand state, not a completed mirror restoration.
|
||||
|
||||
**Fix:** The harness records every transition to mirror and explicitly requests
|
||||
and waits for a newer completed sync pass before validating local bytes. This is
|
||||
tracked as a harness synchronization defect; transient absence before that pass
|
||||
is expected asynchronous behavior, not data loss.
|
||||
|
||||
---
|
||||
|
||||
## T4 — Successful Managed Cleanup Left Its Own Sentinel Behind
|
||||
|
||||
**Status:** Fixed (2026-07-22)
|
||||
**Found:** second consecutive managed acceptance run
|
||||
**Where:** `scripts/fuse-acceptance.py`, managed cleanup
|
||||
|
||||
**Repro:** Complete the mode matrix in mirror/mirror, then immediately start a
|
||||
new managed run. Registration cleanup removed the sync-folder records and remote
|
||||
test folders, but the next run's empty-directory precheck found the harness's
|
||||
128 KiB preservation sentinel in each local directory.
|
||||
|
||||
**Cause:** Removing a mirror registration intentionally preserves the local
|
||||
copy. The harness treated unregistering as if it also emptied local storage.
|
||||
|
||||
**Fix:** After successful unregister, cleanup deletes only the sentinel whose
|
||||
contents match the per-run bytes generated by this harness. Unknown files remain
|
||||
untouched and continue to fail the strict precheck.
|
||||
|
||||
---
|
||||
|
||||
## B48 — Successful Unlink Can Reappear From a Stale Remote Listing
|
||||
|
||||
**Status:** Fixed (unverified)
|
||||
**Found:** 2026-07-22, managed acceptance cleanup after all on-demand I/O passed
|
||||
**Where:** `crates/pdfs-fuse/src/lib.rs`, child enumeration and trash paths
|
||||
|
||||
**Repro:** Unlink every child of a directory and immediately remove the
|
||||
directory. `unlink` returned success, but a parent invalidation followed by an
|
||||
eventually consistent remote enumeration returned the just-trashed child again.
|
||||
`rmdir` then returned `ENOTEMPTY`; repeated recursive removal could reproduce it
|
||||
for the entire acceptance timeout.
|
||||
|
||||
**Cause:** Three stale-data paths combined. A queued cross-directory rename
|
||||
updated `State` but did not invalidate the kernel's cached empty destination
|
||||
listing, so recursive walkers never saw or unlinked the moved child. Removing a node recorded no
|
||||
authoritative local tombstone, so an eventually consistent Drive listing could
|
||||
intern it again. Separately, `State::has_children` treated a resident empty
|
||||
listing as inconclusive and fell back to obsolete SQLite child rows. Thus
|
||||
`readdir` could report empty while `rmdir` returned `ENOTEMPTY` forever.
|
||||
|
||||
**Fix:** Every successful online or queued trash records its uid in a
|
||||
session-shared hidden set. Both persisted and remote child enumeration filter
|
||||
that set. Secondary on-demand mounts share it, and an explicit restore removes
|
||||
the uid so restored content can appear normally. `has_children` now treats any
|
||||
resident listing—including an empty one—as authoritative and consults SQLite
|
||||
only when the listing is absent. After replying successfully to a queued rename,
|
||||
the FUSE handler invalidates both exact dentries and both directory inodes; the
|
||||
ordering matters because an invalidation sent before the rename reply is
|
||||
overwritten by the kernel's response processing. Uids are immutable and never
|
||||
reused, making a session-long tombstone safe.
|
||||
@@ -53,6 +53,10 @@ in `pdfs sync list`. Use `PDFS_ACCEPTANCE_SYNC_TIMEOUT` to extend transition
|
||||
timeouts and `PDFS_ACCEPTANCE_PDFS` to select a non-installed CLI binary, for
|
||||
example `target/debug/pdfs`.
|
||||
|
||||
During development, `PDFS_ACCEPTANCE_ONLY` runs tests whose descriptive name
|
||||
contains the supplied text (for example `PDFS_ACCEPTANCE_ONLY=namespace`). A
|
||||
release acceptance run must leave it unset so the complete contract executes.
|
||||
|
||||
Passing more paths runs the same filesystem suite independently against each.
|
||||
The first path must be the FUSE mount under test; later paths may be another
|
||||
FUSE mount or a normal local mirror filesystem:
|
||||
|
||||
@@ -160,6 +160,7 @@ def test_namespace_and_errors(root: Path) -> None:
|
||||
check(read(root / "victim") == b"source" and not (root / "source").exists(), "replace failed")
|
||||
os.rename(root / "victim", b / "moved")
|
||||
check(read(b / "moved") == b"source", "cross-directory move failed")
|
||||
check("moved" in os.listdir(b), "cross-directory move missing from readdir")
|
||||
os.rename(b / "moved", b / "moved")
|
||||
check(read(b / "moved") == b"source", "same-name rename changed data")
|
||||
|
||||
@@ -257,10 +258,14 @@ def run_contract(parent: Path, label: str) -> tuple[Path, str]:
|
||||
root.mkdir()
|
||||
print(f"[target] {label}: {parent}")
|
||||
try:
|
||||
for name, operation in TESTS:
|
||||
selected = os.environ.get("PDFS_ACCEPTANCE_ONLY")
|
||||
tests = TESTS if not selected else [test for test in TESTS if selected.lower() in test[0].lower()]
|
||||
check(bool(tests), f"PDFS_ACCEPTANCE_ONLY={selected!r} matched no tests")
|
||||
for name, operation in tests:
|
||||
print(f" [test] {name}")
|
||||
operation(root)
|
||||
digest = hashlib.sha256(read(root / "positioned.bin")).hexdigest()
|
||||
digest_path = root / "positioned.bin"
|
||||
digest = hashlib.sha256(read(digest_path)).hexdigest() if digest_path.exists() else ""
|
||||
return root, digest
|
||||
except BaseException:
|
||||
shutil.rmtree(root, ignore_errors=True)
|
||||
@@ -310,6 +315,41 @@ class ManagedSyncPair:
|
||||
value = json.loads(self.command("sync", "list", json_output=True))
|
||||
return value["items"]
|
||||
|
||||
def wait_for_queue(self) -> None:
|
||||
import json
|
||||
deadline = time.monotonic() + self.timeout
|
||||
last = None
|
||||
while time.monotonic() < deadline:
|
||||
value = json.loads(self.command("status", json_output=True))
|
||||
last = value.get("mount") or {}
|
||||
if last.get("pending_uploads", 0) == 0 and last.get("pending_changes", 0) == 0:
|
||||
return
|
||||
time.sleep(1)
|
||||
raise TimeoutError(f"daemon mutation queue did not drain: {last}")
|
||||
|
||||
def remove_tree(self, root: Path) -> None:
|
||||
deadline = time.monotonic() + self.timeout
|
||||
while True:
|
||||
try:
|
||||
shutil.rmtree(root)
|
||||
return
|
||||
except FileNotFoundError:
|
||||
return
|
||||
except OSError as error:
|
||||
if error.errno != errno.ENOTEMPTY or time.monotonic() >= deadline:
|
||||
survivors = []
|
||||
if root.exists():
|
||||
for directory, dirs, files in os.walk(root):
|
||||
survivors.extend(str(Path(directory) / name) for name in dirs + files)
|
||||
raise OSError(
|
||||
error.errno,
|
||||
f"{error.strerror}; surviving test entries: {survivors[:50]}",
|
||||
error.filename,
|
||||
) from error
|
||||
# A queued namespace operation can land between rmtree's
|
||||
# enumeration and rmdir. Re-walk the test-owned tree.
|
||||
time.sleep(0.25)
|
||||
|
||||
def validate(self) -> None:
|
||||
if shutil.which(self.pdfs) is None and not Path(self.pdfs).is_file():
|
||||
raise RuntimeError(
|
||||
@@ -317,7 +357,20 @@ class ManagedSyncPair:
|
||||
)
|
||||
if self.paths[0] == self.paths[1]:
|
||||
raise ValueError("managed live paths must be different directories")
|
||||
registered = {Path(item["local_path"]).resolve() for item in self.folders()}
|
||||
deadline = time.monotonic() + self.timeout
|
||||
last_error = None
|
||||
while True:
|
||||
try:
|
||||
current = self.folders()
|
||||
break
|
||||
except Exception as error:
|
||||
last_error = error
|
||||
if time.monotonic() >= deadline:
|
||||
raise TimeoutError(
|
||||
f"pdfs daemon did not become ready within {self.timeout}s: {last_error}"
|
||||
) from error
|
||||
time.sleep(1)
|
||||
registered = {Path(item["local_path"]).resolve() for item in current}
|
||||
for path in self.paths:
|
||||
check(path.is_dir(), f"managed path is not a directory: {path}")
|
||||
check(not any(path.iterdir()), f"managed path is not empty: {path}")
|
||||
@@ -376,13 +429,21 @@ class ManagedSyncPair:
|
||||
raise TimeoutError(f"forced sync for {path} did not complete: {last}")
|
||||
|
||||
def set_modes(self, modes: tuple[str, str]) -> None:
|
||||
changed_to_mirror: set[Path] = set()
|
||||
for path, mode in zip(self.paths, modes, strict=True):
|
||||
current = self.wait_for(path)
|
||||
if current["mode"] != mode:
|
||||
print(f"[setup] switching {path} to {mode}")
|
||||
self.command("sync", "mode", str(self.ids[path]), mode)
|
||||
if mode == "mirror":
|
||||
changed_to_mirror.add(path)
|
||||
for path, mode in zip(self.paths, modes, strict=True):
|
||||
self.wait_for(path, mode=mode)
|
||||
# The mode row flips before the asynchronous restore pass starts,
|
||||
# and its prior idle/last_sync values remain visible meanwhile.
|
||||
# Demand a completed pass before inspecting restored local bytes.
|
||||
if path in changed_to_mirror:
|
||||
self.force_sync(path)
|
||||
mounted = is_mountpoint(path)
|
||||
check(mounted == (mode == "ondemand"), f"{path}: mode is {mode}, mounted={mounted}")
|
||||
check(
|
||||
@@ -407,6 +468,17 @@ class ManagedSyncPair:
|
||||
self.command("sync", "rm", str(folder_id), "--delete-remote")
|
||||
except Exception as error:
|
||||
print(f"WARNING: cleanup failed for sync folder {folder_id}: {error}")
|
||||
continue
|
||||
# Unregistering a mirror intentionally preserves its local copy.
|
||||
# Remove only the sentinel owned by this harness so the next run's
|
||||
# strict empty-directory precondition remains meaningful. Unknown
|
||||
# survivors are left untouched and will fail validate() next time.
|
||||
sentinel = path / "pdfs-mode-preservation.bin"
|
||||
try:
|
||||
if sentinel.exists() and read(sentinel) == self.sentinels[path]:
|
||||
sentinel.unlink()
|
||||
except OSError as error:
|
||||
print(f"WARNING: could not remove managed sentinel {sentinel}: {error}")
|
||||
|
||||
|
||||
def is_mountpoint(path: Path) -> bool:
|
||||
@@ -432,8 +504,10 @@ def run_managed_matrix(paths: list[Path]) -> None:
|
||||
for path, mode in zip(pair.paths, modes, strict=True):
|
||||
root, _ = run_contract(path, f"managed {mode}")
|
||||
roots.append(root)
|
||||
shutil.rmtree(root)
|
||||
pair.wait_for_queue()
|
||||
pair.remove_tree(root)
|
||||
roots.remove(root)
|
||||
pair.wait_for_queue()
|
||||
# Mirror changes are asynchronous; force and await a pass before
|
||||
# changing its mode so authored bytes/deletions cannot be lost.
|
||||
if mode == "mirror":
|
||||
|
||||
@@ -23,13 +23,13 @@ case "${1:-}" in
|
||||
-h|--help) usage ;;
|
||||
--offline-only|"")
|
||||
(( $# <= 1 )) || usage
|
||||
exec python3 "$script_dir/fuse-acceptance.py"
|
||||
exec python3 -u "$script_dir/fuse-acceptance.py"
|
||||
;;
|
||||
--live) shift ; (( $# > 0 )) || usage ;;
|
||||
--managed-live)
|
||||
shift
|
||||
(( $# == 2 )) || usage
|
||||
exec python3 "$script_dir/fuse-acceptance.py" --managed-live "$@"
|
||||
exec python3 -u "$script_dir/fuse-acceptance.py" --managed-live "$@"
|
||||
;;
|
||||
--*) usage ;;
|
||||
esac
|
||||
@@ -44,4 +44,4 @@ fstype="$(findmnt -T "$1" -n -o FSTYPE | head -n 1)"
|
||||
exit 2
|
||||
}
|
||||
|
||||
exec python3 "$script_dir/fuse-acceptance.py" --live "$@"
|
||||
exec python3 -u "$script_dir/fuse-acceptance.py" --live "$@"
|
||||
Reference in new issue
Block a user