From b578586d4a8c84bc5c5f8fdbb21c674313784d03 Mon Sep 17 00:00:00 2001 From: Daniel Nelson Date: Thu, 22 Aug 2019 10:50:02 -0700 Subject: [PATCH] Send TERM to exec processes before sending KILL signal (#6302) --- internal/exec.go | 30 +++++++++++++++++++++ internal/exec_unix.go | 58 ++++++++++++++++++++++++++++++++++++++++ internal/exec_windows.go | 41 ++++++++++++++++++++++++++++ internal/internal.go | 49 --------------------------------- 4 files changed, 129 insertions(+), 49 deletions(-) create mode 100644 internal/exec.go create mode 100644 internal/exec_unix.go create mode 100644 internal/exec_windows.go diff --git a/internal/exec.go b/internal/exec.go new file mode 100644 index 000000000..795822f46 --- /dev/null +++ b/internal/exec.go @@ -0,0 +1,30 @@ +package internal + +import ( + "bytes" + "os/exec" + "time" +) + +// CombinedOutputTimeout runs the given command with the given timeout and +// returns the combined output of stdout and stderr. +// If the command times out, it attempts to kill the process. +func CombinedOutputTimeout(c *exec.Cmd, timeout time.Duration) ([]byte, error) { + var b bytes.Buffer + c.Stdout = &b + c.Stderr = &b + if err := c.Start(); err != nil { + return nil, err + } + err := WaitTimeout(c, timeout) + return b.Bytes(), err +} + +// RunTimeout runs the given command with the given timeout. +// If the command times out, it attempts to kill the process. +func RunTimeout(c *exec.Cmd, timeout time.Duration) error { + if err := c.Start(); err != nil { + return err + } + return WaitTimeout(c, timeout) +} diff --git a/internal/exec_unix.go b/internal/exec_unix.go new file mode 100644 index 000000000..d41aae825 --- /dev/null +++ b/internal/exec_unix.go @@ -0,0 +1,58 @@ +// +build !windows + +package internal + +import ( + "log" + "os/exec" + "syscall" + "time" +) + +// KillGrace is the amount of time we allow a process to shutdown before +// sending a SIGKILL. +const KillGrace = 5 * time.Second + +// WaitTimeout waits for the given command to finish with a timeout. +// It assumes the command has already been started. +// If the command times out, it attempts to kill the process. +func WaitTimeout(c *exec.Cmd, timeout time.Duration) error { + var kill *time.Timer + term := time.AfterFunc(timeout, func() { + err := c.Process.Signal(syscall.SIGTERM) + if err != nil { + log.Printf("E! [agent] Error terminating process: %s", err) + return + } + + kill = time.AfterFunc(KillGrace, func() { + err := c.Process.Kill() + if err != nil { + log.Printf("E! [agent] Error killing process: %s", err) + return + } + }) + }) + + err := c.Wait() + + // Shutdown all timers + if kill != nil { + kill.Stop() + } + termSent := !term.Stop() + + // If the process exited without error treat it as success. This allows a + // process to do a clean shutdown on signal. + if err == nil { + return nil + } + + // If SIGTERM was sent then treat any process error as a timeout. + if termSent { + return TimeoutErr + } + + // Otherwise there was an error unrelated to termination. + return err +} diff --git a/internal/exec_windows.go b/internal/exec_windows.go new file mode 100644 index 000000000..f010bdd96 --- /dev/null +++ b/internal/exec_windows.go @@ -0,0 +1,41 @@ +// +build windows + +package internal + +import ( + "log" + "os/exec" + "time" +) + +// WaitTimeout waits for the given command to finish with a timeout. +// It assumes the command has already been started. +// If the command times out, it attempts to kill the process. +func WaitTimeout(c *exec.Cmd, timeout time.Duration) error { + timer := time.AfterFunc(timeout, func() { + err := c.Process.Kill() + if err != nil { + log.Printf("E! [agent] Error killing process: %s", err) + return + } + }) + + err := c.Wait() + + // Shutdown all timers + termSent := !timer.Stop() + + // If the process exited without error treat it as success. This allows a + // process to do a clean shutdown on signal. + if err == nil { + return nil + } + + // If SIGTERM was sent then treat any process error as a timeout. + if termSent { + return TimeoutErr + } + + // Otherwise there was an error unrelated to termination. + return err +} diff --git a/internal/internal.go b/internal/internal.go index 893f34383..13c851a8d 100644 --- a/internal/internal.go +++ b/internal/internal.go @@ -9,7 +9,6 @@ import ( "errors" "fmt" "io" - "log" "math" "math/big" "os" @@ -200,54 +199,6 @@ func SnakeCase(in string) string { return string(out) } -// CombinedOutputTimeout runs the given command with the given timeout and -// returns the combined output of stdout and stderr. -// If the command times out, it attempts to kill the process. -func CombinedOutputTimeout(c *exec.Cmd, timeout time.Duration) ([]byte, error) { - var b bytes.Buffer - c.Stdout = &b - c.Stderr = &b - if err := c.Start(); err != nil { - return nil, err - } - err := WaitTimeout(c, timeout) - return b.Bytes(), err -} - -// RunTimeout runs the given command with the given timeout. -// If the command times out, it attempts to kill the process. -func RunTimeout(c *exec.Cmd, timeout time.Duration) error { - if err := c.Start(); err != nil { - return err - } - return WaitTimeout(c, timeout) -} - -// WaitTimeout waits for the given command to finish with a timeout. -// It assumes the command has already been started. -// If the command times out, it attempts to kill the process. -func WaitTimeout(c *exec.Cmd, timeout time.Duration) error { - timer := time.AfterFunc(timeout, func() { - err := c.Process.Kill() - if err != nil { - log.Printf("E! [agent] Error killing process: %s", err) - return - } - }) - - err := c.Wait() - if err == nil { - timer.Stop() - return nil - } - - if !timer.Stop() { - return TimeoutErr - } - - return err -} - // RandomSleep will sleep for a random amount of time up to max. // If the shutdown channel is closed, it will return before it has finished // sleeping.