From 194a733c37a84f30914261200a1fab52786d2009 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 4 Nov 2026 15:30:58 +0000 Subject: [PATCH 1/3] Kill the whole process tree from the netstandard builds where the runtime can [patch] No build targets .NET Core 3.x or .NET 5-7, so apps on those runtimes load the netstandard2.1 build, which compiled Kill to a plain process.Kill(). A cancelled command's grandchildren (build servers, test hosts, npm children) kept running there, although README promised the whole tree is terminated on .NET Core 3.0 and later. Process.Kill(bool) exists on every one of those runtimes but not on the netstandard reference surface, so the netstandard builds now look it up once by reflection and call it, falling back to process.Kill() on .NET Framework and Mono. TargetInvocationException is unwrapped, so TryKill still treats the overload's AggregateException, InvalidOperationException and Win32Exception as best effort (#107). The README now says which runtimes kill the tree, and CLAUDE.md drops the net5.0-net7.0 targets it still listed and describes the reflection path. KillEntireTreeWhereSupportedShouldEndTheGrandchildrenToo starts a shell with a background sleep and checks the sleep is gone after the kill. The method is compiled on every target so the net10.0 tests reach it. Fixes #134 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017vfJW5DyciGEV4wZvik3pQ --- CLAUDE.md | 9 ++++-- README.md | 2 +- RunCommand.Test/RunCommandTests.cs | 50 ++++++++++++++++++++++++++++++ RunCommand/RunCommand.cs | 41 ++++++++++++++++++++++-- 4 files changed, 95 insertions(+), 7 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index ca78b82..0d9bdfa 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -46,7 +46,7 @@ output through delegates. The solution uses: - **ktsu.Sdk** - Custom SDK providing shared build configuration - **MSTest.Sdk** - Test project SDK with Microsoft Testing Platform -- Multi-targeting: `net10.0`, `net9.0`, `net8.0`, `net7.0`, `net6.0`, `net5.0`, `netstandard2.0`, `netstandard2.1` +- Multi-targeting: `net10.0`, `net9.0`, `net8.0`, `netstandard2.0`, `netstandard2.1` (set by ktsu.Sdk). Apps on .NET Core 3.x and .NET 5–7 load the `netstandard2.1` build The test project targets `net10.0` only. @@ -109,8 +109,11 @@ before returning. **Do not remove this** — without it a cancelled command is i a genuine failure of the underlying tool. The regression test repeats a 1 ms cancellation 50 times, because a single attempt still throws most of the time even when the bug is present. -Process-tree termination requires .NET Core 3.0 or later; the `netstandard2.0`/`netstandard2.1` -builds can only kill the process itself. +Process-tree termination requires .NET Core 3.0 or later. The `netstandard2.0`/`netstandard2.1` builds +find `Process.Kill(bool)` by reflection in `KillEntireTreeWhereSupported`, because those builds are +what .NET Core 3.x and .NET 5–7 apps load, and fall back to killing the process alone on .NET +Framework and Mono. The reflection call unwraps `TargetInvocationException` so `TryKill` still sees +the overload's own exceptions. ### Elevation constraints diff --git a/README.md b/README.md index d86b895..b30ec9f 100644 --- a/README.md +++ b/README.md @@ -179,7 +179,7 @@ class Program A cancelled call always throws `OperationCanceledException` — it never returns the killed process's exit code — so cancellation cannot be mistaken for a genuine failure of the command. -On .NET Core 3.0 and later the entire process tree is terminated. On .NET Standard 2.0 and 2.1 only the process itself can be terminated, so any grandchildren it spawned are left running. +On .NET Core 3.0 and later, including apps on .NET Core 3.x and .NET 5–7 that load the .NET Standard build, the entire process tree is terminated. On .NET Framework and Mono only the process itself can be terminated, so any grandchildren it spawned are left running. ## Process Options diff --git a/RunCommand.Test/RunCommandTests.cs b/RunCommand.Test/RunCommandTests.cs index 3a202a5..599511d 100644 --- a/RunCommand.Test/RunCommandTests.cs +++ b/RunCommand.Test/RunCommandTests.cs @@ -488,6 +488,56 @@ static void PartlyFailingKill(Process _) => } } + [TestMethod] + public void KillEntireTreeWhereSupportedShouldEndTheGrandchildrenToo() + { + SkipOnWindows(); + + // The netstandard builds, which .NET Core 3.x and .NET 5–7 apps load, kill through this method. Killing only + // the shell would leave its background sleep running (#134). + using Process process = Process.Start(new ProcessStartInfo("sh", ["-c", "sleep 300 & echo $!; wait"]) + { + UseShellExecute = false, + RedirectStandardOutput = true, + })!; + int grandchild = int.Parse(process.StandardOutput.ReadLine()!, System.Globalization.CultureInfo.InvariantCulture); + + try + { + RunCommand.KillEntireTreeWhereSupported(process); + process.WaitForExit(); + + Stopwatch stopwatch = Stopwatch.StartNew(); + while (IsRunning(grandchild) && stopwatch.Elapsed < TimeSpan.FromSeconds(5)) + { + Thread.Sleep(20); + } + + Assert.IsFalse(IsRunning(grandchild), "Expected the shell's background sleep to be killed with it."); + } + finally + { + if (IsRunning(grandchild)) + { + using Process leftover = Process.GetProcessById(grandchild); + leftover.Kill(); + } + } + + static bool IsRunning(int processId) + { + try + { + using Process candidate = Process.GetProcessById(processId); + return !candidate.HasExited; + } + catch (ArgumentException) + { + return false; + } + } + } + [TestMethod] public async Task CancelShouldNotRunTheCallersContinuationInline() { diff --git a/RunCommand/RunCommand.cs b/RunCommand/RunCommand.cs index 2e538d5..6202864 100644 --- a/RunCommand/RunCommand.cs +++ b/RunCommand/RunCommand.cs @@ -6,6 +6,8 @@ namespace ktsu.RunCommand; using System.Collections.Generic; using System.Diagnostics; using System.IO; +using System.Reflection; +using System.Runtime.ExceptionServices; using System.Runtime.InteropServices; using System.Threading; #if NETSTANDARD2_0 @@ -727,10 +729,43 @@ internal static void TryKill(Process process, Action kill) } #if NETSTANDARD2_0 || NETSTANDARD2_1 - // Process.Kill(bool) requires .NET Core 3.0 or later, so the older targets can only terminate - // the process itself and not any grandchildren it spawned. - private static void Kill(Process process) => process.Kill(); + private static void Kill(Process process) => KillEntireTreeWhereSupported(process); #else private static void Kill(Process process) => process.Kill(entireProcessTree: true); #endif + + /// + /// 's entireProcessTree overload, looked up at run time. + /// + private static readonly MethodInfo? KillEntireProcessTree = typeof(Process).GetMethod(nameof(Process.Kill), [typeof(bool)]); + + /// + /// Kills and its descendants when the runtime can, and the process alone when it can't. + /// + /// The process to kill. + /// + /// The netstandard2.0/2.1 builds are what .NET Core 3.x and .NET 5–7 apps load, because no build targets those + /// runtimes. Process.Kill(bool) exists on all of them, but the netstandard reference surface does not + /// declare it, so it is found by reflection: without it a cancelled command's grandchildren kept running there + /// (ktsu-dev/RunCommand#134). .NET Framework and Mono have no such overload and kill the process alone. + /// The overload's own exceptions are rethrown unwrapped, so + /// still treats them as best effort. Compiled on every target so the tests can reach it. + /// + internal static void KillEntireTreeWhereSupported(Process process) + { + if (KillEntireProcessTree is null) + { + process.Kill(); + return; + } + + try + { + _ = KillEntireProcessTree.Invoke(process, [true]); + } + catch (TargetInvocationException exception) when (exception.InnerException is not null) + { + ExceptionDispatchInfo.Capture(exception.InnerException).Throw(); + } + } } From 63d3cbd6fd22989658a96f05e4f435aed53c4724 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 4 Nov 2026 16:32:52 +0000 Subject: [PATCH 2/3] Test the netstandard kill's fallback and its exception unwrap KillEntireTreeWhereSupported's fallback for runtimes without Process.Kill(bool) and its TargetInvocationException unwrap had no test, so SonarCloud's new-code coverage was 50% (gate is 80%). The overload's MethodInfo is now a parameter of an internal overload, as TryKill's kill is. One test passes null, standing in for .NET Framework and Mono, and checks the process is still killed. The other kills a Process that was never started and checks the InvalidOperationException arrives unwrapped, so TryKill's catch blocks still see it; with the unwrap removed it gets a TargetInvocationException instead. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017vfJW5DyciGEV4wZvik3pQ --- RunCommand.Test/RunCommandTests.cs | 32 ++++++++++++++++++++++++++++++ RunCommand/RunCommand.cs | 18 ++++++++++++++--- 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/RunCommand.Test/RunCommandTests.cs b/RunCommand.Test/RunCommandTests.cs index 599511d..3ca1ce2 100644 --- a/RunCommand.Test/RunCommandTests.cs +++ b/RunCommand.Test/RunCommandTests.cs @@ -538,6 +538,38 @@ static bool IsRunning(int processId) } } + [TestMethod] + public void KillEntireTreeWhereSupportedShouldKillTheProcessAloneWithoutTheTreeOverload() + { + // .NET Framework and Mono have no Process.Kill(bool), so the process itself is still killed there. + (string fileName, string[] arguments) = GetSleepCommand(); + using Process process = Process.Start(new ProcessStartInfo(fileName, arguments) { UseShellExecute = false })!; + + try + { + RunCommand.KillEntireTreeWhereSupported(process, killEntireProcessTree: null); + + Assert.IsTrue(process.WaitForExit(10_000), "Expected the process to be killed without the tree overload."); + } + finally + { + if (!process.HasExited) + { + process.Kill(entireProcessTree: true); + } + } + } + + [TestMethod] + public void KillEntireTreeWhereSupportedShouldThrowTheOverloadsOwnExceptionUnwrapped() + { + // TryKill catches InvalidOperationException, Win32Exception and AggregateException by type, so a + // TargetInvocationException from the reflection call would escape it (#107). + using Process neverStarted = new(); + + _ = Assert.ThrowsExactly(() => RunCommand.KillEntireTreeWhereSupported(neverStarted)); + } + [TestMethod] public async Task CancelShouldNotRunTheCallersContinuationInline() { diff --git a/RunCommand/RunCommand.cs b/RunCommand/RunCommand.cs index 6202864..57211b6 100644 --- a/RunCommand/RunCommand.cs +++ b/RunCommand/RunCommand.cs @@ -751,9 +751,21 @@ internal static void TryKill(Process process, Action kill) /// The overload's own exceptions are rethrown unwrapped, so /// still treats them as best effort. Compiled on every target so the tests can reach it. /// - internal static void KillEntireTreeWhereSupported(Process process) + internal static void KillEntireTreeWhereSupported(Process process) => + KillEntireTreeWhereSupported(process, KillEntireProcessTree); + + /// + /// Kills through , or alone when it is + /// . + /// + /// The process to kill. + /// + /// Process.Kill(bool), or where the runtime lacks it. A parameter so tests can + /// stand in for .NET Framework and Mono. + /// + internal static void KillEntireTreeWhereSupported(Process process, MethodInfo? killEntireProcessTree) { - if (KillEntireProcessTree is null) + if (killEntireProcessTree is null) { process.Kill(); return; @@ -761,7 +773,7 @@ internal static void KillEntireTreeWhereSupported(Process process) try { - _ = KillEntireProcessTree.Invoke(process, [true]); + _ = killEntireProcessTree.Invoke(process, [true]); } catch (TargetInvocationException exception) when (exception.InnerException is not null) { From d13729a7cd50aed018ef6bf90ee73a8951ea357d Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 4 Nov 2026 16:46:19 +0000 Subject: [PATCH 3/3] Wait for the killed grandchild with SpinWait.SpinUntil instead of Thread.Sleep SonarCloud flagged the Thread.Sleep polling loop in KillEntireTreeWhereSupportedShouldEndTheGrandchildrenToo (S2925). SpinUntil keeps the same 5 s bound and asserts on its result. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017vfJW5DyciGEV4wZvik3pQ --- RunCommand.Test/RunCommandTests.cs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/RunCommand.Test/RunCommandTests.cs b/RunCommand.Test/RunCommandTests.cs index 3ca1ce2..aa7261f 100644 --- a/RunCommand.Test/RunCommandTests.cs +++ b/RunCommand.Test/RunCommandTests.cs @@ -507,13 +507,9 @@ public void KillEntireTreeWhereSupportedShouldEndTheGrandchildrenToo() RunCommand.KillEntireTreeWhereSupported(process); process.WaitForExit(); - Stopwatch stopwatch = Stopwatch.StartNew(); - while (IsRunning(grandchild) && stopwatch.Elapsed < TimeSpan.FromSeconds(5)) - { - Thread.Sleep(20); - } + bool ended = SpinWait.SpinUntil(() => !IsRunning(grandchild), TimeSpan.FromSeconds(5)); - Assert.IsFalse(IsRunning(grandchild), "Expected the shell's background sleep to be killed with it."); + Assert.IsTrue(ended, "Expected the shell's background sleep to be killed with it."); } finally {