From e634cf76b07a64b4f84449f0c6428e568c0e5d04 Mon Sep 17 00:00:00 2001
From: Glazier Bot <glazier-bot@google.com>
Date: Tue, 3 Nov 2026 07:44:34 -0800
Subject: [PATCH] Terminate Windows child process tree when ExecTimeout
 expires.

On Windows, `os.Process.Kill()` (`TerminateProcess`) terminates only the immediate process (`cmd.Process`) and leaves descendant child processes running. When `helpers.Exec` runs a command with `conf.Timeout` such as `powershell.exe` or `dism.exe` that spawns child processes (e.g. `DismHost.exe`) and hits the timeout, the child processes remain orphaned and can continue holding file and CBS/DISM locks or open stdout/stderr pipes indefinitely.

When `conf.Timeout` is set:
1. Set `cmd.WaitDelay = 10 * time.Second` so `cmd.Wait()` is guaranteed to return even if a descendant escapes and holds pipes open.
2. In the timeout callback, run `taskkill.exe /T /F /PID` (guarded by `done` to avoid PID reuse after `cmd.Wait()` returns) before `p.Kill()` so the process tree is terminated only on timeout without affecting non-timeout executions.

PiperOrigin-RevId: 1013005744
---
 go/helpers/helpers.go              | 12 +++++++++++-
 go/helpers/helpers_stub.go         |  7 +++++++
 go/helpers/helpers_windows.go      | 12 ++++++++++++
 go/helpers/helpers_windows_test.go | 24 ++++++++++++++++++++++++
 4 files changed, 54 insertions(+), 1 deletion(-)

diff --git a/go/helpers/helpers.go b/go/helpers/helpers.go
index af533111..e27aa759 100644
--- a/go/helpers/helpers.go
+++ b/go/helpers/helpers.go
@@ -25,6 +25,7 @@ import (
 	"path/filepath"
 	"regexp"
 	"strings"
+	"sync/atomic"
 	"syscall"
 	"time"
 
@@ -253,6 +254,11 @@ func execute(path string, args []string, conf *ExecConfig) (ExecResult, error) {
 		cmd.Stderr = &errbuf
 	}
 
+	if conf.Timeout != nil {
+		// Give up on lingering pipe holders after the process exits.
+		cmd.WaitDelay = 10 * time.Second
+	}
+
 	start := time.Now()
 	// Start command asynchronously
 	deck.InfofA("Executing: %v \n", cmd.Args).With(deck.V(2)).Go()
@@ -261,15 +267,19 @@ func execute(path string, args []string, conf *ExecConfig) (ExecResult, error) {
 	}
 
 	var timer *time.Timer
+	var done atomic.Bool
 	// Create a timer that will kill the process
 	if conf.Timeout != nil {
 		timer = time.AfterFunc(*conf.Timeout, func() {
-			cmd.Process.Kill()
+			if !done.Load() { // taskkill /PID isn't safe after Wait (PID reuse)
+				killProcessTree(cmd.Process)
+			}
 		})
 	}
 
 	// Wait for execution
 	result.ExitErr = cmd.Wait()
+	done.Store(true)
 
 	// Populate the result object
 	result.Stdout = outbuf.Bytes()
diff --git a/go/helpers/helpers_stub.go b/go/helpers/helpers_stub.go
index 0acc3209..6669c97a 100644
--- a/go/helpers/helpers_stub.go
+++ b/go/helpers/helpers_stub.go
@@ -17,3 +17,10 @@
 
 package helpers
 
+import "os"
+
+func killProcessTree(p *os.Process) {
+	if p != nil {
+		p.Kill()
+	}
+}
diff --git a/go/helpers/helpers_windows.go b/go/helpers/helpers_windows.go
index ceef7e28..1ea9c5c8 100644
--- a/go/helpers/helpers_windows.go
+++ b/go/helpers/helpers_windows.go
@@ -19,7 +19,10 @@ package helpers
 
 import (
 	"fmt"
+	"os"
+	"os/exec"
 	"regexp"
+	"strconv"
 	"syscall"
 	"time"
 	"unsafe"
@@ -86,6 +89,15 @@ const (
 	swpShowWindow = 0x0040
 )
 
+func killProcessTree(p *os.Process) {
+	if p == nil {
+		return
+	}
+	// taskkill /T walks the PPID chain, so run it before killing the parent.
+	exec.Command(os.ExpandEnv(`${SystemRoot}\System32\taskkill.exe`), "/T", "/F", "/PID", strconv.Itoa(p.Pid)).Run()
+	p.Kill()
+}
+
 // GetServiceState interrogates local system services and returns their status and configuration.
 func GetServiceState(name string) (svc.Status, mgr.Config, error) {
 	m, err := mgr.Connect()
diff --git a/go/helpers/helpers_windows_test.go b/go/helpers/helpers_windows_test.go
index ee91951f..dca43918 100644
--- a/go/helpers/helpers_windows_test.go
+++ b/go/helpers/helpers_windows_test.go
@@ -18,8 +18,10 @@
 package helpers
 
 import (
+	"bytes"
 	"errors"
 	"fmt"
+	"os"
 	"regexp"
 	"testing"
 	"time"
@@ -104,3 +106,25 @@ func TestHasAdmin(t *testing.T) {
 		t.Skip("Test running with elevated privileges, skipping check for non-elevated context.")
 	}
 }
+
+func TestExecuteTimeoutKillsChildProcessTree(t *testing.T) {
+	timeout := 200 * time.Millisecond
+	var stdout, stderr bytes.Buffer
+	start := time.Now()
+	_, err := execute(
+		os.ExpandEnv("${SystemRoot}\\System32\\cmd.exe"),
+		[]string{"/c", PsPath, "-NoProfile", "-NonInteractive", "-NoLogo", "-Command", "Start-Sleep -Seconds 30"},
+		&ExecConfig{
+			Timeout:     &timeout,
+			WriteStdOut: &stdout,
+			WriteStdErr: &stderr,
+		},
+	)
+	elapsed := time.Since(start)
+	if !errors.Is(err, ErrTimeout) {
+		t.Errorf("execute() error = %v, want %v", err, ErrTimeout)
+	}
+	if elapsed >= 8*time.Second {
+		t.Errorf("execute() elapsed = %v, want < 8s (child process tree was not killed promptly on timeout)", elapsed)
+	}
+}