mirror of
https://github.com/narrrl/proton-drive-linux.git
synced 2026-11-03 16:41:26 +01:00
fix(core,fuse): never stat a foreign mount from the local-file indexer
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
This commit is contained in:
1 parent
f4d6580cd3
commit
b095b9f6b0
3 files changed
+180
-1
No files matched your search
@@ -199,6 +199,9 @@ fn to_entry(entry: &ignore::DirEntry) -> Option<LocalEntry> {
|
||||
/// 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<PathBuf> {
|
||||
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<PathBuf> {
|
||||
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<PathBuf> {
|
||||
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");
|
||||
}
|
||||
}
|
||||
@@ -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();
|
||||
|
||||
|
||||
@@ -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/<id>/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.
|
||||
|
||||
Reference in new issue
Block a user