Merge pull request #583 from mackerelio/pid-check-command-name

Check command name on pid check for pid confliction after OS restart

pidfile/pid.go

@@ -4,3 +4,8 @@ package pidfile
 func ExistsPid(pid int) bool {
 	return existsPid(pid)
 }
+
+// GetCmdName gets the command name of pid
+func GetCmdName(pid int) string {
+	return getCmdName(pid)
+}

pidfile/pid_darwin.go

@@ -1,9 +1,12 @@
 package pidfile
 
 import (
+	"bytes"
 	"fmt"
 	"io/ioutil"
 	"os/exec"
+	"path/filepath"
+	"strings"
 )
 
 func existsPid(pid int) bool {
@@ -14,3 +17,22 @@ func existsPid(pid int) bool {
 	err := cmd.Run()
 	return err == nil
 }
+
+func getCmdName(pid int) string {
+	cmd := exec.Command("/bin/ps", "-o", "command=", fmt.Sprint(pid))
+
+	var stdout bytes.Buffer
+	cmd.Stdout = &stdout
+	cmd.Stderr = ioutil.Discard
+
+	err := cmd.Run()
+	if err != nil {
+		return ""
+	}
+
+	out := stdout.String()
+	if i := strings.IndexRune(out, ' '); i > 0 {
+		out = out[:i]
+	}
+	return filepath.Base(out)
+}

pidfile/pid_test.go

@@ -3,8 +3,10 @@
 package pidfile
 
 import (
+	"context"
 	"math"
 	"os"
+	"os/exec"
 	"testing"
 )
 
@@ -16,3 +18,20 @@ func TestExistsPid(t *testing.T) {
 		t.Errorf("something went wrong")
 	}
 }
+
+func TestGetCmdName(t *testing.T) {
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	cmd := exec.CommandContext(ctx, "sleep", "10")
+	cmd.Stdout = os.Stdout
+	err := cmd.Start()
+	if err != nil {
+		t.Fatal(err)
+	}
+	pid := cmd.Process.Pid
+
+	expected := "sleep"
+	if got := GetCmdName(pid); got != expected {
+		t.Errorf("GetCmdName should return %q bot got: %q", expected, got)
+	}
+}

pidfile/pid_unix.go

@@ -4,10 +4,26 @@ package pidfile
 
 import (
 	"fmt"
+	"io/ioutil"
 	"os"
+	"path/filepath"
+	"strings"
 )
 
 func existsPid(pid int) bool {
 	_, err := os.Stat(fmt.Sprintf("/proc/%d/", pid))
 	return err == nil
 }
+
+func getCmdName(pid int) string {
+	cnt, err := ioutil.ReadFile(fmt.Sprintf("/proc/%d/cmdline", pid))
+	if err != nil {
+		return ""
+	}
+
+	out := string(cnt)
+	if i := strings.IndexRune(out, '\x00'); i > 0 {
+		out = out[:i]
+	}
+	return filepath.Base(out)
+}

pidfile/pid_windows.go

@@ -1,6 +1,11 @@
 package pidfile
 
-func existsPid(_ int) bool {
+func existsPid(int) bool {
 	// XXX not implemented. should use `tasklist` command or so
 	return false
 }
+
+func getCmdName(int) string {
+	// XXX not implemented.
+	return ""
+}

pidfile/pidfile.go

@@ -22,7 +22,7 @@ func Create(pidfile string) error {
 			if pid == os.Getpid() {
 				return nil
 			}
-			if ExistsPid(pid) {
+			if GetCmdName(pid) == filepath.Base(os.Args[0]) {
 				return fmt.Errorf("pidfile found, try stopping another running mackerel-agent or delete %s", pidfile)
 			}
 			// Note mackerel-agent in windows can't remove pidfile during stoping the service

pidfile/pidfile_test.go

@@ -3,7 +3,6 @@
 package pidfile
 
 import (
-	"fmt"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -17,7 +16,7 @@ func TestCreate(t *testing.T) {
 		t.Errorf("err should be nil but: %v", err)
 	}
 
-	dir, err := ioutil.TempDir("", "")
+	dir, err := ioutil.TempDir("", "mackerel-agent-test-pidfile")
 	if err != nil {
 		t.Fatalf("failed to create tempdir")
 	}
@@ -39,26 +38,12 @@ func TestCreate(t *testing.T) {
 	}
 }
 
-func TestCreate_mutex(t *testing.T) {
-	dir, err := ioutil.TempDir("", "")
-	if err != nil {
-		t.Fatalf("failed to create tempdir")
-	}
-	defer os.RemoveAll(dir)
-	pidfile := filepath.Join(dir, "pidfile")
-	ioutil.WriteFile(pidfile, []byte(fmt.Sprintf("%d", os.Getppid())), 0644)
-	err = Create(pidfile)
-	if err == nil {
-		t.Errorf("if pidfile exists and its process is running, an error should be returned, but successfully overwriting the pidfile unintentionally")
-	}
-}
-
 func TestRemove(t *testing.T) {
 	err := Remove("")
 	if err != nil {
 		t.Errorf("err should be nil but: %v", err)
 	}
-	dir, err := ioutil.TempDir("", "")
+	dir, err := ioutil.TempDir("", "mackerel-agent-test-pidfile")
 	if err != nil {
 		t.Fatalf("failed to create tempdir")
 	}