From 25f578161d4a24bdb62eb1c335a53905d6b297df Mon Sep 17 00:00:00 2001 From: Nils Pukropp Date: Wed, 22 Jul 2026 11:43:17 +0200 Subject: [PATCH 1/3] added more comprehensive testing --- docs/TESTING.md | 48 +++- scripts/fuse-acceptance.py | 498 +++++++++++++++++++++++++++++++++++++ scripts/fuse-acceptance.sh | 229 +++-------------- 3 files changed, 570 insertions(+), 205 deletions(-) create mode 100755 scripts/fuse-acceptance.py diff --git a/docs/TESTING.md b/docs/TESTING.md index b6d4c84..2961104 100644 --- a/docs/TESTING.md +++ b/docs/TESTING.md @@ -9,9 +9,21 @@ cargo test --workspace cargo clippy --workspace --all-targets -- -D warnings ``` -## Live FUSE acceptance suite +## Filesystem API and live FUSE acceptance suite -Kernel behavior and remote convergence need a real mount. The live suite runs +The account-free suite exercises the filesystem syscall contract against a +temporary local directory. This validates the runner and covers creation/open +flags, positioned and vectored I/O, truncation and sparse files, `mmap`, +`sendfile`, durability barriers, namespace/error semantics, open-file lifetime, +names and enumeration, and concurrent I/O. It never reads application state or +contacts Proton: + +```console +scripts/fuse-acceptance.sh --offline-only +``` + +Kernel/FUSE behavior and remote convergence need a real mount. Live testing is +explicitly opt-in and always runs the account-free contract first. It runs destructive POSIX operations only below a fresh `pdfs-acceptance-*` directory. It refuses paths that are not FUSE mounts and removes its directory on success, failure, or interruption. @@ -19,16 +31,35 @@ failure, or interruption. Use a dedicated test account without irreplaceable data: ```console -scripts/fuse-acceptance.sh /mnt/on-demand-testmount +scripts/fuse-acceptance.sh --live /mnt/on-demand-testmount ``` +### Automated mode matrix + +The managed live runner accepts two existing, empty, unmounted directories. It +registers both as new sync folders, waits for initial synchronization, then +tests all four pairs in order: on-demand/on-demand, on-demand/mirror, +mirror/on-demand, and mirror/mirror. Every transition must become idle, and an +on-demand folder must actually appear as a mount before its contract runs. + +```console +mkdir -p /mnt/pdfs-test-a /mnt/pdfs-test-b +scripts/fuse-acceptance.sh --managed-live /mnt/pdfs-test-a /mnt/pdfs-test-b +``` + +This creates remote folders and permanently deletes those test remotes during +cleanup. It refuses non-empty paths, existing mounts, and paths already present +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`. + 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: ```console PDFS_ACCEPTANCE_SYNC_TIMEOUT=180 \ - scripts/fuse-acceptance.sh /mnt/on-demand-testmount /mnt/testmount + scripts/fuse-acceptance.sh --live /mnt/on-demand-testmount /mnt/testmount ``` If two paths really are views of the **same remote folder**, enable an @@ -36,16 +67,15 @@ additional byte-for-byte convergence check explicitly: ```console PDFS_ACCEPTANCE_CONVERGENCE=1 \ - scripts/fuse-acceptance.sh /mnt/first-view /mnt/second-view + scripts/fuse-acceptance.sh --live /mnt/first-view /mnt/second-view ``` Do not enable that flag merely because two sync folders belong to the same account. Their configured remote UIDs must be identical. -The suite covers create/read/rewrite/truncate, multi-megabyte binary content, -unaligned random writes, rename replacement, cross-directory moves, important -POSIX error cases, open-then-unlink behavior, concurrent writers, directory -enumeration, and optional cross-mount byte-for-byte convergence. A pass is +Normally the live suite applies the same API contract independently to each +supplied mount. In convergence mode it exercises the primary once and verifies +that the resulting bytes appear through every secondary view. A pass is strong evidence for those paths, not a blanket data-loss guarantee. Before a release, also run the recovery drills in `docs/RECOVERY.md` and inspect `docs/BUGS.md` for live-verification items. diff --git a/scripts/fuse-acceptance.py b/scripts/fuse-acceptance.py new file mode 100755 index 0000000..57706d5 --- /dev/null +++ b/scripts/fuse-acceptance.py @@ -0,0 +1,498 @@ +#!/usr/bin/env python3 +"""Account-free filesystem API contract plus opt-in live FUSE acceptance.""" + +from __future__ import annotations + +import argparse +import concurrent.futures +import errno +import hashlib +import mmap +import os +from pathlib import Path +import shutil +import stat +import subprocess +import tempfile +import time +import uuid + + +def check(condition: bool, message: str) -> None: + if not condition: + raise AssertionError(message) + + +def expect_errno(expected: set[int], operation, description: str) -> None: + try: + operation() + except OSError as error: + check(error.errno in expected, f"{description}: errno {error.errno}, expected {expected}") + else: + raise AssertionError(f"{description}: unexpectedly succeeded") + + +def read(path: Path) -> bytes: + with path.open("rb", buffering=0) as file: + return file.read() + + +def write_durable(path: Path, data: bytes) -> None: + fd = os.open(path, os.O_CREAT | os.O_WRONLY | os.O_TRUNC, 0o600) + try: + view = memoryview(data) + while view: + written = os.write(fd, view) + check(written > 0, "write made no progress") + view = view[written:] + os.fsync(fd) + finally: + os.close(fd) + + +def test_create_flags(root: Path) -> None: + path = root / "flags" + fd = os.open(path, os.O_CREAT | os.O_EXCL | os.O_RDWR, 0o600) + os.write(fd, b"abcdef") + os.close(fd) + expect_errno({errno.EEXIST}, lambda: os.open(path, os.O_CREAT | os.O_EXCL), "O_EXCL") + fd = os.open(path, os.O_WRONLY | os.O_APPEND) + os.lseek(fd, 0, os.SEEK_SET) + os.write(fd, b"++") + os.close(fd) + check(read(path) == b"abcdef++", "O_APPEND ignored") + fd = os.open(path, os.O_WRONLY | os.O_TRUNC) + os.close(fd) + check(path.stat().st_size == 0, "O_TRUNC did not truncate") + + +def test_positioned_and_vectored_io(root: Path) -> None: + path = root / "positioned.bin" + base = bytes(range(256)) * 32768 + write_durable(path, base) + fd = os.open(path, os.O_RDWR) + try: + check(os.pread(fd, 19, 4093) == base[4093:4112], "pread returned wrong range") + check(os.pwrite(fd, b"boundary-write", 4093) == 14, "short pwrite") + if hasattr(os, "pwritev"): + check(os.pwritev(fd, [b"vector-", b"write"], 65531) == 12, "short pwritev") + if hasattr(os, "preadv"): + chunks = [bytearray(7), bytearray(5)] + check(os.preadv(fd, chunks, 65531) == 12, "short preadv") + check(b"".join(chunks) == b"vector-write", "preadv data mismatch") + os.fdatasync(fd) + finally: + os.close(fd) + expected = bytearray(base) + expected[4093:4107] = b"boundary-write" + expected[65531:65543] = b"vector-write" + got = read(path) + check(got == expected, "positioned write damaged surrounding bytes") + print(f" sha256 {hashlib.sha256(got).hexdigest()}") + + +def test_resize_and_sparse_io(root: Path) -> None: + path = root / "resize" + write_durable(path, b"0123456789") + os.truncate(path, 4) + check(read(path) == b"0123", "shrink did not preserve prefix") + os.truncate(path, 8193) + data = read(path) + check(data[:4] == b"0123" and data[4:] == bytes(8189), "grown range is not zero-filled") + + sparse = root / "sparse" + fd = os.open(sparse, os.O_CREAT | os.O_RDWR, 0o600) + try: + os.pwrite(fd, b"head", 0) + os.pwrite(fd, b"tail", 8 * 1024 * 1024 + 17) + os.fsync(fd) + finally: + os.close(fd) + check(sparse.stat().st_size == 8 * 1024 * 1024 + 21, "sparse size mismatch") + fd = os.open(sparse, os.O_RDONLY) + try: + check(os.pread(fd, 8, 4) == bytes(8), "sparse hole is not zero-filled") + check(os.pread(fd, 4, 8 * 1024 * 1024 + 17) == b"tail", "sparse tail missing") + check(os.pread(fd, 1, sparse.stat().st_size + 4096) == b"", "read beyond EOF not empty") + finally: + os.close(fd) + + +def test_mmap_and_copy_paths(root: Path) -> None: + source = root / "mapped" + data = bytearray((b"mmap-and-sendfile\0" * 65536)[:1024 * 1024]) + write_durable(source, data) + fd = os.open(source, os.O_RDWR) + try: + with mmap.mmap(fd, len(data), access=mmap.ACCESS_WRITE) as mapping: + mapping[4091:4107] = b"mapped-boundary!" + mapping.flush() + os.fsync(fd) + finally: + os.close(fd) + data[4091:4107] = b"mapped-boundary!" + check(read(source) == data, "shared mmap write/read mismatch") + + target = root / "sendfile-copy" + src = os.open(source, os.O_RDONLY) + dst = os.open(target, os.O_CREAT | os.O_WRONLY | os.O_TRUNC, 0o600) + try: + offset = 0 + while offset < len(data): + count = os.sendfile(dst, src, offset, len(data) - offset) + check(count > 0, "sendfile made no progress") + offset += count + os.fsync(dst) + finally: + os.close(src) + os.close(dst) + check(read(target) == data, "sendfile copy mismatch") + + +def test_namespace_and_errors(root: Path) -> None: + a, b = root / "dir-a", root / "dir-b" + a.mkdir(); b.mkdir() + (a / "nested").mkdir() + write_durable(a / "nested" / "child", b"child") + write_durable(root / "source", b"source") + write_durable(root / "victim", b"victim") + os.replace(root / "source", root / "victim") + 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") + os.rename(b / "moved", b / "moved") + check(read(b / "moved") == b"source", "same-name rename changed data") + + expect_errno({errno.ENOTEMPTY, errno.EEXIST}, lambda: os.rmdir(a), "rmdir(non-empty)") + expect_errno({errno.ENOTDIR}, lambda: os.rmdir(b / "moved"), "rmdir(file)") + expect_errno({errno.EISDIR, errno.EPERM}, lambda: os.unlink(a), "unlink(directory)") + expect_errno({errno.EINVAL}, lambda: os.rename(a, a / "nested" / "cycle"), "directory cycle") + expect_errno({errno.ENOENT}, lambda: os.unlink(root / "absent"), "unlink(absent)") + expect_errno({errno.ENOENT}, lambda: os.rename(root / "absent", root / "new"), "rename(absent)") + + empty_src, empty_dst = root / "empty-src", root / "empty-dst" + empty_src.mkdir(); empty_dst.mkdir() + os.replace(empty_src, empty_dst) + check(empty_dst.is_dir() and not empty_src.exists(), "empty directory replacement failed") + nonempty = root / "nonempty-dst" + nonempty.mkdir(); write_durable(nonempty / "keep", b"keep") + replacement = root / "replacement-dir"; replacement.mkdir() + expect_errno({errno.ENOTEMPTY, errno.EEXIST}, lambda: os.replace(replacement, nonempty), "replace non-empty dir") + check(read(nonempty / "keep") == b"keep", "failed replacement damaged destination") + + +def test_open_lifetime(root: Path) -> None: + path = root / "open-unlink" + data = b"held-open\0" * 4096 + write_durable(path, data) + fd = os.open(path, os.O_RDWR) + os.unlink(path) + check(not path.exists(), "unlinked name remains visible") + check(os.pread(fd, len(data), 0) == data, "open file lost data after unlink") + os.pwrite(fd, b"still-open", 0) + check(os.pread(fd, 10, 0) == b"still-open", "unlinked open file is not writable") + os.close(fd) + + old, new = root / "open-rename-old", root / "open-rename-new" + write_durable(old, b"before") + fd = os.open(old, os.O_RDWR) + os.rename(old, new) + os.pwrite(fd, b"after!", 0) + os.fsync(fd); os.close(fd) + check(not old.exists() and read(new) == b"after!", "open handle did not follow rename") + + +def test_names_and_enumeration(root: Path) -> None: + names = ["space name.txt", "unicodé-文件", ".hidden", "trailing.dot.", "case", "CASE"] + for index, name in enumerate(names): + write_durable(root / name, f"name-{index}".encode()) + listed = set(os.listdir(root)) + check(set(names) <= listed, "directory enumeration omitted valid names") + for name in listed: + os.lstat(root / name) + expect_errno({errno.ENAMETOOLONG}, lambda: os.open(root / ("x" * 256), os.O_CREAT), "overlong name") + + +def test_concurrency(root: Path) -> None: + def independent(index: int) -> None: + data = bytes([index]) * (131072 + index) + path = root / f"concurrent-{index}" + write_durable(path, data) + check(read(path) == data, f"concurrent file {index} mismatch") + + with concurrent.futures.ThreadPoolExecutor(max_workers=8) as pool: + list(pool.map(independent, range(1, 17))) + + shared = root / "concurrent-ranges" + extent = 64 * 1024 + write_durable(shared, bytes(extent * 8)) + fd = os.open(shared, os.O_RDWR) + try: + def positioned(index: int) -> None: + payload = bytes([index + 1]) * extent + check(os.pwrite(fd, payload, index * extent) == extent, f"short concurrent pwrite {index}") + with concurrent.futures.ThreadPoolExecutor(max_workers=8) as pool: + list(pool.map(positioned, range(8))) + os.fsync(fd) + finally: + os.close(fd) + expected = b"".join(bytes([i + 1]) * extent for i in range(8)) + check(read(shared) == expected, "concurrent disjoint writes overlapped or vanished") + + +TESTS = [ + ("creation and open flags", test_create_flags), + ("positioned and vectored I/O", test_positioned_and_vectored_io), + ("truncate, growth, sparse ranges, and EOF", test_resize_and_sparse_io), + ("mmap and zero-copy copy paths", test_mmap_and_copy_paths), + ("namespace operations and POSIX errors", test_namespace_and_errors), + ("open-handle lifetime across unlink and rename", test_open_lifetime), + ("names, lookup, stat, and enumeration", test_names_and_enumeration), + ("independent and shared-file concurrency", test_concurrency), +] + + +def run_contract(parent: Path, label: str) -> tuple[Path, str]: + root = parent / f"pdfs-acceptance-{uuid.uuid4().hex}" + root.mkdir() + print(f"[target] {label}: {parent}") + try: + for name, operation in TESTS: + print(f" [test] {name}") + operation(root) + digest = hashlib.sha256(read(root / "positioned.bin")).hexdigest() + return root, digest + except BaseException: + shutil.rmtree(root, ignore_errors=True) + raise + + +def wait_for_copy(root: Path, relative: Path, digest: str, timeout: int) -> None: + deadline = time.monotonic() + timeout + target = root / relative + while time.monotonic() < deadline: + try: + if target.is_file() and hashlib.sha256(read(target)).hexdigest() == digest: + return + except OSError: + pass + time.sleep(2) + raise AssertionError(f"{target} did not converge byte-for-byte within {timeout}s") + + +class ManagedSyncPair: + """Two sync registrations created by this run and removed on exit.""" + + def __init__(self, paths: list[Path], timeout: int) -> None: + self.paths = [path.resolve() for path in paths] + self.timeout = timeout + self.pdfs = os.environ.get("PDFS_ACCEPTANCE_PDFS", "pdfs") + self.ids: dict[Path, int] = {} + self.sentinels = { + path: hashlib.sha256(f"pdfs-preservation:{path}:{uuid.uuid4()}".encode()).digest() + * 4096 + for path in self.paths + } + + def command(self, *args: str, json_output: bool = False) -> str: + command = [self.pdfs] + if json_output: + command.append("--json") + command.extend(args) + result = subprocess.run(command, text=True, capture_output=True) + if result.returncode: + detail = result.stderr.strip() or result.stdout.strip() + raise RuntimeError(f"{' '.join(command)} failed: {detail}") + return result.stdout + + def folders(self) -> list[dict]: + import json + value = json.loads(self.command("sync", "list", json_output=True)) + return value["items"] + + def validate(self) -> None: + if shutil.which(self.pdfs) is None and not Path(self.pdfs).is_file(): + raise RuntimeError( + f"cannot find {self.pdfs!r}; set PDFS_ACCEPTANCE_PDFS to the CLI binary" + ) + 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()} + 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}") + check(path not in registered, f"managed path is already registered: {path}") + check(not is_mountpoint(path), f"managed path is already a mount: {path}") + + def wait_for(self, path: Path, *, mode: str | None = None) -> dict: + deadline = time.monotonic() + self.timeout + last = None + while time.monotonic() < deadline: + for item in self.folders(): + if Path(item["local_path"]).resolve() != path: + continue + last = item + if ( + item["state"] == "idle" + and item.get("pending_mode") is None + and (mode is None or item["mode"] == mode) + ): + return item + if item["state"] in {"error", "conflict"}: + raise RuntimeError(f"sync folder {path} entered {item['state']}: {item}") + time.sleep(2) + raise TimeoutError(f"sync folder {path} did not become idle in mode {mode}: {last}") + + def create(self) -> None: + self.validate() + for path in self.paths: + print(f"[setup] registering empty sync folder {path}") + self.command("sync", "add", str(path)) + item = self.wait_for(path, mode="mirror") + self.ids[path] = int(item["id"]) + for path in self.paths: + write_durable(path / "pdfs-mode-preservation.bin", self.sentinels[path]) + self.force_sync(path) + + def force_sync(self, path: Path) -> None: + before = int(self.wait_for(path)["last_sync"]) + # last_sync has one-second resolution. Ensure this requested pass cannot + # finish with the same timestamp and look indistinguishable from no pass. + while int(time.time()) <= before: + time.sleep(0.1) + self.command("sync", "now", str(self.ids[path])) + deadline = time.monotonic() + self.timeout + last = None + while time.monotonic() < deadline: + last = next( + (item for item in self.folders() if Path(item["local_path"]).resolve() == path), + None, + ) + if last and last["state"] == "idle" and int(last["last_sync"]) > before: + return + if last and last["state"] in {"error", "conflict"}: + raise RuntimeError(f"forced sync for {path} entered {last['state']}: {last}") + time.sleep(1) + raise TimeoutError(f"forced sync for {path} did not complete: {last}") + + def set_modes(self, modes: tuple[str, str]) -> None: + 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) + for path, mode in zip(self.paths, modes, strict=True): + self.wait_for(path, mode=mode) + mounted = is_mountpoint(path) + check(mounted == (mode == "ondemand"), f"{path}: mode is {mode}, mounted={mounted}") + check( + read(path / "pdfs-mode-preservation.bin") == self.sentinels[path], + f"{path}: preservation sentinel changed or vanished after switch to {mode}", + ) + + def cleanup(self) -> None: + # An async add can create its row and then time out before create() has + # recorded the id. validate() proved these exact paths were unregistered + # at entry, so rows now at those paths belong to this run. + try: + for item in self.folders(): + path = Path(item["local_path"]).resolve() + if path in self.paths: + self.ids[path] = int(item["id"]) + except Exception as error: + print(f"WARNING: could not rediscover managed sync folders: {error}") + for path, folder_id in reversed(list(self.ids.items())): + try: + print(f"[cleanup] removing test sync folder {path}") + self.command("sync", "rm", str(folder_id), "--delete-remote") + except Exception as error: + print(f"WARNING: cleanup failed for sync folder {folder_id}: {error}") + + +def is_mountpoint(path: Path) -> bool: + """True for a distinct mount, including FUSE mounts over an existing dir.""" + return os.path.ismount(path) + + +def run_managed_matrix(paths: list[Path]) -> None: + timeout = int(os.environ.get("PDFS_ACCEPTANCE_SYNC_TIMEOUT", "180")) + pair = ManagedSyncPair(paths, timeout) + roots: list[Path] = [] + try: + pair.create() + matrices = [ + ("ondemand", "ondemand"), + ("ondemand", "mirror"), + ("mirror", "ondemand"), + ("mirror", "mirror"), + ] + for modes in matrices: + print(f"[matrix] first={modes[0]}, second={modes[1]}") + pair.set_modes(modes) + for path, mode in zip(pair.paths, modes, strict=True): + root, _ = run_contract(path, f"managed {mode}") + roots.append(root) + shutil.rmtree(root) + roots.remove(root) + # Mirror changes are asynchronous; force and await a pass before + # changing its mode so authored bytes/deletions cannot be lost. + if mode == "mirror": + pair.force_sync(path) + finally: + for root in roots: + shutil.rmtree(root, ignore_errors=True) + pair.cleanup() + + +def main() -> None: + parser = argparse.ArgumentParser(description=__doc__) + live_mode = parser.add_mutually_exclusive_group() + live_mode.add_argument("--live", nargs="+", type=Path, metavar="MOUNTPOINT") + live_mode.add_argument( + "--managed-live", + nargs=2, + type=Path, + metavar=("EMPTY_DIR_A", "EMPTY_DIR_B"), + ) + args = parser.parse_args() + + with tempfile.TemporaryDirectory(prefix="pdfs-api-reference-") as directory: + reference, _ = run_contract(Path(directory), "local filesystem API reference (no account)") + shutil.rmtree(reference) + print("[pass] account-free filesystem API contract") + + if args.managed_live: + run_managed_matrix(args.managed_live) + print("PASS: offline API and managed live mode-matrix suites completed") + return + + if not args.live: + print("PASS: offline acceptance suite completed; live FUSE test was not requested") + return + + completed: list[tuple[Path, Path, str]] = [] + convergence = os.environ.get("PDFS_ACCEPTANCE_CONVERGENCE", "0") == "1" + try: + # Views of one remote folder must not independently create the same + # fixtures. Exercise the primary, then observe that exact tree through + # every secondary. Without convergence mode, each mount is independent + # and receives the full contract. + targets = args.live[:1] if convergence and len(args.live) > 1 else args.live + for mountpoint in targets: + root, digest = run_contract(mountpoint.resolve(), "live FUSE") + completed.append((mountpoint.resolve(), root, digest)) + if convergence and len(args.live) > 1: + print(" [test] cross-mount remote convergence") + timeout = int(os.environ.get("PDFS_ACCEPTANCE_SYNC_TIMEOUT", "120")) + source_root, digest = completed[0][1], completed[0][2] + relative = source_root.relative_to(completed[0][0]) / "positioned.bin" + for mountpoint in args.live[1:]: + wait_for_copy(mountpoint.resolve(), relative, digest, timeout) + finally: + for _, root, _ in completed: + shutil.rmtree(root, ignore_errors=True) + print("PASS: offline API and live FUSE acceptance suites completed") + + +if __name__ == "__main__": + main() diff --git a/scripts/fuse-acceptance.sh b/scripts/fuse-acceptance.sh index 2c486a1..dfa8543 100755 --- a/scripts/fuse-acceptance.sh +++ b/scripts/fuse-acceptance.sh @@ -1,210 +1,47 @@ #!/usr/bin/env bash -# Destructive only inside a uniquely named directory created by this script. +# Filesystem syscall contract, with optional tests against mounted Drive views. set -Eeuo pipefail usage() { - echo "usage: $0 MOUNTPOINT [MOUNTPOINT ...]" >&2 - echo "example: $0 /mnt/on-demand-testmount /mnt/testmount" >&2 + cat >&2 <<'EOF' +usage: scripts/fuse-acceptance.sh [--offline-only] + scripts/fuse-acceptance.sh --live MOUNTPOINT [MOUNTPOINT ...] + scripts/fuse-acceptance.sh --managed-live EMPTY_DIR EMPTY_DIR + scripts/fuse-acceptance.sh MOUNTPOINT [MOUNTPOINT ...] # compatibility + +The local reference suite always runs first and requires no account. --live then +runs the same contract on each mount. Set PDFS_ACCEPTANCE_CONVERGENCE=1 only +when all live mountpoints show the same remote folder. +EOF exit 2 } -(( $# > 0 )) || usage command -v python3 >/dev/null || { echo "python3 is required" >&2; exit 2; } -command -v findmnt >/dev/null || { echo "findmnt is required" >&2; exit 2; } +script_dir="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)" -mounts=("$@") -run_id="pdfs-acceptance-$(date +%Y%m%dT%H%M%S)-$$" -roots=() +case "${1:-}" in + -h|--help) usage ;; + --offline-only|"") + (( $# <= 1 )) || usage + exec python3 "$script_dir/fuse-acceptance.py" + ;; + --live) shift ; (( $# > 0 )) || usage ;; + --managed-live) + shift + (( $# == 2 )) || usage + exec python3 "$script_dir/fuse-acceptance.py" --managed-live "$@" + ;; + --*) usage ;; +esac -cleanup() { - local root - for root in "${roots[@]}"; do - [[ -n "$root" && "$root" == */pdfs-acceptance-* ]] || continue - rm -rf -- "$root" 2>/dev/null || true - done -} -trap cleanup EXIT INT TERM - -for index in "${!mounts[@]}"; do - mountpoint="${mounts[$index]}" +command -v findmnt >/dev/null || { echo "findmnt is required for --live" >&2; exit 2; } +for mountpoint in "$@"; do [[ -d "$mountpoint" ]] || { echo "not a directory: $mountpoint" >&2; exit 2; } - fs_type="$(findmnt -T "$mountpoint" -n -o FSTYPE | head -n 1)" - if (( index == 0 )) && [[ "$fs_type" != fuse* ]]; then - echo "refusing primary non-FUSE path $mountpoint (type: ${fs_type:-unknown})" >&2 - exit 2 - fi - roots+=("${mountpoint%/}/$run_id") done - -step() { printf '[test] %s\n' "$1"; } -assert_text() { - python3 - "$1" "$2" <<'PY' -import sys -p, expected = sys.argv[1], sys.argv[2].encode() -with open(p, "rb") as f: got = f.read() -if got != expected: - raise AssertionError( - f"{p}: expected {expected!r} ({len(expected)} bytes), " - f"got {got!r} ({len(got)} bytes)" - ) -PY +fstype="$(findmnt -T "$1" -n -o FSTYPE | head -n 1)" +[[ "$fstype" == fuse* ]] || { + echo "refusing primary non-FUSE path $1 (type: ${fstype:-unknown})" >&2 + exit 2 } -run_suite() { -local primary="$1" -printf '[mount] testing %s\n' "${primary%/*}" -mkdir -- "$primary" - -step "create, read, overwrite, truncate" -printf 'alpha\n' > "$primary/plain.txt" -assert_text "$primary/plain.txt" $'alpha\n' -printf 'a much longer replacement\n' > "$primary/plain.txt" -truncate -s 4 "$primary/plain.txt" -assert_text "$primary/plain.txt" "a mu" - -step "binary round-trip and random-access writes" -python3 - "$primary/binary.dat" <<'PY' -import hashlib, sys -p = sys.argv[1] -data = bytes(range(256)) * 16384 -with open(p, "wb") as f: f.write(data) -with open(p, "r+b", buffering=0) as f: - f.seek(4093); f.write(b"boundary-write") -with open(p, "rb") as f: got = f.read() -expected = data[:4093] + b"boundary-write" + data[4107:] -assert got == expected -print(hashlib.sha256(got).hexdigest()) -PY - -step "media-style header and distant seek reads" -python3 - "$primary/seek-sample.mkv" <<'PY' -import os, sys - -p = sys.argv[1] -size = 12 * 1024 * 1024 + 137 -header = b"\x1aE\xdf\xa3pdfs-media-header" -middle = b"pdfs-middle-cue" -trailer = b"pdfs-media-trailer" -middle_at = 7 * 1024 * 1024 + 19 -trailer_at = size - len(trailer) - -# A player probes the container header, jumps to cues/index data, then commonly -# seeks near EOF. Write a sparse fixture so the acceptance test stays cheap, -# and issue separate unbuffered reads so FUSE receives genuinely distant ranges -# rather than one accidental sequential read. -with open(p, "wb", buffering=0) as f: - f.truncate(size) - f.seek(0); f.write(header) - f.seek(middle_at); f.write(middle) - f.seek(trailer_at); f.write(trailer) - -with open(p, "rb", buffering=0) as f: - assert f.read(len(header)) == header - f.seek(middle_at); assert f.read(len(middle)) == middle - f.seek(trailer_at); assert f.read(len(trailer)) == trailer - assert f.read(1) == b"" - f.seek(size + 4096); assert f.read(16) == b"" -assert os.stat(p).st_size == size -PY - -step "rename, move, and replacement semantics" -mkdir -- "$primary/dir-a" "$primary/dir-b" -printf 'source' > "$primary/source" -printf 'victim' > "$primary/victim" -mv -- "$primary/source" "$primary/victim" -assert_text "$primary/victim" "source" -[[ ! -e "$primary/source" ]] -mv -- "$primary/victim" "$primary/dir-a/moved" -assert_text "$primary/dir-a/moved" "source" - -step "POSIX error behavior" -if rmdir -- "$primary/dir-a/moved" 2>/dev/null; then echo "rmdir removed a file" >&2; exit 1; fi -if rm -- "$primary/dir-a" 2>/dev/null; then echo "rm removed a directory" >&2; exit 1; fi -if mv -- "$primary/dir-a" "$primary/dir-a/child" 2>/dev/null; then - echo "moving a directory below itself succeeded" >&2; exit 1 -fi - -step "open file remains readable after unlink" -python3 - "$primary/open-unlink" <<'PY' -import os, sys -p = sys.argv[1]; data = b"held-open\0" * 4096 -with open(p, "wb") as f: f.write(data) -f = open(p, "rb", buffering=0); os.unlink(p) -assert not os.path.exists(p) and f.read() == data -f.close() -PY - -step "concurrent independent writers" -for i in $(seq 1 12); do - (python3 - "$primary/concurrent-$i" "$i" <<'PY' -import sys -p, seed = sys.argv[1], int(sys.argv[2]); data = bytes([seed]) * (131072 + seed) -with open(p, "wb") as f: f.write(data) -with open(p, "rb") as f: assert f.read() == data -PY - ) & -done -wait - -step "directory enumeration agrees with stat" -python3 - "$primary" <<'PY' -import os, sys -for name in os.listdir(sys.argv[1]): os.lstat(os.path.join(sys.argv[1], name)) -PY -} - -for root in "${roots[@]}"; do - run_suite "$root" -done - -if [[ "${PDFS_ACCEPTANCE_CONVERGENCE:-0}" == 1 ]] && (( ${#roots[@]} > 1 )); then - step "cross-mount convergence" - primary="${roots[0]}" - expected="$(sha256sum "$primary/binary.dat" | cut -d' ' -f1)" - for secondary in "${roots[@]:1}"; do - deadline=$((SECONDS + ${PDFS_ACCEPTANCE_SYNC_TIMEOUT:-120})) - while (( SECONDS < deadline )); do - if [[ -f "$secondary/binary.dat" ]] && - [[ "$(sha256sum "$secondary/binary.dat" 2>/dev/null | cut -d' ' -f1)" == "$expected" ]]; then break; fi - sleep 2 - done - [[ -f "$secondary/binary.dat" ]] || { echo "did not appear on $secondary" >&2; exit 1; } - [[ "$(sha256sum "$secondary/binary.dat" | cut -d' ' -f1)" == "$expected" ]] || { - echo "content mismatch on $secondary" >&2; exit 1 - } - - # The media fixture may drain after binary.dat. Wait independently, - # then probe only its sparse ranges on the other mount. Unlike the - # same-mount check above, this exercises remote FUSE hydration and seek - # reads instead of serving the still-local staged file. - deadline=$((SECONDS + ${PDFS_ACCEPTANCE_SYNC_TIMEOUT:-120})) - while (( SECONDS < deadline )); do - [[ -f "$secondary/seek-sample.mkv" ]] && - [[ "$(stat -c %s "$secondary/seek-sample.mkv" 2>/dev/null)" == 12583049 ]] && break - sleep 2 - done - [[ -f "$secondary/seek-sample.mkv" ]] || { - echo "media fixture did not appear on $secondary" >&2; exit 1 - } - python3 - "$secondary/seek-sample.mkv" <<'PY' -import os, sys -p = sys.argv[1] -size = 12 * 1024 * 1024 + 137 -checks = [ - (0, b"\x1aE\xdf\xa3pdfs-media-header"), - (7 * 1024 * 1024 + 19, b"pdfs-middle-cue"), - (size - len(b"pdfs-media-trailer"), b"pdfs-media-trailer"), -] -assert os.stat(p).st_size == size -with open(p, "rb", buffering=0) as f: - for offset, expected in checks: - f.seek(offset) - assert f.read(len(expected)) == expected -PY - done -fi - -step "cleanup" -cleanup -roots=() -echo "PASS: FUSE acceptance suite completed ($run_id)" +exec python3 "$script_dir/fuse-acceptance.py" --live "$@" From d0b49d3c8cefa5390890dd172ff65f7484a999d4 Mon Sep 17 00:00:00 2001 From: Nils Pukropp Date: Wed, 22 Jul 2026 11:50:58 +0200 Subject: [PATCH 2/3] fixed a truncate defect --- crates/pdfs-fuse/src/lib.rs | 143 +++++++++++++++++++++++++++++++++--- 1 file changed, 134 insertions(+), 9 deletions(-) diff --git a/crates/pdfs-fuse/src/lib.rs b/crates/pdfs-fuse/src/lib.rs index 2237ffd..407cbb4 100644 --- a/crates/pdfs-fuse/src/lib.rs +++ b/crates/pdfs-fuse/src/lib.rs @@ -1300,6 +1300,21 @@ impl Core { if size == base_size { return Ok(()); } + // A close immediately followed by truncate is common (`write`, close, + // `truncate(2)`) and usually beats the write-back debounce. In that + // case the remote is *not* our base: the complete staged revision is. + // Compose from those local bytes so the truncate can supersede the + // queued upload without fetching stale remote content or returning EIO. + let pending_base = self.pending.lock().get(&uid).cloned(); + if pending_base + .as_ref() + .is_some_and(|pending| !pending.meta.complete) + { + // An incomplete pending blob still has holes referring to the + // remote revision. Stacking another incomplete transform on it is + // not representable safely; preserve the existing refusal. + return Err(Errno::EIO); + } let (authored, complete) = if size == 0 { // An empty file has no content to be missing. (Vec::new(), true) @@ -1316,19 +1331,45 @@ impl Core { error!(%uid, error = %e, "create scratch file for truncate failed"); Errno::EIO })?; + if let Some(pending) = &pending_base + && let Err(e) = copy_pending_for_truncate(pending, &file) + { + error!(%uid, source = %pending.path.display(), error = %e, + "copy pending revision for truncate failed"); + let _ = std::fs::remove_file(&path); + return Err(Errno::EIO); + } file.set_len(size).map_err(|e| { error!(%uid, error = %e, "resize scratch file for truncate failed"); let _ = std::fs::remove_file(&path); Errno::EIO })?; - let meta = StagedWrite { - uid: uid.to_string(), - len: size, - base_size, - base_mtime, - authored, - complete, - based_on: self.remote_baseline(&uid, base_mtime, base_size), + let meta = match pending_base { + Some(pending) => StagedWrite { + uid: uid.to_string(), + len: size, + base_size: pending.meta.len, + base_mtime: pending.meta.base_mtime, + authored: if size == 0 { + Vec::new() + } else { + vec![(0, size)] + }, + complete: true, + // Both local revisions descend from the same last-observed + // remote revision. Never replace this with the optimistic + // mtime/size currently published by State. + based_on: pending.meta.based_on, + }, + None => StagedWrite { + uid: uid.to_string(), + len: size, + base_size, + base_mtime, + authored, + complete, + based_on: self.remote_baseline(&uid, base_mtime, base_size), + }, }; self.enqueue_staged_write(&uid, ino, &path, meta)?; debug!(%uid, size, complete, "queued truncate"); @@ -2861,6 +2902,27 @@ fn count_noun(n: usize, one: &str, many: &str) -> String { format!("{n} {}", if n == 1 { one } else { many }) } +/// Seed a truncate scratch file from a complete queued revision. +/// +/// `File` writes use an explicit offset so this is independent of either +/// descriptor's cursor and remains correct when the destination came from the +/// cache's scratch allocator. +fn copy_pending_for_truncate(pending: &PendingRevision, destination: &File) -> std::io::Result<()> { + let source = File::open(&pending.path)?; + destination.set_len(0)?; + let mut offset = 0u64; + let mut buffer = vec![0u8; 1024 * 1024]; + loop { + let count = source.read_at(&mut buffer, offset)?; + if count == 0 { + break; + } + destination.write_all_at(&buffer[..count], offset)?; + offset += count as u64; + } + destination.set_len(offset) +} + /// 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 { @@ -3224,7 +3286,10 @@ mod pending_size_tests { #[cfg(test)] mod tests { - use super::{Intervals, conflict_name, is_stale_mount}; + use super::{ + Intervals, PendingRevision, conflict_name, copy_pending_for_truncate, is_stale_mount, + }; + use pdfs_core::cache::{Baseline, StagedWrite}; /// The predicate must answer *only* for a dead FUSE connection. A healthy /// directory and an absent path are both "not stale" — widening it to any @@ -3331,6 +3396,66 @@ mod tests { assert_eq!(segs(&iv, 0, 30), vec![(0, 30, true)]); } + fn pending_revision(dir: &TestDir, bytes: &[u8], complete: bool) -> PendingRevision { + let path = dir.0.join("pending"); + std::fs::write(&path, bytes).unwrap(); + PendingRevision { + path, + meta: StagedWrite { + uid: "vol~file".into(), + len: bytes.len() as u64, + base_size: 3, + base_mtime: 42, + authored: if complete { + vec![(0, bytes.len() as u64)] + } else { + vec![(3, bytes.len() as u64)] + }, + complete, + based_on: Some(Baseline { + mtime: 42, + size: 3, + hash: None, + }), + }, + } + } + + #[test] + fn complete_pending_revision_can_seed_immediate_shrink() { + let (_, dir) = state_test_helper(); + let pending = pending_revision(&dir, b"0123456789", true); + let destination_path = dir.0.join("shrink"); + let destination = std::fs::File::create(&destination_path).unwrap(); + copy_pending_for_truncate(&pending, &destination).unwrap(); + destination.set_len(4).unwrap(); + assert_eq!(std::fs::read(destination_path).unwrap(), b"0123"); + } + + #[test] + fn complete_pending_revision_can_seed_immediate_growth_and_zero() { + let (_, dir) = state_test_helper(); + let pending = pending_revision(&dir, b"0123456789", true); + let destination_path = dir.0.join("grow"); + let destination = std::fs::File::create(&destination_path).unwrap(); + copy_pending_for_truncate(&pending, &destination).unwrap(); + destination.set_len(14).unwrap(); + assert_eq!( + std::fs::read(&destination_path).unwrap(), + b"0123456789\0\0\0\0" + ); + destination.set_len(0).unwrap(); + assert!(std::fs::read(destination_path).unwrap().is_empty()); + } + + #[test] + fn incomplete_pending_revision_is_not_a_safe_truncate_base() { + let (_, dir) = state_test_helper(); + let pending = pending_revision(&dir, b"\0\0\0authored", false); + assert!(!pending.meta.complete); + assert_ne!(pending.meta.authored, vec![(0, pending.meta.len)]); + } + use proton_drive_rs::proton_sdk::ids::{LinkId, NodeUid, VolumeId}; use proton_drive_rs::{Node, NodeKind}; From a76f36bdff757148d2bc271e04d1b4d8bbc7f106 Mon Sep 17 00:00:00 2001 From: Nils Pukropp Date: Wed, 22 Jul 2026 14:24:40 +0200 Subject: [PATCH 3/3] fixed fs bugs + compleance test for fuse fs --- crates/pdfs-fuse/src/filesystem.rs | 140 ++++++++++++++++++------- crates/pdfs-fuse/src/lib.rs | 110 +++++++++++++++++++- crates/pdfs-fuse/src/mount.rs | 1 + crates/pdfs-fuse/src/state.rs | 15 ++- docs/BUGS.md | 161 +++++++++++++++++++++++++++-- docs/TESTING.md | 4 + scripts/fuse-acceptance.py | 82 ++++++++++++++- scripts/fuse-acceptance.sh | 6 +- 8 files changed, 459 insertions(+), 60 deletions(-) diff --git a/crates/pdfs-fuse/src/filesystem.rs b/crates/pdfs-fuse/src/filesystem.rs index 3a7990b..8ad17dd 100644 --- a/crates/pdfs-fuse/src/filesystem.rs +++ b/crates/pdfs-fuse/src/filesystem.rs @@ -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); diff --git a/crates/pdfs-fuse/src/lib.rs b/crates/pdfs-fuse/src/lib.rs index 407cbb4..e373d22 100644 --- a/crates/pdfs-fuse/src/lib.rs +++ b/crates/pdfs-fuse/src/lib.rs @@ -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>>, + /// 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>>, /// 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, 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 = 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 { + 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) -> 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}; diff --git a/crates/pdfs-fuse/src/mount.rs b/crates/pdfs-fuse/src/mount.rs index 7494a44..0e4cec8 100644 --- a/crates/pdfs-fuse/src/mount.rs +++ b/crates/pdfs-fuse/src/mount.rs @@ -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)), diff --git a/crates/pdfs-fuse/src/state.rs b/crates/pdfs-fuse/src/state.rs index d7b0b6a..681919a 100644 --- a/crates/pdfs-fuse/src/state.rs +++ b/crates/pdfs-fuse/src/state.rs @@ -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"); } } diff --git a/docs/BUGS.md b/docs/BUGS.md index 7eaf073..a0edc33 100644 --- a/docs/BUGS.md +++ b/docs/BUGS.md @@ -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(/<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. diff --git a/docs/TESTING.md b/docs/TESTING.md index 2961104..f1df3c1 100644 --- a/docs/TESTING.md +++ b/docs/TESTING.md @@ -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: diff --git a/scripts/fuse-acceptance.py b/scripts/fuse-acceptance.py index 57706d5..b7a0fc8 100755 --- a/scripts/fuse-acceptance.py +++ b/scripts/fuse-acceptance.py @@ -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": diff --git a/scripts/fuse-acceptance.sh b/scripts/fuse-acceptance.sh index dfa8543..6c5409a 100755 --- a/scripts/fuse-acceptance.sh +++ b/scripts/fuse-acceptance.sh @@ -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 "$@"