Merge pull request #354 from sean-/fix-cmd-leak

Improve Command() handling and signal handling for launched processes.

internal/common/common.go

@@ -9,10 +9,10 @@ package common
 import (
 	"bufio"
 	"bytes"
+	"context"
 	"errors"
 	"fmt"
 	"io/ioutil"
-	"log"
 	"net/url"
 	"os"
 	"os/exec"
@@ -37,8 +37,24 @@ type Invoker interface {
 type Invoke struct{}
 
 func (i Invoke) Command(name string, arg ...string) ([]byte, error) {
-	cmd := exec.Command(name, arg...)
-	return CombinedOutputTimeout(cmd, Timeout)
+	ctxt, cancel := context.WithTimeout(context.Background(), Timeout)
+	defer cancel()
+
+	cmd := exec.CommandContext(ctxt, name, arg...)
+
+	var buf bytes.Buffer
+	cmd.Stdout = &buf
+	cmd.Stderr = &buf
+
+	if err := cmd.Start(); err != nil {
+		return buf.Bytes(), err
+	}
+
+	if err := cmd.Wait(); err != nil {
+		return buf.Bytes(), err
+	}
+
+	return buf.Bytes(), nil
 }
 
 type FakeInvoke struct {
@@ -300,44 +316,6 @@ func HostEtc(combineWith ...string) string {
 	return GetEnv("HOST_ETC", "/etc", combineWith...)
 }
 
-// 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.
-// copied from https://github.com/influxdata/telegraf
-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
-}
-
-// 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.
-// copied from https://github.com/influxdata/telegraf
-func WaitTimeout(c *exec.Cmd, timeout time.Duration) error {
-	timer := time.NewTimer(timeout)
-	done := make(chan error)
-	go func() { done <- c.Wait() }()
-	select {
-	case err := <-done:
-		timer.Stop()
-		return err
-	case <-timer.C:
-		if err := c.Process.Kill(); err != nil {
-			log.Printf("FATAL error killing process: %s", err)
-			return err
-		}
-		// wait for the command to return after killing it
-		<-done
-		return ErrTimeout
-	}
-}
-
 // https://gist.github.com/kylelemons/1525278
 func Pipeline(cmds ...*exec.Cmd) ([]byte, []byte, error) {
 	// Require at least one command

process/process_posix.go

@@ -4,14 +4,11 @@ package process
 
 import (
 	"os"
-	"os/exec"
 	"os/user"
 	"path/filepath"
 	"strconv"
 	"strings"
 	"syscall"
-
-	"github.com/shirou/gopsutil/internal/common"
 )
 
 // POSIX
@@ -66,28 +63,12 @@ func getTerminalMap() (map[uint64]string, error) {
 // SendSignal sends a syscall.Signal to the process.
 // Currently, SIGSTOP, SIGCONT, SIGTERM and SIGKILL are supported.
 func (p *Process) SendSignal(sig syscall.Signal) error {
-	sigAsStr := "INT"
-	switch sig {
-	case syscall.SIGSTOP:
-		sigAsStr = "STOP"
-	case syscall.SIGCONT:
-		sigAsStr = "CONT"
-	case syscall.SIGTERM:
-		sigAsStr = "TERM"
-	case syscall.SIGKILL:
-		sigAsStr = "KILL"
-	}
-
-	kill, err := exec.LookPath("kill")
+	process, err := os.FindProcess(int(p.Pid))
 	if err != nil {
 		return err
 	}
-	cmd := exec.Command(kill, "-s", sigAsStr, strconv.Itoa(int(p.Pid)))
-	cmd.Stderr = os.Stderr
-	if err := cmd.Start(); err != nil {
-		return err
-	}
-	err = common.WaitTimeout(cmd, common.Timeout)
+
+	err = process.Signal(sig)
 	if err != nil {
 		return err
 	}