Change to using gopsutil for cross-OS process ops

Instead of trying to write out own code to do basic process
operations (e.g. checking if a PID is still running in a multi-OS
friendly manner), use shirou/gopsutil, a multi-platform library
that should abstract all the complexity away. Unlike our previous
approach on Windows, this one should actually work.

Signed-off-by: Matt Heon <[email protected]>

cmd/podman/machine/server9p.go

@@ -8,11 +8,12 @@ import (
 	"fmt"
 	"strconv"
 	"strings"
+	"time"
 
 	"github.com/containers/common/pkg/completion"
 	"github.com/containers/podman/v4/cmd/podman/registry"
 	"github.com/containers/podman/v4/pkg/fileserver"
-	"github.com/containers/podman/v4/pkg/util"
+	psutil "github.com/shirou/gopsutil/v3/process"
 	"github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 )
@@ -78,9 +79,20 @@ func remoteDirServer(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
-	// Wait for the given PID to exit
-	if err := util.WaitForPIDExit(uint(pid)); err != nil {
-		return err
+	p, err := psutil.NewProcess(int32(pid))
+	if err != nil {
+		return fmt.Errorf("opening gvproxy PID: %w", err)
+	}
+	for {
+		running, err := p.IsRunning()
+		if err != nil {
+			return fmt.Errorf("checking is gvproxy is dead: %w", err)
+		}
+		if !running {
+			break
+		}
+
+		time.Sleep(1 * time.Second)
 	}
 
 	logrus.Infof("Exiting cleanly as PID %d has died", pid)

go.mod

@@ -57,6 +57,7 @@ require (
 	github.com/opencontainers/selinux v1.11.0
 	github.com/openshift/imagebuilder v1.2.5
 	github.com/rootless-containers/rootlesskit v1.1.1
+	github.com/shirou/gopsutil/v3 v3.23.9
 	github.com/sirupsen/logrus v1.9.3
 	github.com/spf13/cobra v1.7.0
 	github.com/spf13/pflag v1.0.5
@@ -148,6 +149,7 @@ require (
 	github.com/kr/fs v0.1.0 // indirect
 	github.com/leodido/go-urn v1.2.4 // indirect
 	github.com/letsencrypt/boulder v0.0.0-20230213213521-fdfea0d469b6 // indirect
+	github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0 // indirect
 	github.com/mailru/easyjson v0.7.7 // indirect
 	github.com/manifoldco/promptui v0.9.0 // indirect
 	github.com/mattn/go-isatty v0.0.19 // indirect
@@ -169,11 +171,13 @@ require (
 	github.com/pkg/errors v0.9.1 // indirect
 	github.com/pkg/sftp v1.13.6 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect
+	github.com/power-devops/perfstat v0.0.0-20210106213030-5aafc221ea8c // indirect
 	github.com/proglottis/gpgme v0.1.3 // indirect
 	github.com/rivo/uniseg v0.4.4 // indirect
 	github.com/seccomp/libseccomp-golang v0.10.0 // indirect
 	github.com/secure-systems-lab/go-securesystemslib v0.7.0 // indirect
 	github.com/segmentio/ksuid v1.0.4 // indirect
+	github.com/shoenig/go-m1cpu v0.1.6 // indirect
 	github.com/sigstore/fulcio v1.4.0 // indirect
 	github.com/sigstore/rekor v1.2.2 // indirect
 	github.com/sigstore/sigstore v1.7.3 // indirect
@@ -183,11 +187,14 @@ require (
 	github.com/tchap/go-patricia/v2 v2.3.1 // indirect
 	github.com/theupdateframework/go-tuf v0.5.2 // indirect
 	github.com/titanous/rocacheck v0.0.0-20171023193734-afe73141d399 // indirect
+	github.com/tklauser/go-sysconf v0.3.12 // indirect
+	github.com/tklauser/numcpus v0.6.1 // indirect
 	github.com/twitchyliquid64/golang-asm v0.15.1 // indirect
 	github.com/u-root/uio v0.0.0-20230305220412-3e8cd9d6bf63 // indirect
 	github.com/ugorji/go/codec v1.2.11 // indirect
 	github.com/vbatts/tar-split v0.11.5 // indirect
 	github.com/vishvananda/netns v0.0.4 // indirect
+	github.com/yusufpapurcu/wmi v1.2.3 // indirect
 	go.mongodb.org/mongo-driver v1.11.3 // indirect
 	go.mozilla.org/pkcs7 v0.0.0-20210826202110-33d05740a352 // indirect
 	go.opencensus.io v0.24.0 // indirect

go.sum

@@ -723,6 +723,8 @@ github.com/letsencrypt/boulder v0.0.0-20230213213521-fdfea0d469b6/go.mod h1:PUgW
 github.com/linuxkit/virtsock v0.0.0-20201010232012-f8cee7dfc7a3/go.mod h1:3r6x7q95whyfWQpmGZTu3gk3v2YkMi05HEzl7Tf7YEo=
 github.com/linuxkit/virtsock v0.0.0-20220523201153-1a23e78aa7a2 h1:DZMFueDbfz6PNc1GwDRA8+6lBx1TB9UnxDQliCqR73Y=
 github.com/linuxkit/virtsock v0.0.0-20220523201153-1a23e78aa7a2/go.mod h1:SWzULI85WerrFt3u+nIm5F9l7EvxZTKQvd0InF3nmgM=
+github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0 h1:6E+4a0GO5zZEnZ81pIr0yLvtUWk2if982qA3F3QD6H4=
+github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0/go.mod h1:zJYVVT2jmtg6P3p1VtQj7WsuWi/y4VnjVBn7F8KPB3I=
 github.com/magiconair/properties v1.8.0/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ=
 github.com/mailru/easyjson v0.0.0-20160728113105-d5b7844b561a/go.mod h1:C1wdFJiN94OJF2b5HbByQZoLdCWB1Yqtg26g4irojpc=
 github.com/mailru/easyjson v0.0.0-20190614124828-94de47d64c63/go.mod h1:C1wdFJiN94OJF2b5HbByQZoLdCWB1Yqtg26g4irojpc=
@@ -887,6 +889,8 @@ github.com/pkg/sftp v1.13.6 h1:JFZT4XbOU7l77xGSpOdW+pwIMqP044IyjXX6FGyEKFo=
 github.com/pkg/sftp v1.13.6/go.mod h1:tz1ryNURKu77RL+GuCzmoJYxQczL3wLNNpPWagdg4Qk=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
+github.com/power-devops/perfstat v0.0.0-20210106213030-5aafc221ea8c h1:ncq/mPwQF4JjgDlrVEn3C11VoGHZN7m8qihwgMEtzYw=
+github.com/power-devops/perfstat v0.0.0-20210106213030-5aafc221ea8c/go.mod h1:OmDBASR4679mdNQnz2pUhc2G8CO2JrUAVFDRBDP/hJE=
 github.com/pquerna/cachecontrol v0.0.0-20171018203845-0dec1b30a021/go.mod h1:prYjPmNq4d1NPVmpShWobRqXY3q7Vp+80DqgxxUrUIA=
 github.com/proglottis/gpgme v0.1.3 h1:Crxx0oz4LKB3QXc5Ea0J19K/3ICfy3ftr5exgUK1AU0=
 github.com/proglottis/gpgme v0.1.3/go.mod h1:fPbW/EZ0LvwQtH8Hy7eixhp1eF3G39dtx7GUN+0Gmy0=
@@ -948,6 +952,12 @@ github.com/secure-systems-lab/go-securesystemslib v0.7.0/go.mod h1:/2gYnlnHVQ6xe
 github.com/segmentio/ksuid v1.0.4 h1:sBo2BdShXjmcugAMwjugoGUdUV0pcxY5mW4xKRn3v4c=
 github.com/segmentio/ksuid v1.0.4/go.mod h1:/XUiZBD3kVx5SmUOl55voK5yeAbBNNIed+2O73XgrPE=
 github.com/sergi/go-diff v1.2.0 h1:XU+rvMAioB0UC3q1MFrIQy4Vo5/4VsRDQQXHsEya6xQ=
+github.com/shirou/gopsutil/v3 v3.23.9 h1:ZI5bWVeu2ep4/DIxB4U9okeYJ7zp/QLTO4auRb/ty/E=
+github.com/shirou/gopsutil/v3 v3.23.9/go.mod h1:x/NWSb71eMcjFIO0vhyGW5nZ7oSIgVjrCnADckb85GA=
+github.com/shoenig/go-m1cpu v0.1.6 h1:nxdKQNcEB6vzgA2E2bvzKIYRuNj7XNJ4S/aRSwKzFtM=
+github.com/shoenig/go-m1cpu v0.1.6/go.mod h1:1JJMcUBvfNwpq05QDQVAnx3gUHr9IYF7GNg9SUEw2VQ=
+github.com/shoenig/test v0.6.4 h1:kVTaSd7WLz5WZ2IaoM0RSzRsUD+m8wRR+5qvntpn4LU=
+github.com/shoenig/test v0.6.4/go.mod h1:byHiCGXqrVaflBLAMq/srcZIHynQPQgeyvkvXnjqq0k=
 github.com/shurcooL/sanitized_anchor_name v1.0.0/go.mod h1:1NzhyTcUVG4SuEtjjoZeVRXNmyL/1OwPU0+IJeTBvfc=
 github.com/sigstore/fulcio v1.4.0 h1:05+k8BFvwTQzfCkVxESWzCN4b70KIRliGYz0Upmdrs8=
 github.com/sigstore/fulcio v1.4.0/go.mod h1:wcjlktbhoy6+ZTxO3yXpvqUxsLV+JEH4FF3a5Jz4VPI=
@@ -1028,6 +1038,10 @@ github.com/tidwall/pretty v1.0.0/go.mod h1:XNkn88O1ChpSDQmQeStsy+sBenx6DDtFZJxhV
 github.com/tidwall/pretty v1.2.0 h1:RWIZEg2iJ8/g6fDDYzMpobmaoGh5OLl4AXtGUGPcqCs=
 github.com/titanous/rocacheck v0.0.0-20171023193734-afe73141d399 h1:e/5i7d4oYZ+C1wj2THlRK+oAhjeS/TRQwMfkIuet3w0=
 github.com/titanous/rocacheck v0.0.0-20171023193734-afe73141d399/go.mod h1:LdwHTNJT99C5fTAzDz0ud328OgXz+gierycbcIx2fRs=
+github.com/tklauser/go-sysconf v0.3.12 h1:0QaGUFOdQaIVdPgfITYzaTegZvdCjmYO52cSFAEVmqU=
+github.com/tklauser/go-sysconf v0.3.12/go.mod h1:Ho14jnntGE1fpdOqQEEaiKRpvIavV0hSfmBq8nJbHYI=
+github.com/tklauser/numcpus v0.6.1 h1:ng9scYS7az0Bk4OZLvrNXNSAO2Pxr1XXRAPyjhIx+Fk=
+github.com/tklauser/numcpus v0.6.1/go.mod h1:1XfjsgE2zo8GVw7POkMbHENHzVg3GzmoZ9fESEdAacY=
 github.com/tmc/grpc-websocket-proxy v0.0.0-20170815181823-89b8d40f7ca8/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U=
 github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U=
 github.com/twitchyliquid64/golang-asm v0.15.1 h1:SU5vSMR7hnwNxj24w34ZyCi/FmDZTkS4MhqMhdFk5YI=
@@ -1090,6 +1104,8 @@ github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9de
 github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
 github.com/yuin/goldmark v1.3.5/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k=
 github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY=
+github.com/yusufpapurcu/wmi v1.2.3 h1:E1ctvB7uKFMOJw3fdOW32DwGE9I7t++CRUEMKvFoFiw=
+github.com/yusufpapurcu/wmi v1.2.3/go.mod h1:SBZ9tNy3G9/m5Oi98Zks0QjeHVDvuK0qfxQmPyzfmi0=
 github.com/yvasiyarov/go-metrics v0.0.0-20140926110328-57bccd1ccd43/go.mod h1:aX5oPXxHm3bOH+xeAttToC8pqch2ScQN/JoXYupl6xs=
 github.com/yvasiyarov/gorelic v0.0.0-20141212073537-a9bba5b9ab50/go.mod h1:NUSPSUX/bi6SeDMUh6brw0nXpxHnc96TguQh0+r/ssA=
 github.com/yvasiyarov/newrelic_platform_go v0.0.0-20140908184405-b21fdbd4370f/go.mod h1:GlGEuHIJweS1mbCqG+7vt2nvWLzLLnRHbXz5JKd/Qbg=
@@ -1328,6 +1344,7 @@ golang.org/x/sys v0.0.0-20201117170446-d9b008d0a637/go.mod h1:h1NjWce9XRLGQEsW7w
 golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20201201145000-ef89a241ccb3/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20201202213521-69691e467435/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
+golang.org/x/sys v0.0.0-20201204225414-ed752295db88/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20210112080510-489259a85091/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20210124154548-22da62e12c0c/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20210324051608-47abb6519492/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=

pkg/machine/gvproxy.go

@@ -1,38 +1,41 @@
 package machine
 
 import (
+	"errors"
 	"fmt"
 	"runtime"
 	"strconv"
 	"syscall"
 	"time"
 
+	psutil "github.com/shirou/gopsutil/v3/process"
 	"github.com/sirupsen/logrus"
 )
 
 const (
-	loops     = 10
+	loops     = 8
 	sleepTime = time.Millisecond * 1
 )
 
 // backoffForProcess checks if the process still exists, for something like
 // sigterm. If the process still exists after loops and sleep time are exhausted,
 // an error is returned
-func backoffForProcess(pid int) error {
+func backoffForProcess(p *psutil.Process) error {
 	sleepInterval := sleepTime
 	for i := 0; i < loops; i++ {
-		logrus.Debugf("Sleeping for %d milliseconds", sleepInterval.Milliseconds())
-		proxyProc, err := findProcess(pid)
-		if proxyProc == nil && err != nil {
-			logrus.Debugf("Error opening process %d: %v", pid, err)
-			// process is killed, gone
-			return nil //nolint: nilerr
+		running, err := p.IsRunning()
+		if err != nil {
+			return fmt.Errorf("checking if process running: %w", err)
 		}
+		if !running {
+			return nil
+		}
+
 		time.Sleep(sleepInterval)
 		// double the time
 		sleepInterval += sleepInterval
 	}
-	return fmt.Errorf("process %d has not ended", pid)
+	return fmt.Errorf("process %d has not ended", p.Pid)
 }
 
 // waitOnProcess takes a pid and sends a sigterm to it. it then waits for the
@@ -41,40 +44,41 @@ func backoffForProcess(pid int) error {
 func waitOnProcess(processID int) error {
 	logrus.Infof("Going to stop gvproxy (PID %d)", processID)
 
-	proxyProc, err := findProcess(processID)
+	p, err := psutil.NewProcess(int32(processID))
 	if err != nil {
-		return err
+		return fmt.Errorf("looking up PID %d: %w", processID, err)
 	}
 
 	// Try to kill the pid with sigterm
 	if runtime.GOOS != "windows" { // FIXME: temporary work around because signals are lame in windows
-		if err := proxyProc.Signal(syscall.SIGTERM); err != nil {
-			if err == syscall.ESRCH {
+		if err := p.SendSignal(syscall.SIGTERM); err != nil {
+			if errors.Is(err, syscall.ESRCH) {
 				return nil
 			}
-			return err
+			return fmt.Errorf("sending SIGTERM to grproxy: %w", err)
 		}
 
-		if err := backoffForProcess(processID); err == nil {
+		if err := backoffForProcess(p); err == nil {
 			return nil
 		}
 	}
 
-	// sigterm has not killed it yet, lets send a sigkill
-	proxyProc, err = findProcess(processID)
-	if proxyProc == nil && err != nil {
-		// process is killed, gone
-		logrus.Debugf("Error opening gvproxy process: %v", err)
-		return nil //nolint: nilerr
+	running, err := p.IsRunning()
+	if err != nil {
+		return fmt.Errorf("checking if gvproxy is running: %w", err)
+	}
+	if !running {
+		return nil
 	}
-	if err := proxyProc.Signal(syscall.SIGKILL); err != nil {
-		if err == syscall.ESRCH {
+
+	if err := p.Kill(); err != nil {
+		if errors.Is(err, syscall.ESRCH) {
 			logrus.Debugf("Gvproxy already dead, exiting cleanly")
 			return nil
 		}
 		return err
 	}
-	return backoffForProcess(processID)
+	return backoffForProcess(p)
 }
 
 // CleanupGVProxy reads the --pid-file for gvproxy attempts to stop it

pkg/util/utils_supported.go

@@ -14,7 +14,6 @@ import (
 	"strconv"
 	"syscall"
 
-	"github.com/containers/podman/v4/libpod/define"
 	"github.com/containers/podman/v4/pkg/rootless"
 	"github.com/sirupsen/logrus"
 )
@@ -119,9 +118,3 @@ func GetRootlessPauseProcessPidPath() (string, error) {
 	// the tmpdir which can be changed via --tmpdir.
 	return filepath.Join(runtimeDir, "libpod", "tmp", "pause.pid"), nil
 }
-
-// WaitForPIDExit waits for a PID to exit.
-// Not implemented for Linux at this time, only for Windows.
-func WaitForPIDExit(pid uint) error {
-	return define.ErrNotImplemented
-}

pkg/util/utils_windows.go

@@ -9,7 +9,6 @@ import (
 	"path/filepath"
 
 	"github.com/containers/storage/pkg/homedir"
-	"golang.org/x/sys/windows"
 )
 
 var errNotImplemented = errors.New("not yet implemented")
@@ -45,35 +44,3 @@ func GetRuntimeDir() (string, error) {
 func GetRootlessConfigHomeDir() (string, error) {
 	return "", errors.New("this function is not implemented for windows")
 }
-
-// Wait until the given PID exits. Returns nil if wait was successful, errors on
-// unexpected condition (IE, pid was not valid)
-func WaitForPIDExit(pid uint) error {
-	const PROCESS_ALL_ACCESS = 0x1F0FFF
-
-	// We need to turn the PID into a Windows handle.
-	// To do this we need Windows' OpenProcess func.
-	// To get that, we need to open the kernel32 DLL.
-	kernel32, err := windows.LoadDLL("kernel32.dll")
-	if err != nil {
-		return fmt.Errorf("loading kernel32 dll: %w", err)
-	}
-
-	openProc, err := kernel32.FindProc("OpenProcess")
-	if err != nil {
-		return fmt.Errorf("loading OpenProcess API: %w", err)
-	}
-
-	handle, _, err := openProc.Call(uintptr(PROCESS_ALL_ACCESS), uintptr(1), uintptr(pid))
-	if err != nil {
-		return fmt.Errorf("converting PID to handle: %w", err)
-	}
-
-	// We can now wait for the handle.
-	_, err = windows.WaitForSingleObject(windows.Handle(handle), 0)
-	if err != nil {
-		return fmt.Errorf("waiting for handle: %w", err)
-	}
-
-	return nil
-}