From ce8df2391443a375e7530033de912b43f08aa834 Mon Sep 17 00:00:00 2001 From: Xiaowen-Yang Date: Sun, 13 Sep 2026 11:53:40 +0200 Subject: [PATCH] machine: clean up orphaned gvproxy and win-sshproxy when starting Windows machines Fixes: #29474 Signed-off-by: Xiaowen-Yang --- pkg/machine/gvproxy.go | 6 ++- pkg/machine/machine_windows.go | 56 ++++++++++++++++++++++++++ pkg/machine/machine_windows_test.go | 36 +++++++++++++++++ pkg/machine/shim/networking.go | 7 ++++ pkg/machine/shim/networking_unix.go | 4 ++ pkg/machine/shim/networking_windows.go | 27 +++++++++++++ 6 files changed, 135 insertions(+), 1 deletion(-) diff --git a/pkg/machine/gvproxy.go b/pkg/machine/gvproxy.go index b255128f09..c95ad9a276 100644 --- a/pkg/machine/gvproxy.go +++ b/pkg/machine/gvproxy.go @@ -48,7 +48,11 @@ func CleanupGVProxy(f define.VMFile) error { if err != nil { return fmt.Errorf("unable to convert pid to integer: %w", err) } - if err := waitOnProcess(proxyPid); err != nil { + return cleanupGVProxy(proxyPid, f) +} + +func cleanupGVProxy(proxyPID int, f define.VMFile) error { + if err := waitOnProcess(proxyPID); err != nil { return err } return removeGVProxyPIDFile(f) diff --git a/pkg/machine/machine_windows.go b/pkg/machine/machine_windows.go index 24f02cd938..aa4e09c054 100644 --- a/pkg/machine/machine_windows.go +++ b/pkg/machine/machine_windows.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "io/fs" + "math" "net" "os" "os/exec" @@ -102,6 +103,58 @@ func DialNamedPipe(ctx context.Context, path string) (net.Conn, error) { return winio.DialPipeContext(ctx, path) } +func cleanupStaleProxy(pipeName string, recordedPID uint32, cleanup func() error) error { + if err := cleanup(); err != nil { + return fmt.Errorf("cleaning up stale proxy process %d: %w", recordedPID, err) + } + if !PipeNameAvailable(pipeName, MachineNameWait) { + return fmt.Errorf("named pipe %q is still in use after cleaning up stale proxy process %d", pipeName, recordedPID) + } + return nil +} + +// CleanupStaleGVProxy stops a gvproxy process left behind by an externally +// stopped VM when its named pipe and PID file still exist. +func CleanupStaleGVProxy(pipeName string, pidFile define.VMFile) error { + if PipeNameAvailable(pipeName, 0) { + return nil + } + + pid, err := pidFile.ReadPIDFrom() + if err != nil { + return fmt.Errorf("reading gvproxy PID file while named pipe %q is in use: %w", pipeName, err) + } + // Accept proxy PIDs from 1 through 2^32-1 (4,294,967,295) to avoid truncation or invalidation during conversion. + if pid <= 0 || uint64(pid) > uint64(math.MaxUint32) { + return fmt.Errorf("invalid gvproxy PID %d while named pipe %q is in use", pid, pipeName) + } + + return cleanupStaleProxy(pipeName, uint32(pid), func() error { + return cleanupGVProxy(pid, pidFile) + }) +} + +// CleanupStaleWinProxy stops a win-sshproxy process left behind by an +// externally stopped WSL VM when its named pipe and PID/TID file still exist. +func CleanupStaleWinProxy(name string, vmtype define.VMType) error { + pipeName := env.WithPodmanPrefix(name) + if PipeNameAvailable(pipeName, 0) { + return nil + } + + pid, tid, tidFile, err := readWinProxyTid(name, vmtype) + if err != nil { + return fmt.Errorf("reading win-sshproxy state while named pipe %q is in use: %w", pipeName, err) + } + if pid == 0 || tid == 0 { + return fmt.Errorf("invalid win-sshproxy state %d:%d while named pipe %q is in use", pid, tid, pipeName) + } + + return cleanupStaleProxy(pipeName, pid, func() error { + return stopWinProxy(pid, tid, tidFile) + }) +} + func LaunchWinProxy(opts WinProxyOpts, noInfo bool) { globalName, pipeName, err := launchWinProxy(opts) if !noInfo { @@ -194,7 +247,10 @@ func StopWinProxy(name string, vmtype define.VMType) error { if err != nil { return err } + return stopWinProxy(pid, tid, tidFile) +} +func stopWinProxy(pid, tid uint32, tidFile string) error { proc, err := os.FindProcess(int(pid)) if err != nil { //nolint:nilerr diff --git a/pkg/machine/machine_windows_test.go b/pkg/machine/machine_windows_test.go index c15e58ed82..8ade990018 100644 --- a/pkg/machine/machine_windows_test.go +++ b/pkg/machine/machine_windows_test.go @@ -3,13 +3,49 @@ package machine import ( + "fmt" "os" "os/exec" + "path/filepath" "testing" + winio "github.com/Microsoft/go-winio" "github.com/stretchr/testify/require" + "go.podman.io/podman/v6/pkg/machine/define" ) +func testNamedPipe(t *testing.T) (string, func() error) { + t.Helper() + + pipeName := fmt.Sprintf("podman-machine-test-%d", os.Getpid()) + listener, err := winio.ListenPipe(`\\.\pipe\`+pipeName, nil) + require.NoError(t, err) + t.Cleanup(func() { _ = listener.Close() }) + return pipeName, listener.Close +} + +// A stale proxy is cleaned up when its state file has already supplied the PID. +func TestCleanupStaleProxy(t *testing.T) { + pipeName, closePipe := testNamedPipe(t) + cleaned := false + err := cleanupStaleProxy(pipeName, uint32(os.Getpid()), func() error { + cleaned = true + return closePipe() + }) + + require.NoError(t, err) + require.True(t, cleaned) +} + +func TestCleanupStaleGVProxyFailsWithoutPIDFile(t *testing.T) { + pipeName, closePipe := testNamedPipe(t) + defer func() { _ = closePipe() }() + + pidFile := define.VMFile{Path: filepath.Join(t.TempDir(), "missing.pid")} + err := CleanupStaleGVProxy(pipeName, pidFile) + require.ErrorContains(t, err, "reading gvproxy PID file") +} + // CreateNewItemWithPowerShell creates a new item using PowerShell. // It's an helper to easily create junctions on Windows (as well as other file types). // It constructs a PowerShell command to create a new item at the specified path with the given item type. diff --git a/pkg/machine/shim/networking.go b/pkg/machine/shim/networking.go index 160d000250..0dfc73e112 100644 --- a/pkg/machine/shim/networking.go +++ b/pkg/machine/shim/networking.go @@ -92,6 +92,13 @@ func startHostForwarder(mc *vmconfigs.MachineConfig, provider vmconfigs.VMProvid } func startNetworking(mc *vmconfigs.MachineConfig, provider vmconfigs.VMProvider) (string, machine.APIForwardingState, error) { + // An externally stopped VM can leave its host proxy behind. On Windows, + // clean up a verified orphan before checking the SSH port; otherwise the + // orphan itself can cause an unnecessary port reassignment. + if err := cleanupStaleHostForwarder(mc, provider); err != nil { + return "", 0, err + } + // Check if SSH port is in use, and reassign if necessary if !ports.IsLocalPortAvailable(mc.SSH.Port) { logrus.Warnf("detected port conflict on machine ssh port [%d], reassigning", mc.SSH.Port) diff --git a/pkg/machine/shim/networking_unix.go b/pkg/machine/shim/networking_unix.go index 9db91698e0..8160b6903f 100644 --- a/pkg/machine/shim/networking_unix.go +++ b/pkg/machine/shim/networking_unix.go @@ -17,6 +17,10 @@ import ( func setGvproxyProcessAttributes(_ *exec.Cmd) {} +func cleanupStaleHostForwarder(_ *vmconfigs.MachineConfig, _ vmconfigs.VMProvider) error { + return nil +} + func setupMachineSockets(mc *vmconfigs.MachineConfig, dirs *define.MachineDirs) ([]string, string, machine.APIForwardingState, error) { hostSocket, err := mc.APISocket() if err != nil { diff --git a/pkg/machine/shim/networking_windows.go b/pkg/machine/shim/networking_windows.go index 50ab0ba1c6..73d76c06c1 100644 --- a/pkg/machine/shim/networking_windows.go +++ b/pkg/machine/shim/networking_windows.go @@ -25,6 +25,33 @@ func setGvproxyProcessAttributes(c *exec.Cmd) { } } +func cleanupStaleHostForwarder(mc *vmconfigs.MachineConfig, provider vmconfigs.VMProvider) error { + if provider.VMType() == define.WSLVirt { + if err := machine.CleanupStaleWinProxy(mc.Name, provider.VMType()); err != nil { + return fmt.Errorf("could not recover api proxy for %s: %w", env.WithPodmanPrefix(mc.Name), err) + } + return nil + } + if provider.UseProviderNetworkSetup() { + return nil + } + + dirs, err := env.GetMachineDirs(provider.VMType()) + if err != nil { + return err + } + pidFile, err := dirs.RuntimeDir.AppendToNewVMFile("gvproxy.pid", nil) + if err != nil { + return err + } + + pipeName := env.WithPodmanPrefix(mc.Name) + if err := machine.CleanupStaleGVProxy(pipeName, *pidFile); err != nil { + return fmt.Errorf("could not recover api proxy for %s: %w", pipeName, err) + } + return nil +} + func setupMachineSockets(mc *vmconfigs.MachineConfig, _ *define.MachineDirs) ([]string, string, machine.APIForwardingState, error) { machinePipe := env.WithPodmanPrefix(mc.Name) if !machine.PipeNameAvailable(machinePipe, machine.MachineNameWait) {