diff --git a/agent/proxy/daemon.go b/agent/proxy/daemon.go index e1ec2e1b0..15950c196 100644 --- a/agent/proxy/daemon.go +++ b/agent/proxy/daemon.go @@ -342,9 +342,11 @@ func (p *Daemon) UnmarshalSnapshot(m map[string]interface{}) error { return err } - // TODO(mitchellh): we should check if proc refers to a process that - // is currently alive. If not, we should return here and not manage the - // process. + // FindProcess on many systems returns no error even if the process + // is now dead. We perform an extra check that the process is alive. + if err := processAlive(proc); err != nil { + return err + } // "Start it" stopCh := make(chan struct{}) diff --git a/agent/proxy/daemon_test.go b/agent/proxy/daemon_test.go index 6e74cdf88..a96e90699 100644 --- a/agent/proxy/daemon_test.go +++ b/agent/proxy/daemon_test.go @@ -372,6 +372,7 @@ func TestDaemonUnmarshalSnapshot(t *testing.T) { ProxyToken: uuid, Logger: testLogger, } + defer d.Stop() require.NoError(d.Start()) // Wait for the file to exist @@ -408,3 +409,43 @@ func TestDaemonUnmarshalSnapshot(t *testing.T) { r.Fatalf("should not exist: %s", err) }) } + +func TestDaemonUnmarshalSnapshot_notRunning(t *testing.T) { + t.Parallel() + + require := require.New(t) + td, closer := testTempDir(t) + defer closer() + + path := filepath.Join(td, "file") + uuid, err := uuid.GenerateUUID() + require.NoError(err) + + d := &Daemon{ + Command: helperProcess("start-stop", path), + ProxyToken: uuid, + Logger: testLogger, + } + defer d.Stop() + require.NoError(d.Start()) + + // Wait for the file to exist + retry.Run(t, func(r *retry.R) { + _, err := os.Stat(path) + if err == nil { + return + } + + r.Fatalf("error: %s", err) + }) + + // Snapshot + snap := d.MarshalSnapshot() + + // Stop the original daemon + require.NoError(d.Stop()) + + // Restore the second daemon + d2 := &Daemon{Logger: testLogger} + require.Error(d2.UnmarshalSnapshot(snap)) +} diff --git a/agent/proxy/process_unix.go b/agent/proxy/process_unix.go new file mode 100644 index 000000000..6db64c59c --- /dev/null +++ b/agent/proxy/process_unix.go @@ -0,0 +1,23 @@ +// +build !windows + +package proxy + +import ( + "os" + "syscall" +) + +// processAlive for non-Windows. Note that this very likely doesn't +// work for all non-Windows platforms Go supports and we should expand +// support as we experience it. +func processAlive(p *os.Process) error { + // On Unix-like systems, we can verify a process is alive by sending + // a 0 signal. This will do nothing to the process but will still + // return errors if the process is gone. + err := p.Signal(syscall.Signal(0)) + if err == nil || err == syscall.EPERM { + return nil + } + + return err +} diff --git a/agent/proxy/process_windows.go b/agent/proxy/process_windows.go new file mode 100644 index 000000000..0268958e9 --- /dev/null +++ b/agent/proxy/process_windows.go @@ -0,0 +1,19 @@ +// +build windows + +package proxy + +import ( + "fmt" + "os" +) + +func processAlive(p *os.Process) error { + // On Windows, os.FindProcess will error if the process is not alive, + // so we don't have to do any further checking. The nature of it being + // non-nil means it seems to be healthy. + if p == nil { + return fmt.Errof("process no longer alive") + } + + return nil +}