From caf5dd9d5e4fda9fbb752760d8dcc0127d6a7ed6 Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Wed, 29 Nov 2023 15:11:11 +0000 Subject: [PATCH] mount: notice daemon dying much quicker Before this change we waited until until the timeout to check the daemon was alive. Now we check it every 100ms like we do the mount status. This also fixes compiling on all platforms which was broken by the previous change 9bfbf2a4a mount: fix macOS not noticing errors with --daemon See: https://forum.rclone.org/t/rclone-mount-daemon-exits-successfully-even-when-mount-fails/43146 --- cmd/mountlib/check_linux.go | 23 ++----------------- cmd/mountlib/check_other.go | 12 ++-------- cmd/mountlib/mount.go | 42 +++++++++++++++++++++++++++-------- lib/daemonize/daemon_other.go | 12 +++++++--- lib/daemonize/daemon_unix.go | 22 ++++++++++++++++-- 5 files changed, 66 insertions(+), 45 deletions(-) diff --git a/cmd/mountlib/check_linux.go b/cmd/mountlib/check_linux.go index 81e3ccf37..58d839bf5 100644 --- a/cmd/mountlib/check_linux.go +++ b/cmd/mountlib/check_linux.go @@ -7,15 +7,10 @@ import ( "fmt" "path/filepath" "strings" - "time" "github.com/moby/sys/mountinfo" ) -const ( - pollInterval = 100 * time.Millisecond -) - // CheckMountEmpty checks if folder is not already a mountpoint. // On Linux we use the OS-specific /proc/self/mountinfo API so the check won't access the path. // Directories marked as "mounted" by autofs are considered not mounted. @@ -80,19 +75,5 @@ func CheckMountReady(mountpoint string) error { return fmt.Errorf(msg, mountpointAbs) } -// WaitMountReady waits until mountpoint is mounted by rclone. -func WaitMountReady(mountpoint string, timeout time.Duration) (err error) { - endTime := time.Now().Add(timeout) - for { - err = CheckMountReady(mountpoint) - delay := time.Until(endTime) - if err == nil || delay <= 0 { - break - } - if delay > pollInterval { - delay = pollInterval - } - time.Sleep(delay) - } - return -} +// CanCheckMountReady is set if CheckMountReady is functional +var CanCheckMountReady = true diff --git a/cmd/mountlib/check_other.go b/cmd/mountlib/check_other.go index 328f83bca..760be4ec6 100644 --- a/cmd/mountlib/check_other.go +++ b/cmd/mountlib/check_other.go @@ -3,10 +3,6 @@ package mountlib -import ( - "time" -) - // CheckMountEmpty checks if mountpoint folder is empty. // On non-Linux unixes we list directory to ensure that. func CheckMountEmpty(mountpoint string) error { @@ -19,9 +15,5 @@ func CheckMountReady(mountpoint string) error { return nil } -// WaitMountReady should wait until mountpoint is mounted by rclone. -// The check is implemented only for Linux so we just sleep a little. -func WaitMountReady(mountpoint string, timeout time.Duration) error { - time.Sleep(timeout) - return nil -} +// CanCheckMountReady is set if CheckMountReady is functional +var CanCheckMountReady = false diff --git a/cmd/mountlib/mount.go b/cmd/mountlib/mount.go index 558a7b241..a94f4a7dd 100644 --- a/cmd/mountlib/mount.go +++ b/cmd/mountlib/mount.go @@ -10,7 +10,6 @@ import ( "runtime" "strings" "sync" - "syscall" "time" "github.com/rclone/rclone/cmd" @@ -157,6 +156,38 @@ func AddFlags(flagSet *pflag.FlagSet) { flags.DurationVarP(flagSet, &Opt.DaemonWait, "daemon-wait", "", Opt.DaemonWait, "Time to wait for ready mount from daemon (maximum time on Linux, constant sleep time on OSX/BSD) (not supported on Windows)", "Mount") } +const ( + pollInterval = 100 * time.Millisecond +) + +// WaitMountReady waits until mountpoint is mounted by rclone. +// +// If the mount daemon dies prematurely it will notice too. +func WaitMountReady(mountpoint string, timeout time.Duration, daemon *os.Process) (err error) { + endTime := time.Now().Add(timeout) + for { + if CanCheckMountReady { + err = CheckMountReady(mountpoint) + if err == nil { + break + } + } + err = daemonize.Check(daemon) + if err != nil { + return err + } + delay := time.Until(endTime) + if delay <= 0 { + break + } + if delay > pollInterval { + delay = pollInterval + } + time.Sleep(delay) + } + return +} + // NewMountCommand makes a mount command with the given name and Mount function func NewMountCommand(commandName string, hidden bool, mount MountFn) *cobra.Command { var commandDefinition = &cobra.Command{ @@ -220,16 +251,9 @@ func NewMountCommand(commandName string, hidden bool, mount MountFn) *cobra.Comm handle := atexit.Register(func() { killDaemon("Got interrupt") }) - err = WaitMountReady(mnt.MountPoint, Opt.DaemonWait) + err = WaitMountReady(mnt.MountPoint, Opt.DaemonWait, daemon) if err != nil { killDaemon("Daemon timed out") - } else { - // Double check daemon is still alive - // on non Linux OSes WaitMountReady is just a no-op - err = daemon.Signal(syscall.Signal(0)) - if err != nil { - err = fmt.Errorf("daemon has died: %w", err) - } } atexit.Unregister(handle) } diff --git a/lib/daemonize/daemon_other.go b/lib/daemonize/daemon_other.go index a0781d453..a7d663d0e 100644 --- a/lib/daemonize/daemon_other.go +++ b/lib/daemonize/daemon_other.go @@ -1,5 +1,4 @@ -//go:build windows || plan9 || js -// +build windows plan9 js +//go:build !unix // Package daemonize provides daemonization stub for non-Unix platforms. package daemonize @@ -10,7 +9,14 @@ import ( "runtime" ) +var errNotSupported = fmt.Errorf("daemon mode is not supported on the %s platform", runtime.GOOS) + // StartDaemon runs background twin of current process. func StartDaemon(args []string) (*os.Process, error) { - return nil, fmt.Errorf("background mode is not supported on %s platform", runtime.GOOS) + return nil, errNotSupported +} + +// Check returns non nil if the daemon process has died +func Check(daemon *os.Process) error { + return errNotSupported } diff --git a/lib/daemonize/daemon_unix.go b/lib/daemonize/daemon_unix.go index 1fcb2c55c..6e1e0442e 100644 --- a/lib/daemonize/daemon_unix.go +++ b/lib/daemonize/daemon_unix.go @@ -1,15 +1,16 @@ -//go:build !windows && !plan9 && !js -// +build !windows,!plan9,!js +//go:build unix // Package daemonize provides daemonization interface for Unix platforms. package daemonize import ( + "fmt" "os" "strings" "syscall" "github.com/rclone/rclone/fs" + "golang.org/x/sys/unix" ) // StartDaemon runs background twin of current process. @@ -108,3 +109,20 @@ func argsToEnv(origArgs, origEnv []string) (args, env []string) { } return } + +// Check returns non nil if the daemon process has died +func Check(daemon *os.Process) error { + var status unix.WaitStatus + wpid, err := unix.Wait4(daemon.Pid, &status, unix.WNOHANG, nil) + // fs.Debugf(nil, "wait4 returned wpid=%d, err=%v, status=%d", wpid, err, status) + if err != nil { + return err + } + if wpid == 0 { + return nil + } + if status.Exited() { + return fmt.Errorf("daemon exited with error code %d", status.ExitStatus()) + } + return nil +}