Merge pull request #713 from Lomanic/issue607

[process] Fix #607 properly check PID existence
shirou · Jul 27, 2019 · 1edab19 · 1edab19
2 parents 1917a27 + 4a95469
commit 1edab19
Show file tree

Hide file tree

Showing 4 changed files with 92 additions and 35 deletions.
diff --git a/process/process.go b/process/process.go
@@ -139,21 +139,6 @@ func PidExists(pid int32) (bool, error) {
 	return PidExistsWithContext(context.Background(), pid)
 }
 
-func PidExistsWithContext(ctx context.Context, pid int32) (bool, error) {
-	pids, err := Pids()
-	if err != nil {
-		return false, err
-	}
-
-	for _, i := range pids {
-		if i == pid {
-			return true, err
-		}
-	}
-
-	return false, err
-}
-
 // Background returns true if the process is in background, false otherwise.
 func (p *Process) Background() (bool, error) {
 	return p.BackgroundWithContext(context.Background())

diff --git a/process/process_fallback.go b/process/process_fallback.go
@@ -48,6 +48,21 @@ func NewProcess(pid int32) (*Process, error) {
 	return nil, common.ErrNotImplementedError
 }
 
+func PidExistsWithContext(ctx context.Context, pid int32) (bool, error) {
+	pids, err := PidsWithContext(ctx)
+	if err != nil {
+		return false, err
+	}
+
+	for _, i := range pids {
+		if i == pid {
+			return true, err
+		}
+	}
+
+	return false, err
+}
+
 func (p *Process) Ppid() (int32, error) {
 	return p.PpidWithContext(context.Background())
 }

diff --git a/process/process_posix.go b/process/process_posix.go
@@ -4,6 +4,7 @@ package process
 
 import (
 	"context"
+	"fmt"
 	"os"
 	"os/user"
 	"path/filepath"
@@ -69,6 +70,34 @@ func getTerminalMap() (map[uint64]string, error) {
 	return ret, nil
 }
 
+func PidExistsWithContext(ctx context.Context, pid int32) (bool, error) {
+	if pid <= 0 {
+		return false, fmt.Errorf("invalid pid %v", pid)
+	}
+	proc, err := os.FindProcess(int(pid))
+	if err != nil {
+		return false, err
+	}
+	err = proc.Signal(syscall.Signal(0))
+	if err == nil {
+		return true, nil
+	}
+	if err.Error() == "os: process already finished" {
+		return false, nil
+	}
+	errno, ok := err.(syscall.Errno)
+	if !ok {
+		return false, err
+	}
+	switch errno {
+	case syscall.ESRCH:
+		return false, nil
+	case syscall.EPERM:
+		return true, nil
+	}
+	return false, err
+}
+
 // SendSignal sends a unix.Signal to the process.
 // Currently, SIGSTOP, SIGCONT, SIGTERM and SIGKILL are supported.
 func (p *Process) SendSignal(sig syscall.Signal) error {

diff --git a/process/process_windows.go b/process/process_windows.go
@@ -19,11 +19,6 @@ import (
 	"golang.org/x/sys/windows"
 )
 
-const (
-	NoMoreFiles   = 0x12
-	MaxPathLength = 260
-)
-
 var (
 	modpsapi                     = windows.NewLazySystemDLL("psapi.dll")
 	procGetProcessMemoryInfo     = modpsapi.NewProc("GetProcessMemoryInfo")
@@ -185,6 +180,44 @@ func PidsWithContext(ctx context.Context) ([]int32, error) {
 
 }
 
+func PidExistsWithContext(ctx context.Context, pid int32) (bool, error) {
+	if pid == 0 { // special case for pid 0 System Idle Process
+		return true, nil
+	}
+	if pid < 0 {
+		return false, fmt.Errorf("invalid pid %v", pid)
+	}
+	if pid%4 != 0 {
+		// OpenProcess will succeed even on non-existing pid here https://devblogs.microsoft.com/oldnewthing/20080606-00/?p=22043
+		// so we list every pid just to be sure and be future-proof
+		pids, err := PidsWithContext(ctx)
+		if err != nil {
+			return false, err
+		}
+		for _, i := range pids {
+			if i == pid {
+				return true, err
+			}
+		}
+		return false, err
+	}
+	const STILL_ACTIVE = 259 // https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-getexitcodeprocess
+	h, err := windows.OpenProcess(windows.PROCESS_QUERY_LIMITED_INFORMATION, false, uint32(pid))
+	if err == windows.ERROR_ACCESS_DENIED {
+		return true, nil
+	}
+	if err == windows.ERROR_INVALID_PARAMETER {
+		return false, nil
+	}
+	if err != nil {
+		return false, err
+	}
+	defer syscall.CloseHandle(syscall.Handle(h))
+	var exitCode uint32
+	err = windows.GetExitCodeProcess(h, &exitCode)
+	return exitCode == STILL_ACTIVE, err
+}
+
 func (p *Process) Ppid() (int32, error) {
 	return p.PpidWithContext(context.Background())
 }
@@ -238,12 +271,11 @@ func (p *Process) Exe() (string, error) {
 }
 
 func (p *Process) ExeWithContext(ctx context.Context) (string, error) {
-	// 0x1000 is PROCESS_QUERY_LIMITED_INFORMATION
-	c, err := syscall.OpenProcess(0x1000, false, uint32(p.Pid))
+	c, err := windows.OpenProcess(windows.PROCESS_QUERY_LIMITED_INFORMATION, false, uint32(p.Pid))
 	if err != nil {
 		return "", err
 	}
-	defer syscall.CloseHandle(c)
+	defer windows.CloseHandle(c)
 	buf := make([]uint16, syscall.MAX_LONG_PATH)
 	size := uint32(syscall.MAX_LONG_PATH)
 	if err := procQueryFullProcessImageNameW.Find(); err == nil { // Vista+
@@ -346,15 +378,14 @@ func (p *Process) Username() (string, error) {
 
 func (p *Process) UsernameWithContext(ctx context.Context) (string, error) {
 	pid := p.Pid
-	// 0x1000 is PROCESS_QUERY_LIMITED_INFORMATION
-	c, err := syscall.OpenProcess(0x1000, false, uint32(pid))
+	c, err := windows.OpenProcess(windows.PROCESS_QUERY_LIMITED_INFORMATION, false, uint32(pid))
 	if err != nil {
 		return "", err
 	}
-	defer syscall.CloseHandle(c)
+	defer windows.CloseHandle(c)
 
 	var token syscall.Token
-	err = syscall.OpenProcessToken(c, syscall.TOKEN_QUERY, &token)
+	err = syscall.OpenProcessToken(syscall.Handle(c), syscall.TOKEN_QUERY, &token)
 	if err != nil {
 		return "", err
 	}
@@ -411,19 +442,18 @@ func (p *Process) Nice() (int32, error) {
 }
 
 func (p *Process) NiceWithContext(ctx context.Context) (int32, error) {
-	// 0x1000 is PROCESS_QUERY_LIMITED_INFORMATION
-	c, err := syscall.OpenProcess(0x1000, false, uint32(p.Pid))
+	c, err := windows.OpenProcess(windows.PROCESS_QUERY_LIMITED_INFORMATION, false, uint32(p.Pid))
 	if err != nil {
 		return 0, err
 	}
-	defer syscall.CloseHandle(c)
+	defer windows.CloseHandle(c)
 	ret, _, err := procGetPriorityClass.Call(uintptr(c))
 	if ret == 0 {
 		return 0, err
 	}
 	priority, ok := priorityClasses[int(ret)]
 	if !ok {
-		return 0, fmt.Errorf("unknown priority class %s", ret)
+		return 0, fmt.Errorf("unknown priority class %v", ret)
 	}
 	return priority, nil
 }
@@ -788,8 +818,7 @@ func getRusage(pid int32) (*windows.Rusage, error) {
 
 func getMemoryInfo(pid int32) (PROCESS_MEMORY_COUNTERS, error) {
 	var mem PROCESS_MEMORY_COUNTERS
-	// PROCESS_QUERY_LIMITED_INFORMATION is 0x1000
-	c, err := windows.OpenProcess(0x1000, false, uint32(pid))
+	c, err := windows.OpenProcess(windows.PROCESS_QUERY_LIMITED_INFORMATION, false, uint32(pid))
 	if err != nil {
 		return mem, err
 	}
@@ -823,8 +852,7 @@ type SYSTEM_TIMES struct {
 func getProcessCPUTimes(pid int32) (SYSTEM_TIMES, error) {
 	var times SYSTEM_TIMES
 
-	// PROCESS_QUERY_LIMITED_INFORMATION is 0x1000
-	h, err := windows.OpenProcess(0x1000, false, uint32(pid))
+	h, err := windows.OpenProcess(windows.PROCESS_QUERY_LIMITED_INFORMATION, false, uint32(pid))
 	if err != nil {
 		return times, err
 	}