Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
78 changes: 78 additions & 0 deletions RunCommand.Test/RunCommandTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,84 @@ 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();

bool ended = SpinWait.SpinUntil(() => !IsRunning(grandchild), TimeSpan.FromSeconds(5));

Assert.IsTrue(ended, "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 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<InvalidOperationException>(() => RunCommand.KillEntireTreeWhereSupported(neverStarted));
}

[TestMethod]
public async Task CancelShouldNotRunTheCallersContinuationInline()
{
Expand Down
53 changes: 50 additions & 3 deletions RunCommand/RunCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
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
Expand All @@ -24,7 +26,7 @@
/// </summary>
/// <param name="command">The command to execute.</param>
/// <returns>The exit code of the executed process.</returns>
[Obsolete("A command string is split on its first space, which cannot handle an executable path "

Check warning on line 29 in RunCommand/RunCommand.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Do not forget to remove this deprecated code someday.
+ "containing spaces. Use the overload taking a file name and an argument list instead.")]
public static int Execute(string command) =>
ExecuteAsync(command).Result;
Expand All @@ -35,7 +37,7 @@
/// <param name="command">The command to execute.</param>
/// <param name="outputHandler">The handler for processing command output.</param>
/// <returns>The exit code of the executed process.</returns>
[Obsolete("A command string is split on its first space, which cannot handle an executable path "

Check warning on line 40 in RunCommand/RunCommand.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Do not forget to remove this deprecated code someday.
+ "containing spaces. Use the overload taking a file name and an argument list instead.")]
public static int Execute(string command, OutputHandler outputHandler) =>
ExecuteAsync(command, outputHandler).Result;
Expand All @@ -46,7 +48,7 @@
/// <param name="command">The command to execute.</param>
/// <param name="elevation">The privilege level under which to run the command.</param>
/// <returns>The exit code of the executed process.</returns>
[Obsolete("A command string is split on its first space, which cannot handle an executable path "

Check warning on line 51 in RunCommand/RunCommand.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Do not forget to remove this deprecated code someday.
+ "containing spaces. Use the overload taking a file name and an argument list instead.")]
public static int Execute(string command, Elevation elevation) =>
ExecuteAsync(command, elevation).Result;
Expand All @@ -62,7 +64,7 @@
/// </param>
/// <param name="elevation">The privilege level under which to run the command.</param>
/// <returns>The exit code of the executed process.</returns>
[Obsolete("A command string is split on its first space, which cannot handle an executable path "

Check warning on line 67 in RunCommand/RunCommand.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Do not forget to remove this deprecated code someday.
+ "containing spaces. Use the overload taking a file name and an argument list instead.")]
public static int Execute(string command, OutputHandler outputHandler, Elevation elevation) =>
ExecuteAsync(command, outputHandler, elevation).Result;
Expand Down Expand Up @@ -109,7 +111,7 @@
/// </summary>
/// <param name="command">The command to execute.</param>
/// <returns>A task representing the asynchronous operation with the process exit code.</returns>
[Obsolete("A command string is split on its first space, which cannot handle an executable path "

Check warning on line 114 in RunCommand/RunCommand.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Do not forget to remove this deprecated code someday.
+ "containing spaces. Use the overload taking a file name and an argument list instead.")]
public static async Task<int> ExecuteAsync(string command)
=> await ExecuteAsync(command, new OutputHandler()).ConfigureAwait(false);
Expand All @@ -120,7 +122,7 @@
/// <param name="command">The command to execute.</param>
/// <param name="outputHandler">The handler for processing command output.</param>
/// <returns>A task representing the asynchronous operation with the process exit code.</returns>
[Obsolete("A command string is split on its first space, which cannot handle an executable path "

Check warning on line 125 in RunCommand/RunCommand.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Do not forget to remove this deprecated code someday.
+ "containing spaces. Use the overload taking a file name and an argument list instead.")]
public static async Task<int> ExecuteAsync(string command, OutputHandler outputHandler)
=> await ExecuteAsync(command, outputHandler, Elevation.Default).ConfigureAwait(false);
Expand All @@ -131,7 +133,7 @@
/// <param name="command">The command to execute.</param>
/// <param name="elevation">The privilege level under which to run the command.</param>
/// <returns>A task representing the asynchronous operation with the process exit code.</returns>
[Obsolete("A command string is split on its first space, which cannot handle an executable path "

Check warning on line 136 in RunCommand/RunCommand.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Do not forget to remove this deprecated code someday.
+ "containing spaces. Use the overload taking a file name and an argument list instead.")]
public static async Task<int> ExecuteAsync(string command, Elevation elevation)
=> await ExecuteAsync(command, new(), elevation).ConfigureAwait(false);
Expand All @@ -147,7 +149,7 @@
/// </param>
/// <param name="elevation">The privilege level under which to run the command.</param>
/// <returns>A task representing the asynchronous operation with the process exit code.</returns>
[Obsolete("A command string is split on its first space, which cannot handle an executable path "

Check warning on line 152 in RunCommand/RunCommand.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Do not forget to remove this deprecated code someday.
+ "containing spaces. Use the overload taking a file name and an argument list instead.")]
public static async Task<int> ExecuteAsync(string command, OutputHandler outputHandler, Elevation elevation)
=> await ExecuteAsync(command, outputHandler, elevation, CancellationToken.None).ConfigureAwait(false);
Expand All @@ -160,7 +162,7 @@
/// A token that, when cancelled, terminates the running process and its children.
/// </param>
/// <returns>A task representing the asynchronous operation with the process exit code.</returns>
[Obsolete("A command string is split on its first space, which cannot handle an executable path "

Check warning on line 165 in RunCommand/RunCommand.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Do not forget to remove this deprecated code someday.
+ "containing spaces. Use the overload taking a file name and an argument list instead.")]
public static async Task<int> ExecuteAsync(string command, CancellationToken cancellationToken)
=> await ExecuteAsync(command, new OutputHandler(), cancellationToken).ConfigureAwait(false);
Expand All @@ -175,7 +177,7 @@
/// A token that, when cancelled, terminates the running process and its children.
/// </param>
/// <returns>A task representing the asynchronous operation with the process exit code.</returns>
[Obsolete("A command string is split on its first space, which cannot handle an executable path "

Check warning on line 180 in RunCommand/RunCommand.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Do not forget to remove this deprecated code someday.
+ "containing spaces. Use the overload taking a file name and an argument list instead.")]
public static async Task<int> ExecuteAsync(string command, OutputHandler outputHandler, CancellationToken cancellationToken)
=> await ExecuteAsync(command, outputHandler, Elevation.Default, cancellationToken).ConfigureAwait(false);
Expand Down Expand Up @@ -727,10 +729,55 @@
}

#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

/// <summary>
/// <see cref="Process.Kill()"/>'s <c>entireProcessTree</c> overload, looked up at run time.
/// </summary>
private static readonly MethodInfo? KillEntireProcessTree = typeof(Process).GetMethod(nameof(Process.Kill), [typeof(bool)]);

/// <summary>
/// Kills <paramref name="process"/> and its descendants when the runtime can, and the process alone when it can't.
/// </summary>
/// <param name="process">The process to kill.</param>
/// <remarks>
/// The netstandard2.0/2.1 builds are what .NET Core 3.x and .NET 5–7 apps load, because no build targets those
/// runtimes. <c>Process.Kill(bool)</c> 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 <see cref="TryKill(Process, Action{Process})"/>
/// still treats them as best effort. Compiled on every target so the tests can reach it.
/// </remarks>
internal static void KillEntireTreeWhereSupported(Process process) =>
KillEntireTreeWhereSupported(process, KillEntireProcessTree);

/// <summary>
/// Kills <paramref name="process"/> through <paramref name="killEntireProcessTree"/>, or alone when it is
/// <see langword="null"/>.
/// </summary>
/// <param name="process">The process to kill.</param>
/// <param name="killEntireProcessTree">
/// <c>Process.Kill(bool)</c>, or <see langword="null"/> where the runtime lacks it. A parameter so tests can
/// stand in for .NET Framework and Mono.
/// </param>
internal static void KillEntireTreeWhereSupported(Process process, MethodInfo? killEntireProcessTree)
{
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();
}
}
}
Loading