diff --git a/DECISIONS.md b/DECISIONS.md index f6541aa..ef96091 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -1,5 +1,45 @@ # Decisions +## 2026-05-04 – Detect mounts via `/proc/self/mountinfo` instead of `stat` +- **Who**: Dom, ClaudeCode +- **Why**: `mountinfo.Mounted(path)` from `github.com/moby/sys/mountinfo` works + by `lstat`-ing the path. When a sshfs connection dies, the FUSE endpoint + stays in `/proc/mounts` but every `stat` on the mountpoint returns EIO. + All three sshfsc paths broke as a result: + - `-l` silently filtered the dead mount out (`Mounted` returned an error, + code skipped on `merr != nil`) + - mount path failed with `mkdir … file exists` because `MkdirAll` stats + before creating + - `-u` failed with `lstat … input/output error` before reaching + `fusermount` + Net effect: a stale mount was a dead end — couldn't be listed, couldn't be + unmounted, blocked re-mount. +- **Tradeoffs**: + - Switched mount detection to `mountinfo.GetMounts(SingleEntryFilter(path))`, + which parses `/proc/self/mountinfo` and never touches the path. Robust to + EIO on the mount target. + - `list_mounts` now uses `PrefixFilter(base)` and trims the base prefix. + Direct children only — nested mounts under an alias dir would be skipped + on purpose; sshfsc only manages one level. + - `verify_mount_dir` skips `MkdirAll` when the path is already a mountpoint. + Re-running `sshfsc ` against a stale mount no longer errors on + mkdir; main's existing `is_mounted_at` check then prints "Already mounted" + and returns. The user can `sshfsc -u ` to recover. + - `unmount_sshfs` falls back to `fusermount -uz` (lazy) when `-u` fails. + Lazy unmount detaches the FS from the tree immediately and lets dangling + refs settle later — exactly what stale FUSE needs. Plain `-u` first + keeps the clean path noise-free. +- **How**: + - New helper `is_mounted_at(path)`; replaces `mountinfo.Mounted` at every + call site (`main`, `verify_mount_dir`, `unmount_sshfs`). + - `list_mounts` rewritten around `PrefixFilter`. Trims `base + "/"` and + skips entries containing further separators. + - `run_fusermount(flag, mount)` extracted to keep `-u` and `-uz` calls + tidy. `unmount_sshfs` retries with `-uz` on first-call error. + - New tests: `TestIsMountedAtFalseOnPlainDir`, + `TestIsMountedAtFalseOnMissingPath`. Existing tests stay green — + `mountinfo.GetMounts` returns empty under the test tmpdirs. + ## 2026-05-04 – Add `-l` (list) and `-u` (unmount) flags - **Who**: Dom, ClaudeCode - **Why**: Mounts under `$XDG_RUNTIME_DIR/sshfs/` had to be listed and diff --git a/main.go b/main.go index 70a396c..2963e91 100644 --- a/main.go +++ b/main.go @@ -167,9 +167,9 @@ func main() { fmt.Println("Mount: ", mount) } - chkmount, chkmount_err := mountinfo.Mounted(mount) + chkmount, chkmount_err := is_mounted_at(mount) if chkmount_err != nil { - fmt.Fprintf(os.Stderr, "mountinfo.Mounted() failed with %s\n", chkmount_err) + fmt.Fprintf(os.Stderr, "is_mounted_at() failed with %s\n", chkmount_err) os.Exit(5) } if !chkmount { @@ -258,12 +258,28 @@ func verify_mount_dir(name string) (string, error) { if err != nil { return "", err } + // If the path is already a mountpoint (possibly stale), skip MkdirAll — + // it would stat the target, which fails with EIO on stale FUSE mounts. + if ok, _ := is_mounted_at(mount); ok { + return mount, nil + } if err := os.MkdirAll(mount, 0700); err != nil { return "", fmt.Errorf("create mount dir %q: %w", mount, err) } return mount, nil } +// is_mounted_at reports whether path is a mountpoint per /proc/self/mountinfo. +// Unlike mountinfo.Mounted, this does not stat the path, so stale FUSE mounts +// (whose targets return EIO on stat) are still detected. +func is_mounted_at(path string) (bool, error) { + mounts, err := mountinfo.GetMounts(mountinfo.SingleEntryFilter(path)) + if err != nil { + return false, err + } + return len(mounts) > 0, nil +} + func list_mounts(out io.Writer) error { base, err := mount_base(false) if err != nil { @@ -272,24 +288,28 @@ func list_mounts(out io.Writer) error { } return err } - entries, err := os.ReadDir(base) + mounts, err := mountinfo.GetMounts(mountinfo.PrefixFilter(base)) if err != nil { - return fmt.Errorf("read base %q: %w", base, err) + return fmt.Errorf("read mountinfo: %w", err) } - for _, entry := range entries { - if !entry.IsDir() { + prefix := base + string(os.PathSeparator) + for _, m := range mounts { + rel := strings.TrimPrefix(m.Mountpoint, prefix) + if rel == "" || strings.Contains(rel, string(os.PathSeparator)) { continue } - path := filepath.Join(base, entry.Name()) - ok, merr := mountinfo.Mounted(path) - if merr != nil || !ok { - continue - } - fmt.Fprintln(out, entry.Name()) + fmt.Fprintln(out, rel) } return nil } +func run_fusermount(flag, mount string) error { + cmd := exec.Command("fusermount", flag, mount) + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + return cmd.Run() +} + func unmount_sshfs(alias string) error { if err := validate_ssh_field("alias", alias, rxHostUser); err != nil { return err @@ -305,18 +325,19 @@ func unmount_sshfs(alias string) error { if err != nil { return err } - ok, err := mountinfo.Mounted(mount) + ok, err := is_mounted_at(mount) if err != nil { - return fmt.Errorf("mountinfo.Mounted(%q): %w", mount, err) + return fmt.Errorf("check mount %q: %w", mount, err) } if !ok { return fmt.Errorf("not mounted: %s", alias) } - cmd := exec.Command("fusermount", "-u", mount) - cmd.Stdout = os.Stdout - cmd.Stderr = os.Stderr - if err := cmd.Run(); err != nil { - return fmt.Errorf("fusermount -u %q: %w", mount, err) + if err := run_fusermount("-u", mount); err != nil { + // stale FUSE mounts often need lazy unmount; -u fails with + // "Transport endpoint is not connected" on those. + if err2 := run_fusermount("-uz", mount); err2 != nil { + return fmt.Errorf("fusermount -u %q: %w (lazy retry: %v)", mount, err, err2) + } } if err := os.Remove(mount); err != nil { fmt.Fprintf(os.Stderr, "warning: remove empty mount dir %q: %v\n", mount, err) diff --git a/main_test.go b/main_test.go index 0b90d1b..858aa2e 100644 --- a/main_test.go +++ b/main_test.go @@ -200,3 +200,25 @@ func TestUnmountNotMountedExistingDir(t *testing.T) { t.Fatalf("want not-mounted error, got %v", err) } } + +func TestIsMountedAtFalseOnPlainDir(t *testing.T) { + dir := t.TempDir() + ok, err := is_mounted_at(dir) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if ok { + t.Fatalf("plain tempdir should not be a mountpoint") + } +} + +func TestIsMountedAtFalseOnMissingPath(t *testing.T) { + dir := t.TempDir() + ok, err := is_mounted_at(filepath.Join(dir, "nope")) + if err != nil { + t.Fatalf("unexpected error (no stat should happen): %v", err) + } + if ok { + t.Fatalf("missing path should not be a mountpoint") + } +}