Clean up Windows action process trees

pkg/runner/BUILD.bazel

@@ -7,6 +7,8 @@ go_library(
         "clean_runner.go",
         "local_runner.go",
         "local_runner_darwin.go",
+        "local_runner_process.go",
+        "local_runner_process_windows.go",
         "local_runner_rss_bytes.go",
         "local_runner_rss_kibibytes.go",
         "local_runner_unix.go",
@@ -69,12 +71,13 @@ go_test(
     srcs = [
         "apple_xcode_resolving_runner_test.go",
         "clean_runner_test.go",
+        "local_runner_process_windows_test.go",
         "local_runner_test.go",
         "path_existence_checking_runner_test.go",
         "temporary_directory_symlinking_runner_test.go",
     ],
+    embed = [":runner"],
     deps = [
-        ":runner",
         "//internal/mock",
         "//pkg/cleaner",
         "//pkg/proto/resourceusage",

pkg/runner/local_runner.go

@@ -125,7 +125,9 @@ func NewPlainCommandCreator(sysProcAttr *syscall.SysProcAttr) CommandCreator {
 }
 
 // NewLocalRunner returns a Runner capable of running commands on the
-// local system directly.
+// local system directly. On Windows, commands are placed in a
+// non-breakaway job object, and any surviving descendants are terminated
+// and waited for when the root process exits.
 func NewLocalRunner(buildDirectory filesystem.Directory, buildDirectoryPath *path.Builder, commandCreator CommandCreator, setTmpdirEnvironmentVariable bool) runner.RunnerServer {
 	return &localRunner{
 		buildDirectory:               buildDirectory,
@@ -185,10 +187,17 @@ func (r *localRunner) Run(ctx context.Context, request *runner.RunRequest) (*run
 
 	// Start the subprocess. We can already close the output files
 	// while the process is running.
+	commandProcess, err := prepareCommandForStart(cmd)
+	if err != nil {
+		stdout.Close()
+		stderr.Close()
+		return nil, util.StatusWrap(err, "Failed to prepare process")
+	}
 	err = cmd.Start()
 	stdout.Close()
 	stderr.Close()
 	if err != nil {
+		commandProcess.Close()
 		code := codes.Internal
 		for _, invalidArgumentErr := range invalidArgumentErrs {
 			if errors.Is(err, invalidArgumentErr) {
@@ -198,13 +207,24 @@ func (r *localRunner) Run(ctx context.Context, request *runner.RunRequest) (*run
 		}
 		return nil, util.StatusWrapWithCode(err, code, "Failed to start process")
 	}
+	if err := commandProcess.AfterStart(cmd); err != nil {
+		return nil, util.StatusWrap(err, "Failed to finish process startup")
+	}
 
 	// Wait for execution to complete. Permit non-zero exit codes.
-	if err := cmd.Wait(); err != nil {
-		if _, ok := err.(*exec.ExitError); !ok {
-			return nil, err
+	waitErr := cmd.Wait()
+	afterWaitErr := commandProcess.AfterWait(cmd)
+	if waitErr != nil {
+		if afterWaitErr != nil {
+			return nil, afterWaitErr
+		}
+		if _, ok := waitErr.(*exec.ExitError); !ok {
+			return nil, waitErr
 		}
 	}
+	if afterWaitErr != nil {
+		return nil, afterWaitErr
+	}
 
 	// Attach rusage information to the response.
 	posixResourceUsage, err := anypb.New(getPOSIXResourceUsage(cmd))

pkg/runner/local_runner_process.go

@@ -0,0 +1,29 @@
+//go:build !windows
+// +build !windows
+
+package runner
+
+import "os/exec"
+
+type commandProcess struct{}
+
+// prepareCommandForStart is called after the command is fully configured and
+// before Start. Non-Windows platforms do not need extra process-tree state.
+func prepareCommandForStart(cmd *exec.Cmd) (*commandProcess, error) {
+	return &commandProcess{}, nil
+}
+
+// AfterStart is called after Start succeeds and before the caller waits.
+func (commandProcess) AfterStart(cmd *exec.Cmd) error {
+	return nil
+}
+
+// AfterWait is called after Wait returns, before build directory cleanup can
+// proceed.
+func (commandProcess) AfterWait(cmd *exec.Cmd) error {
+	return nil
+}
+
+// Close releases any resources allocated by prepareCommandForStart. It must be
+// safe to call if Start fails.
+func (commandProcess) Close() {}