On Windows, an action can (and very often will) spawn a subprocess and exit without terminating it. The remote executor then attempts to delete the action root, but the surviving subprocess may still hold files or its working directory open inside that root.
This causes cleanup to fail with an error stating that the root directory cannot be deleted because it is still in use.
Reproduction
- Run an action that starts a long-lived child process.
- Have the parent process exit without waiting for or terminating the child.
- Ensure the child keeps its working directory inside the action root or holds a file there open.
- Observe that action cleanup fails when the executor tries to delete the root.
The issue is reproducible reliably on Windows and will accur pretty much always for some of our tools.
Suggested fix
Bazel’s native Windows launcher handles this with a Windows Job Object configured with JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE. It starts the root process suspended, assigns it to the non-breakaway job, and only then resumes it. This prevents the process from spawning descendants before job assignment.
The approach cannot be translated one-to-one when using Go’s os/exec. Bazel receives and retains the primary thread handle from CreateProcessW, allowing it to call ResumeThread directly. Go’s Windows process implementation closes that thread handle before cmd.Start() returns and exposes only the process and PID. A Go implementation must therefore use another way to resume the suspended process, such as enumerating its threads, or replace os/exec process creation with lower-level Windows API calls.
When the action finishes or is cancelled, closing or terminating the job kills all surviving descendants. A completion port can be used to wait for JOB_OBJECT_MSG_ACTIVE_PROCESS_ZERO before deleting the action root, ensuring no process still holds files or directories open.
(Maybe to discuss)
Honestly, I would just move the windows implementation always to this object group as it is common in bazel. However, as there is the clean_process_table for linux and mac one could argue that we want that for Windows as well. But then it should be more general like a process_tree_cleanup or cleanup_action_processes. I think this discussion will quickly become a scope creep so I would just roll it out for windows out of the box.
On Windows, an action can (and very often will) spawn a subprocess and exit without terminating it. The remote executor then attempts to delete the action root, but the surviving subprocess may still hold files or its working directory open inside that root.
This causes cleanup to fail with an error stating that the root directory cannot be deleted because it is still in use.
Reproduction
The issue is reproducible reliably on Windows and will accur pretty much always for some of our tools.
Suggested fix
Bazel’s native Windows launcher handles this with a Windows Job Object configured with
JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE. It starts the root process suspended, assigns it to the non-breakaway job, and only then resumes it. This prevents the process from spawning descendants before job assignment.The approach cannot be translated one-to-one when using Go’s
os/exec. Bazel receives and retains the primary thread handle fromCreateProcessW, allowing it to callResumeThreaddirectly. Go’s Windows process implementation closes that thread handle beforecmd.Start()returns and exposes only the process and PID. A Go implementation must therefore use another way to resume the suspended process, such as enumerating its threads, or replaceos/execprocess creation with lower-level Windows API calls.When the action finishes or is cancelled, closing or terminating the job kills all surviving descendants. A completion port can be used to wait for
JOB_OBJECT_MSG_ACTIVE_PROCESS_ZERObefore deleting the action root, ensuring no process still holds files or directories open.(Maybe to discuss)
Honestly, I would just move the windows implementation always to this object group as it is common in bazel. However, as there is the
clean_process_tablefor linux and mac one could argue that we want that for Windows as well. But then it should be more general like aprocess_tree_cleanuporcleanup_action_processes. I think this discussion will quickly become a scope creep so I would just roll it out for windows out of the box.