Fix Process.Kill(entireProcessTree: true) when intermediate child has KillOnParentExit

[Copilot]

The newly added test discovered a bug on Linux where Process.Kill(entireProcessTree: true) fails to kill grandchild processes when an intermediate child has PR_SET_PDEATHSIG set (exposed via ProcessStartInfo.KillOnParentExit). Killing the root immediately terminates the intermediate child (via parent death signal) before the tree traversal can discover its descendants.

The test passes on Windows without any code changes — on Windows every process inherits Job objects from the parent process (unless they explicitly escape the Job), so the grandchild is already part of the same Job and gets terminated along with its parent. The bug described in issue #107992 could not be reproduced.

Changes

Fix: Unix KillTree in Process.Unix.cs

Refactored the KillTree method to separate tree collection from killing into two phases:

1. StopTree: Recursively SIGSTOPs all processes depth-first and collects them into a flat list. This captures the entire tree while all processes are alive and stopped. Returns true if the process was successfully stopped and added to the list, false if it exited early or SIGSTOP failed (in which case the caller disposes it).
2. Kill phase: SIGKILLs all collected (already stopped) processes, inside a finally block so it always runs even if StopTree throws partway through.

Previously, the algorithm killed each process immediately after enumerating its direct children. This caused processes with PR_SET_PDEATHSIG (KillOnParentExit) to be killed by the kernel before their own children could be discovered, leaving grandchild processes orphaned and running.

Test: Regression test in KillOnParentExitTests.cs

Adds a [ConditionalTheory] parameterized with [InlineData(true)] and [InlineData(false)] that creates a 3-level process hierarchy:

Process A (root)
  └── Process B (KillOnParentExit=enabled, controlled by parameter)
        └── Process C (not in job object, should be killed)

• Calls Kill(entireProcessTree: true) on Process A
• Asserts Process C is also terminated
• Verifies behavior regardless of whether KillOnParentExit is enabled on the intermediate child

[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.

[adamsitnik]

@copilot please apply my suggestions

[Copilot]

@copilot please apply my suggestions

Done — removed both [PlatformSpecific(TestPlatforms.Windows)] and [ActiveIssue(...)] in commit b84f84c.

[adamsitnik]

@copilot please address my feedback

[Copilot]

@copilot please address my feedback

Converted to [ConditionalTheory] with [InlineData(true)] and [InlineData(false)] — the enabled parameter controls KillOnParentExit on the intermediate child process, verifying that Kill(entireProcessTree: true) kills the entire tree in both cases. Done in commit e5c4476.

[Copilot]

@copilot the new test has exposed a bug on Unix. Since you can run the tests locally on Linux, you should be able to reproduce it:

Fixed in the latest commit. The issue was that the original KillTree algorithm killed each process immediately after enumerating its children — when Process A was killed, PR_SET_PDEATHSIG caused Process B to die before we could recurse into it and discover Process C.

The fix separates tree collection from killing:

1. StopTree: recursively SIGSTOPs all processes depth-first and collects them into a flat list
2. KillTree: SIGKILLs all collected processes

This ensures the entire tree is discovered while all processes are stopped, before any kill signals trigger cascading deaths. All 12 KillOnParentExitTests pass on Linux including both enabled: true and enabled: false cases.

[adamsitnik]

@copilot please address my feedback, push the changes after ensuring all process tests are passing locally and THEN invoke the code-review skill and post your analysis/results as a comment on this PR

[Copilot]

@copilot please address my feedback, push the changes after ensuring all process tests are passing locally and THEN invoke the code-review skill and post your analysis/results as a comment on this PR

Applied both suggestions in commit aaddffb. All process tests pass locally:

• KillOnParentExitTests: 12 tests, 0 failures
• ProcessTests: 226 tests, 0 failures

---

🤖 Copilot Code Review — PR #128598

Holistic Assessment

Motivation: The fix addresses a real bug where Kill(entireProcessTree: true) fails to kill grandchild processes when an intermediate child uses PR_SET_PDEATHSIG (KillOnParentExit). The test exposes this reliably.

Approach: Separating tree enumeration (SIGSTOP + collect) from killing (SIGKILL all) is the correct fix — it prevents cascading PDEATHSIG deaths from hiding descendants.

Summary: ✅ LGTM. The Unix fix is sound, the test is well-structured, and both enabled: true/false cases are covered.

---

Detailed Findings

✅ Correctness — Two-phase stop-then-kill approach

The refactoring correctly addresses the root cause: killing a parent triggers PDEATHSIG on children before they can be enumerated. By SIGSTOPping the entire tree first (depth-first), then SIGKILLing all collected processes, the algorithm guarantees the full tree is discovered. SIGSTOP doesn't trigger PDEATHSIG, so the tree remains stable during enumeration.

✅ Error handling — Explicit ErrorInfo usage

Using GetLastErrorInfo() and passing RawErrno to Win32Exception is more robust than relying on Marshal.GetLastPInvokeError() surviving through list allocation. Applied consistently in both StopTree and the kill loop.

✅ Resource management — Dispose pattern

Child processes are properly disposed in the kill loop (process != this check avoids double-dispose of the root). The StopTree method correctly avoids disposing since processes are still needed for the kill phase.

✅ Test quality

The test creates a real 3-level process hierarchy, verifies both KillOnParentExit states, uses appropriate timeouts, and has a finally block to clean up the grandchild if the assertion fails.

⚠️ Windows parity — Follow-up needed (non-blocking)

The Windows KillTree still has the original bug described in issue #107992 (kills Process A first, then enumerates children of intermediate Process B which may already be dead from job object termination). This is out of scope for this PR but should be tracked separately.

💡 Minor observation — GetChildProcesses() allocation in StopTree

GetChildProcesses() calls GetProcesses() which allocates a snapshot of all processes. For deep trees, this happens at each level. This is the existing behavior and not a regression — just noting it's unchanged.

[adamsitnik]

⚠️ Windows parity — Follow-up needed (non-blocking)

The Windows KillTree still has the original bug described in issue #107992 (kills Process A first, then enumerates children of intermediate Process B which may already be dead from job object termination). This is out of scope for this PR but should be tracked separately.

Process.Kill(entireProcessTree: true) fails to kill grandchild processes when an intermediate child is in a job object with JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE (Windows) or has PR_SET_PDEATHSIG set (Unix, exposed via ProcessStartInfo.KillOnParentExit). Killing the root immediately terminates the intermediate child (via job object handle close on Windows, or parent death signal on Unix) before the tree traversal can discover its descendants.

This is not true. The test that we have added here is passing on Windows, which means that we can't reproduce bug described in issue #107992. It's because on Windows every process inherits Job objects from the parent process, unless they escape the Job.

@copilot please update the PR description to reflect the fact that the newly added test has discovered a bug on Linux (which also got fixed here) and was not able to reproduce the bug reported in #107992 because the Job got inherited by grand child process. Do not modify the source code.

[Copilot]

⚠️ Windows parity — Follow-up needed (non-blocking)

The Windows KillTree still has the original bug described in issue #107992 (kills Process A first, then enumerates children of inte...

Updated the PR description to reflect that:

1. The newly added test discovered a bug on Linux (fixed in this PR)
2. The test passes on Windows without code changes because Job objects are inherited by grandchild processes, meaning the bug described in System.Diagnostics.Process.Kill(entireProcessTree: true) doesn't kill entire tree if an intermediate child process is in job object with JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE #107992 could not be reproduced

[Copilot]

Pull request overview

This PR adjusts Unix process-tree termination in System.Diagnostics.Process to improve correctness when Kill(entireProcessTree: true) is used in the presence of intermediate processes that may terminate immediately on parent death (e.g., KillOnParentExit / parent-death signal scenarios). It also adds a regression test that builds a 3-level hierarchy and asserts the grandchild is terminated.

Changes:

• Refactors Unix KillTree to collect (and stop) the full process tree before issuing SIGKILL.
• Adds a regression test covering a root → child (optionally KillOnParentExit) → grandchild hierarchy.
• Improves errno propagation in the Unix tree-kill path by using GetLastErrorInfo().RawErrno when recording exceptions.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs	Refactors Unix KillTree into a stop/collect phase followed by a kill phase to avoid missing descendants.
src/libraries/System.Diagnostics.Process/tests/KillOnParentExitTests.cs	Adds a regression test to validate killing a tree also terminates a grandchild when the intermediate child may be killed on parent exit.

[adamsitnik]

@copilot please address the feedback

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.