[adamsitnik]

Summary

Make all Process.Run* overloads terminate the launched process tree on timeout or cancellation using Process.Kill(entireProcessTree: true).

• Apply tree cleanup during output capture and the subsequent exit wait.
• Preserve existing timeout/cancellation reporting, exit codes and signals, successful-completion behavior, and rejection of stream redirection by non-capturing Run methods.
• Keep the implementation in Process.Scenarios.cs, without changing SafeProcessHandle, ProcessManager, or platform-specific tree termination.
• Update XML documentation and add tests covering both overload forms, synchronous timeouts, asynchronous cancellation, and capture before and after EOF.

This is an intentional behavioral breaking change: descendants that previously survived timeout or cancellation are now terminated. Callers that need descendants to survive should use Process.Start and manage termination explicitly.

Validation

From src\libraries\System.Diagnostics.Process, on Windows x64:

• D:\projects\runtime\.dotnet\dotnet.exe build — passed for all configured targets, with zero warnings or errors, including after the final private-helper rename.
• D:\projects\runtime\.dotnet\dotnet.exe build /t:Test tests\System.Diagnostics.Process.Tests.csproj — 716 total: 713 passed, 2 skipped, 1 failed. All 12 tree-cleanup cases and 6 stream-redirection validation cases passed. This full run preceded the final name-only private-helper rename; tests were not rerun after it.
• The sole failure, ProcessHandlesTests.ChildProcess_CanBreakAwayFromJob(killOnParentExit: False), is the pre-existing native job-breakaway access-denied failure, also reproduced using the unchanged Release testhost.
• The 12 tree-cleanup regression cases were previously run against unmodified main; all failed because descendants remained running.
• git --no-pager diff --check — passed.

Unix targets were compiled but not executed.

Resolves #136466

Note

This pull request was created with GitHub Copilot.

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-diagnostics-process
See info in area-owners.md if you want to be subscribed.

[Copilot]

🟡 Changes recommended

Termination failures can leave timeout or cancellation paths waiting indefinitely, while the tests contain Debug-runtime watchdog timing errors and a scheduling-dependent capture assertion.

3 open issues

• Preserve process termination failures during timeout and cancellation · New
• Extend watchdog timeout beyond Debug runtime operation timeout · New
• Synchronize asynchronous capture completion before asserting output · New

What changed in this PR

Updates Process.Run* to terminate descendants as well as the launched process on timeout or cancellation, using the existing process-tree termination support.

Changes:

• Applies tree termination during output capture and subsequent exit waits.
• Updates XML documentation and exposes cancellation state internally on both platform-specific safe handles.
• Adds coverage for tree cleanup, redirected-stream rejection, and tree termination through a process retrieved by ID.

File	Description
src/​libraries/​System.Diagnostics.Process/​tests/​RunTests.cs	Adds tree-cleanup and stream-redirection validation tests.
src/​libraries/​System.Diagnostics.Process/​tests/​ProcessTests.cs	Tests tree termination through GetProcessById.
src/​libraries/​System.Diagnostics.Process/​src/​System/​Diagnostics/​Process.Scenarios.cs	Implements tree-aware timeout and cancellation cleanup; updates documentation.
src/​libraries/​System.Diagnostics.Process/​src/​Microsoft/​Win32/​SafeHandles/​SafeProcessHandle.Windows.cs	Makes cancellation state internally accessible on Windows.
src/​libraries/​System.Diagnostics.Process/​src/​Microsoft/​Win32/​SafeHandles/​SafeProcessHandle.Unix.cs	Makes cancellation state internally accessible on Unix.

_{Review effort: Balanced}

[Copilot]

🟡 Changes recommended

Cleanup failures can mask original capture exceptions, and descendant-cleanup tests omit synchronous timeouts, post-EOF exit waits, and filename overloads.

2 open issues

• Preserve capture exceptions when cleanup termination also fails · New
• Expand process-tree cleanup tests for timeout, EOF, and overload paths · New

3 resolved since last review

• Preserve process termination failures during timeout and cancellation
• Synchronize asynchronous capture completion before asserting output
• Extend watchdog timeout beyond Debug runtime operation timeout

_{Review effort: Balanced}

[Copilot]

🔵 Needs a closer look

On Unix, background descendants can keep redirected stdout open and survive cleanup after the shell exits because process-tree discovery skips an exited root.

No open issues 🎉

1 previously missed issue

In code that hasn't changed since last review

Track launched descendants after Unix root process exits

src/​libraries/​System.Diagnostics.Process/​src/​System/​Diagnostics/​Process.Scenarios.cs:421

On Unix, a shell can exit while a background child keeps redirected stdout open, so capture remains pending until timeout or cancellation. Cleanup then leaves that child running: Process.Unix.cs:170–173 skips tree discovery for an exited root, whose children have already been reparented. To cover this case, retain a way to identify and terminate the launched descendants from startup rather than relying solely on the root PID at cleanup time.

2 resolved since last review

• Expand process-tree cleanup tests for timeout, EOF, and overload paths
• Preserve capture exceptions when cleanup termination also fails

_{Review effort: Balanced}

[adamsitnik]

On Unix, background descendants can keep redirected stdout open and survive cleanup after the shell exits because process-tree discovery skips an exited root.

Based on my understanding of available Unix sys-calls and mechanisms, there is nothing we can do about it ;(

[Copilot]

🔵 Needs a closer look

Runtime validation covers only Windows, leaving Unix process-tree cancellation and callback/exit races unverified, with early-root-exit cleanup semantics still needing maintainer confirmation.

No open issues 🎉

_{Review effort: Balanced}

[jkotas]

Kill(entireProcessTree: true) is a ton on code. This change will add about ~400kB to a simple NAOT compiled app that just starts process using the simple APIs that this PR is changing (on Windows). Is this the right tradeoff?

[adamsitnik]

Kill(entireProcessTree: true) is a ton on code. This change will add about ~400kB to a simple NAOT compiled app that just starts process using the simple APIs that this PR is changing (on Windows).

As of today, it is. With #126273, we could make it trivial, especially here where we know that we are about to start a process and we know whether we may need to kill entire tree or not (timeout/cancelable token was provided). This is what some of our 1P customers do (I keep an eye on code that was not able to remove all custom p/invokes after migrating to the new APIs).

Is this the right tradeoff?

We are going to increase the size on disk, but on the other hand we are going to protect some of our customers from hard to diagnose issues. I hate regressing any kind of benchmarks, but here I think it's the right thing to do. Especially since we can eliminate this regression (in a follow up PR) and make the whole operation faster and atomic (at the cost of putting all processes into a Job).

[jkotas]

This is what some of our 1P customers do

We should be also asking how many are not doing this and would be actually broken by this change. This change can cut both ways - both on functionality and/or on performance.

Are there example in other ecosystems where simple process APIs try to kill the whole process tree on failure?

[adamsitnik]

We should be also asking how many are not doing this and would be actually broken by this change. This change can cut both ways - both on functionality and/or on performance.

I think that 99.9% would benefit and only 0.1% could be broken for scenarios like spawning background compiler process for performance purposes.

Are there example in other ecosystems where simple process APIs try to kill the whole process tree on failure?

I've used the Office Copilot Researcher to answer your quesiton:

@jkotas which leads me to a conclusion that this most likely should be controlled via a new, optional bool parameter:

ProcessExitStatus> RunAsync(
    string fileName, IEnumerable<string>? arguments = null,
    bool silent = false,
+   bool entireProcessTree = false,
    CancellationToken ct = default)

[jkotas]

which leads me to a conclusion that this most likely should be controlled via a new, optional bool parameter:

Yes, it sounds like that to me as well. It is unfortunate that we have made silent a bool - it would be nicer if it is an enum.

If we do this, we should look at trimming the implementation to be leaner.