Fix Process.Kill(entireProcessTree: true) hang on macOS (#131944) by jozkee · Pull Request #133930 · dotnet/runtime

jozkee · 2026-09-15T04:43:46Z

Summary

Fixes a deadlock where Process.Kill(entireProcessTree: true) can hang indefinitely on macOS (#131944), observed as the SDK dotnet-watch.Tests hanging on osx.15.arm64.

Root cause

The hang is a bad interaction between a macOS waitid quirk and the two-phase tree kill.

1. macOS waitid reports stopped children under WEXITED. The SIGCHLD reaper peeks for exited children with:

waitid(P_ALL, &siginfo, WEXITED | WNOHANG | WNOWAIT)  // SystemNative_WaitIdAnyExitedNoHangNoWait

On macOS this also returns children that are merely stopped (SIGSTOP) — si_code == CLD_STOPPED — even though only WEXITED was requested. Because WNOWAIT never consumes the notification, the reaper loop in CheckChildren gets the same stopped PID forever:

WaitIdAnyExitedNoHangNoWait() -> stopped PID   (macOS reports it)
  TryReapChild() -> waitpid(pid, WNOHANG) -> 0  (stopped, not exited: nothing reaped)
loop -> WaitIdAnyExitedNoHangNoWait() -> same stopped PID -> spin forever

This spin runs while holding s_childProcessWaitStates and the s_processStartLock write lock.

2. The two-phase StopTree (from #128598) holds the tree stopped. Kill(entireProcessTree: true) now SIGSTOPs the entire tree up front and defers all SIGKILLs to the end:

KillTree:
    StopTree()          // recursively SIGSTOP the whole tree; kill nothing yet
    finally: SIGKILL every collected process

So a direct child sits in the stopped state across the whole traversal. If a concurrent kill/exit fires SIGCHLD during that window, the reaper wedges on the stopped child. The killing thread's next step (StopTree -> GetChildProcesses -> constructing a Process -> AddRef) needs s_childProcessWaitStates, which the spinning reaper holds — so the tree is never killed and the stop never ends:

Thread A (Kill): StopTree ... GetChildProcesses -> AddRef -> lock(s_childProcessWaitStates)  [BLOCKED]
Thread B (SIGCHLD reaper): spinning on the stopped child, holding that same lock   [FOREVER]

That circular wait is the reported hang ("nine calls entering Kill, only three returned").

Why it's a regression

The macOS waitid quirk is long-standing, but in .NET 10 KillTree was one-phase — it stopped a node and SIGKILLed it immediately, before recursing — so a direct child was stopped only for a microscopic window and killed without needing the lock again. Confirmed on .NET 10.0.12: the repro below runs 30/30 clean. #128598's two-phase design widened that window to the entire tree traversal, turning an essentially-unhittable race into a reliable hang.

Fix

In SystemNative_WaitIdAnyExitedNoHangNoWait, only report children whose si_code indicates an actual exit (CLD_EXITED / CLD_KILLED / CLD_DUMPED). When macOS reports a stopped/continued child, consume that notification with a targeted waitid(P_PID, ..., WSTOPPED | WCONTINUED | WNOHANG) and keep looking for a real exit.

The consuming waitid is essential (not just cosmetic): macOS returns the stopped child in preference to an already-exited sibling and hides the exit behind it. Draining the stop unmasks the exited child so it's still reaped — verified with a standalone probe. On platforms that honor WEXITED (e.g. Linux) the new branch is never taken, so behavior there is unchanged.

Testing

New regression test Kill_EntireProcessTree_Concurrent_DoesNotHang (macOS): concurrently kills 8 process trees with a 60s watchdog. Fails (hangs on iteration 1) without the fix; passes with it.
Full System.Diagnostics.Process suite: green.
Cross-checked against shipped .NET 10.0.12: the same workload does not hang, confirming the regression.

Resolves #131944

Note

This PR description was generated with the assistance of GitHub Copilot.

On macOS, waitid(P_ALL, WEXITED | WNOHANG | WNOWAIT) also reports children that have stopped (SIGSTOP) or continued (SIGCONT), not just exited ones. Because WNOWAIT does not consume the notification, SystemNative_WaitIdAnyExitedNoHangNoWait kept returning the same stopped PID, causing the SIGCHLD reaper (CheckChildren) to spin forever while holding s_childProcessWaitStates / s_processStartLock. Process.Kill(entireProcessTree: true) SIGSTOPs the whole tree before killing it (the two-phase StopTree added in dotnet#128598), so a concurrent kill would leave a direct child stopped long enough for the reaper to wedge on it, deadlocking every concurrent Kill/Start. This is why the SDK dotnet-watch tests hung on osx.15.arm64. Only report children whose si_code indicates an actual exit (CLD_EXITED / CLD_KILLED / CLD_DUMPED). When a stopped/continued child is reported, consume that notification and keep looking for a real exit, which also unmasks any exited child that macOS was hiding behind the stopped one. On platforms that honor WEXITED (e.g. Linux) the new branch is never taken. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

azure-pipelines · 2026-09-15T04:44:27Z

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 · 2026-09-15T04:44:55Z

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

Unresolved moderate issues remain in test synchronization, cleanup, grandchild verification, and native notification-drain scoping.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This pull request fixes a macOS deadlock during concurrent process-tree termination.

Changes:

Filters waitid results and drains stopped/continued notifications.
Adds a macOS concurrent process-tree kill regression test.
Addresses moderate follow-ups for grandchild readiness and verification, failure cleanup, and limiting drains to tracked children.

File summaries

File	Summary
`src/native/libs/System.Native/pal_process.c`	Updates exit detection and notification draining; draining should be limited to managed children.
`src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs`	Adds the regression test; it needs reliable grandchild startup, termination assertions, and timeout cleanup.

Review details

Suppressed comments (2)

src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs:1201

The fixed 500 ms delay does not establish that each /bin/sleep grandchild has started before the roots are stopped. On a slow or loaded macOS runner, tree enumeration can miss a grandchild, making this test pass without exercising the recursive path and leaving that orphaned sleep behind. Use a readiness handshake (or otherwise wait for each grandchild PID) before starting the concurrent kills.

                // Give the grandchildren time to start so the trees are fully formed.
                Thread.Sleep(500);

src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs:1211

If Task.WaitAll times out, it only returns false; the subsequent Assert.True throws while all eight Kill tasks remain blocked. This method has no finally, and ProcessTestBase.Dispose only calls Kill() on the tracked roots, so each remote root's /bin/sleep grandchild is orphaned and left running (and cleanup can encounter the same reaper deadlock). Please add failure-path cleanup that can terminate both the roots and the spawned grandchildren without calling the hanging tree-kill operation.

                bool completed = Task.WaitAll(tasks, TimeSpan.FromSeconds(60));
                Assert.True(completed, $"Kill(entireProcessTree: true) hung on iteration {iteration}.");

Files reviewed: 2/2 changed files
Comments generated: 2
Review effort level: Lite

+                foreach (Process root in roots)
+                {
+                    Assert.True(root.WaitForExit(WaitInMS));


adamsitnik · 2026-09-15T09:11:18Z

+            siginfo_t drain;
+            memset(&drain, 0, sizeof(drain));
+            while (CheckInterrupted(result = waitid(P_PID, (id_t)siginfo.si_pid, &drain, WSTOPPED | WCONTINUED | WNOHANG)));


This is valid feedback. I wish SA_NOCLDSTOP would just prevent us from getting here in the first place.

adamsitnik

@jozkee Big thanks for helping me with this!

adamsitnik · 2026-09-15T09:03:46Z

+        // another Kill) then blocks on that lock, so the stopped child is never SIGKILL'd -> deadlock.
+        [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
+        [PlatformSpecific(TestPlatforms.OSX)]
+        public void Kill_EntireProcessTree_Concurrent_DoesNotHang()


I was able to confirm that this test reproduces the problem: adamsitnik/macosrepro#2

adamsitnik · 2026-09-15T09:07:33Z

+                return 0;
+            }
+
+            // We requested WEXITED only, but some platforms (notably macOS) also report


Hmm this is exactly why we have used SA_NOCLDSTOP in #128598 so it "should work" but apparently was not enough. It's reasonable for me to make it more defensive here. Linux and other reasonable Unixes should not be affected in any negative way (cc @tmds for double checking)

adamsitnik · 2026-09-15T09:09:49Z

+            const int TreeCount = 8;
+            const int Iterations = 30;


Hmm this test spawns a LOT of processes. I think it's reasonable to keep it, but we should most likely consider moving it to Outerloop.

adamsitnik · 2026-09-15T09:11:18Z

+            siginfo_t drain;
+            memset(&drain, 0, sizeof(drain));
+            while (CheckInterrupted(result = waitid(P_PID, (id_t)siginfo.si_pid, &drain, WSTOPPED | WCONTINUED | WNOHANG)));


This is valid feedback. I wish SA_NOCLDSTOP would just prevent us from getting here in the first place.

jozkee requested a review from adamsitnik September 15, 2026 04:43

dotnet-policy-service Bot assigned jozkee Sep 15, 2026

github-actions Bot added the area-System.Diagnostics.Process label Sep 15, 2026

adamsitnik requested a lite review from Copilot September 15, 2026 09:00

Copilot started reviewing on behalf of adamsitnik September 15, 2026 09:01 View session

Copilot AI reviewed Sep 15, 2026

View reviewed changes

adamsitnik reviewed Sep 15, 2026

View reviewed changes

Navigation Menu

Uh oh!

Fix Process.Kill(entireProcessTree: true) hang on macOS (#131944) - #133930

Fix Process.Kill(entireProcessTree: true) hang on macOS (#131944)#133930
jozkee wants to merge 1 commit into
dotnet:mainfrom
jozkee:fix-131944-process-kill-tree-macos-hang

jozkee commented Sep 15, 2026

Uh oh!

azure-pipelines Bot commented Sep 15, 2026

Uh oh!

dotnet-policy-service Bot commented Sep 15, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

adamsitnik Sep 15, 2026

Uh oh!

adamsitnik left a comment

Uh oh!

adamsitnik Sep 15, 2026

Uh oh!

adamsitnik Sep 15, 2026

Uh oh!

adamsitnik Sep 15, 2026

Uh oh!

adamsitnik Sep 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

jozkee commented Sep 15, 2026

Summary

Root cause

Why it's a regression

Fix

Testing

Uh oh!

azure-pipelines Bot commented Sep 15, 2026

Uh oh!

dotnet-policy-service Bot commented Sep 15, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (2)

Uh oh!

adamsitnik Sep 15, 2026

Choose a reason for hiding this comment

Uh oh!

adamsitnik left a comment

Choose a reason for hiding this comment

Uh oh!

adamsitnik Sep 15, 2026

Choose a reason for hiding this comment

Uh oh!

adamsitnik Sep 15, 2026

Choose a reason for hiding this comment

Uh oh!

adamsitnik Sep 15, 2026

Choose a reason for hiding this comment

Uh oh!

adamsitnik Sep 15, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants