From b095b9f6b03713e96bb9ea6d307d4e737db6a8d1 Mon Sep 17 00:00:00 2001 From: Nils Pukropp Date: Wed, 16 Sep 2026 23:05:06 +0200 Subject: [PATCH] fix(core,fuse): never stat a foreign mount from the local-file indexer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "This computer" indexer walks $HOME, and `default_excludes` only kept it out of the Drive mountpoint and our own state/cache dirs. A home directory that also holds third-party FUSE mounts (sshfs, rclone) got walked into. `scan` does ask for `same_file_system(true)`, which looks sufficient and is not: `ignore` implements it as `is_same_file_system` -> `device_num(path)` -> `path.metadata()`, run after the directory is queued. Learning that a mount is foreign requires stat'ing it first, and against a mount whose server is gone that stat never returns. The consequence is worse than a stalled scan. The blocked thread sits in uninterruptible FUSE wait, so it ignores SIGKILL: the daemon can never exit, systemd's stop times out, and the unreapable process keeps the cache.db lock the next start needs, turning one dead mount into a permanent restart loop. No userspace recovery exists once a thread is in that state, so the fix has to be to never enter it. Exclude every mountpoint strictly below the scan root, read by path from /proc/self/mounts (octal-unescaped, so a path with a space still matches). `filter_entry` runs before the device stat, so the walker never touches them. Re-read per scan rather than cached at startup: a mount that appeared since the daemon started is the likeliest to be half-alive. This does not change what is indexed — a nested mount is a different device, so it was already going to be skipped. Only the stat in front of that decision goes away. Refs: docs/BUGS.md B90 --- crates/pdfs-core/src/localindex.rs | 101 +++++++++++++++++++++++++++++ crates/pdfs-fuse/src/background.rs | 7 +- docs/BUGS.md | 73 +++++++++++++++++++++ 3 files changed, 180 insertions(+), 1 deletion(-) diff --git a/crates/pdfs-core/src/localindex.rs b/crates/pdfs-core/src/localindex.rs index 80c477f..8335ea8 100644 --- a/crates/pdfs-core/src/localindex.rs +++ b/crates/pdfs-core/src/localindex.rs @@ -199,6 +199,9 @@ fn to_entry(entry: &ignore::DirEntry) -> Option { /// The paths a scan should never descend into: the Drive mountpoint (walking it /// would fault every remote node in through FUSE) plus our own state and cache /// dirs, which hold blobs no user searches for by name. +/// +/// Callers should extend this with [`nested_mount_points`] for each scan root — +/// see that function for why `same_file_system(true)` is not enough on its own. pub fn default_excludes(mountpoint: &Path, state_dir: &Path, cache_dir: &Path) -> Vec { vec![ mountpoint.to_path_buf(), @@ -207,6 +210,66 @@ pub fn default_excludes(mountpoint: &Path, state_dir: &Path, cache_dir: &Path) - ] } +/// Every mountpoint strictly below `root`, read from `/proc/self/mounts`. +/// +/// [`scan`] already asks the walker to stay on one filesystem, but that check is +/// a *stat of the mountpoint* (`ignore`'s `is_same_file_system`), performed after +/// the directory is queued — so the walker has to touch a foreign mount to learn +/// it should skip it. On a dead network FUSE mount (an sshfs whose server is +/// gone, an unresponsive rclone) that stat blocks in `request_wait_answer` with +/// nobody left to answer it, and blocks *uninterruptibly*: the walker thread then +/// ignores SIGKILL, so the daemon can never exit, systemd's stop times out, and +/// the half-dead process keeps the cache.db lock that the next start needs +/// (docs/BUGS.md B90). +/// +/// Excluding the mountpoints by path avoids that stat entirely. It does not +/// change what ends up indexed: a nested mount is a different device, so +/// `same_file_system(true)` was already going to skip it. +pub fn nested_mount_points(root: &Path) -> Vec { + let Ok(mounts) = std::fs::read_to_string("/proc/self/mounts") else { + return Vec::new(); + }; + parse_nested_mount_points(&mounts, root) +} + +/// The mountpoint-field parse behind [`nested_mount_points`], split out so it can +/// be tested without a real `/proc`. +fn parse_nested_mount_points(mounts: &str, root: &Path) -> Vec { + mounts + .lines() + .filter_map(|line| line.split_whitespace().nth(1)) + .map(|field| PathBuf::from(unescape_mount_field(field))) + // Strictly below: `root` itself being a mountpoint must not exclude the + // whole scan. + .filter(|path| path.starts_with(root) && path != root) + .collect() +} + +/// Undo the octal escaping `/proc/self/mounts` applies to space, tab, newline and +/// backslash in path fields. An un-unescaped `\040` would simply fail to match a +/// real path, silently putting a mount with a space in its name back in the walk. +fn unescape_mount_field(field: &str) -> String { + let mut out = String::with_capacity(field.len()); + let mut rest = field; + while let Some(at) = rest.find('\\') { + out.push_str(&rest[..at]); + let escape = rest[at + 1..].get(..3); + match escape.and_then(|digits| u8::from_str_radix(digits, 8).ok()) { + Some(byte) => { + out.push(byte as char); + rest = &rest[at + 4..]; + } + // Not an octal escape after all — keep the backslash verbatim. + None => { + out.push('\\'); + rest = &rest[at + 1..]; + } + } + } + out.push_str(rest); + out +} + #[cfg(test)] mod tests { use super::*; @@ -268,4 +331,42 @@ mod tests { std::fs::remove_dir_all(&tmp).unwrap(); } + + /// A dead network FUSE mount under the scan root must be skipped by *path*, + /// before anything stats it — see `nested_mount_points`. The scan root itself + /// being a mountpoint must not exclude the entire walk. + #[test] + fn nested_mount_points_are_excluded_but_the_root_mount_is_not() { + let mounts = "\ +/dev/sda2 /home ext4 rw 0 0 +narl@narl.io:/opt /home/narl/remote/narl.io fuse.sshfs rw 0 0 +google_drive: /home/narl/remote/gdrive fuse.rclone rw 0 0 +tmpfs /run/user/1000 tmpfs rw 0 0 +"; + let nested = parse_nested_mount_points(mounts, Path::new("/home/narl")); + assert_eq!( + nested, + vec![ + PathBuf::from("/home/narl/remote/narl.io"), + PathBuf::from("/home/narl/remote/gdrive"), + ] + ); + + // `/home` is an ancestor, not a descendant: excluding it would empty the + // scan. `/run/user/1000` is unrelated. + assert!(parse_nested_mount_points(mounts, Path::new("/home")).len() == 2); + } + + /// `/proc/self/mounts` octal-escapes a space; leaving it escaped would put a + /// mount whose path contains one back into the walk. + #[test] + fn mount_paths_are_unescaped() { + let mounts = "srv:/x /home/narl/My\\040Cloud fuse.sshfs rw 0 0\n"; + assert_eq!( + parse_nested_mount_points(mounts, Path::new("/home/narl")), + vec![PathBuf::from("/home/narl/My Cloud")] + ); + assert_eq!(unescape_mount_field("/a\\134b"), "/a\\b"); + assert_eq!(unescape_mount_field("/plain/path"), "/plain/path"); + } } diff --git a/crates/pdfs-fuse/src/background.rs b/crates/pdfs-fuse/src/background.rs index 57328f0..8b4e829 100644 --- a/crates/pdfs-fuse/src/background.rs +++ b/crates/pdfs-fuse/src/background.rs @@ -487,7 +487,12 @@ fn scan_local_once( } }; - let excludes = localindex::default_excludes(mountpoint, &dirs.state_dir(), &dirs.cache_dir()); + let mut excludes = + localindex::default_excludes(mountpoint, &dirs.state_dir(), &dirs.cache_dir()); + // Resolved per scan rather than once at startup: mounts come and go, and a + // mount that appeared since the daemon started is exactly the one most + // likely to be a half-alive network filesystem (see `nested_mount_points`). + excludes.extend(localindex::nested_mount_points(&home)); indexing.store(true, Ordering::Relaxed); let started = Instant::now(); diff --git a/docs/BUGS.md b/docs/BUGS.md index 53705cb..7801ebe 100644 --- a/docs/BUGS.md +++ b/docs/BUGS.md @@ -12,6 +12,79 @@ Conventions: --- +## B90 — The local-file indexer walks into foreign FUSE mounts, and a dead one makes the daemon unkillable + +**Status:** Fixed (unverified) — the walker no longer stats a nested mountpoint. The stuck-daemon +half cannot be fixed in userspace at all; see "Why there is no second line of defence". +**Found:** 2026-09-16, from a user report of `systemctl --user restart` failing in a loop. +**Where:** `crates/pdfs-core/src/localindex.rs` (`default_excludes`, `scan`), +`crates/pdfs-fuse/src/background.rs` (`scan_local_once`). + +**Symptom.** Stopping the service times out, SIGKILL does not help, and every subsequent start +fails with `another Proton Drive daemon is already using .../cache.db`: + +``` +systemd[2196]: proton-drive.service: State 'final-sigterm' timed out. Killing. +systemd[2196]: proton-drive.service: Killing process 2221 (pdfs) with signal SIGKILL. +systemd[2196]: proton-drive.service: Processes still around after final SIGKILL. Entering failed mode. +systemd[2196]: proton-drive.service: Unit process 2221 (pdfs) remains running after unit stopped. +pdfs[73921]: ERROR mount failed; retrying in 5s error="open cache db: another Proton Drive daemon + is already using /home/narl/.local/state/proton-drive-linux/cache.db" +``` + +**Diagnosis.** The surviving process is a zombie that still has a live thread: + +``` +$ cat /proc/2221/status | grep -E 'State|Threads' +State: Z (zombie) +Threads: 2 +$ cat /proc/2221/task/73231/comm; cat /proc/2221/task/73231/stack +pdfs-localindex +request_wait_answer +``` + +`pdfs-localindex` is parked in a FUSE request nobody will answer, in uninterruptible `D` state, so +it ignores SIGKILL. The thread group leader exits, the thread does not, the process cannot be +reaped — and it keeps the `cache.db` lock, which is what turns one stuck scan into a permanent +restart loop. The same dead connection also blocked an unrelated `duf` in `fuse_statfs` +(`/sys/fs/fuse/connections/69/waiting` was 2). + +**Cause.** `scan_local_once` walks `$HOME`, and `default_excludes` only excluded the Drive +mountpoint plus our own state and cache dirs. The reporter's home also held +`~/remote/narl.io` (sshfs) and `~/remote/gdrive` (rclone) — third-party FUSE mounts the walk +descends into. + +`scan` does set `same_file_system(true)`, which looks like it should already cover this. It does +not: in `ignore` 0.4.33 the check is `is_same_file_system` → `device_num(path)` → +`path.metadata()` (`walk.rs:2164`), i.e. **a stat of the mountpoint**, performed in `run_one` +*after* the directory has been queued. To discover that a mount is foreign, the walker first has +to touch it. Against a network FUSE mount whose server is gone, that stat never returns. + +**Fix.** `localindex::nested_mount_points` reads `/proc/self/mounts` and returns every mountpoint +strictly below the scan root; `scan_local_once` appends those to the excludes. `filter_entry` runs +before the device stat (`walk.rs:1926`, before `send`), so a path-only rejection keeps the walker +from ever touching the mount. Mountpoint fields are octal-unescaped, because a `\040` left in +place would silently fail to match and put the mount back in the walk. Re-read per scan, not +cached at startup: a mount that appeared since the daemon started is the likeliest to be a +half-alive network filesystem. + +This changes nothing about what gets indexed. A nested mount is a different device, so +`same_file_system(true)` was always going to skip it — the exclude only removes the stat that had +to happen first. + +**Why there is no second line of defence.** Once a thread is in uninterruptible FUSE wait, no +userspace change rescues it: not a join timeout, not `std::process::exit`, not SIGKILL. The kernel +will not reap the task until the syscall returns, which needs someone to answer the request or +abort the connection (`echo 1 > /sys/fs/fuse/connections//abort`, root-only). Avoiding entry is +the entire fix. That makes any *new* code path that stats a user-supplied or mount-crossing path +from a daemon thread a repeat of this bug — mirror-folder scanning is the obvious next candidate. + +**Recovery for an already-wedged daemon.** As root, abort the connection with waiters +(`grep . /sys/fs/fuse/connections/*/waiting` finds it), which releases the thread, lets the zombie +be reaped and frees the `cache.db` lock. A reboot also clears it. + +--- + ## B89 — The recorded SDK dependency is three minor versions out of date in the working memory **Status:** Open — documentation only, but it misdirects every reader of the SDK code.