[patch] Treat a partly failed process-tree kill as best effort, so Cancel() never throws

.gitignore

@@ -203,6 +203,11 @@ PublishScripts/
 **/[Pp]ackages/*
 # except build/, which is used as an MSBuild target.
 !**/[Pp]ackages/build/
+# and except a Unity project's Packages/, which is source: Unity's package manifest and its
+# resolved lock file are both meant to be committed, and a NuGet restore folder never contains
+# a file by either name.
+!**/[Pp]ackages/manifest.json
+!**/[Pp]ackages/packages-lock.json
 # Uncomment if necessary however generally it will be regenerated when needed
 #!**/[Pp]ackages/repositories.config
 # NuGet v3's project.json files produces more ignorable files
@@ -651,3 +656,16 @@ Temporary Items
 
 # ImGui.ini files
 imgui.ini
+
+# Game engine projects
+#
+# Godot: the import cache, and the mono/temp bin+obj a C# build writes.
+.godot/
+
+# Unity: .meta files are source, not the Visual Studio C++ build artifact that the `*.meta` rule
+# further up targets. Unity generates one per asset and it carries the GUID that scenes, prefabs
+# and serialized references point at, so ignoring them gives every clone fresh GUIDs and silently
+# breaks those references - including for a plug-in whose .dll is itself a build output. This
+# negation has to come after that rule to win, and is scoped to the asset tree so the Visual
+# Studio artifact stays ignored everywhere else.
+!**/[Aa]ssets/**/*.meta

RunCommand.Test/RunCommandTests.cs

@@ -450,6 +450,42 @@ public async Task ExecuteAsyncShouldTerminateProcessWhenCancelledWhileRunning()
 		await Assert.ThrowsAsync<OperationCanceledException>(() => execution).ConfigureAwait(false);
 	}
 
+	[TestMethod]
+	public void CancelShouldNotThrowWhenTheTreeKillCannotTerminateEveryDescendant()
+	{
+		(string fileName, string[] arguments) = GetSleepCommand();
+		using Process process = Process.Start(new ProcessStartInfo(fileName, arguments) { UseShellExecute = false })!;
+
+		try
+		{
+			// What Process.Kill(entireProcessTree: true) throws when a descendant belongs to another
+			// user or runs elevated.
+			static void PartlyFailingKill(Process _) =>
+				throw new AggregateException(
+					"Not all processes in the process tree could be terminated.",
+					new System.ComponentModel.Win32Exception(1, "Operation not permitted"));
+
+			// Registered the way RunAsync registers its kill, so this is the caller's Cancel().
+			using CancellationTokenSource cancellationTokenSource = new();
+			using CancellationTokenRegistration registration = cancellationTokenSource.Token.Register(
+				() => RunCommand.TryKill(process, PartlyFailingKill));
+
+			cancellationTokenSource.Cancel();
+			Assert.IsTrue(cancellationTokenSource.IsCancellationRequested, "Expected Cancel() to return normally after the kill failed.");
+
+			// Called directly as well, as the catch blocks in RunAsync do: an exception here would
+			// replace the OperationCanceledException or the handler failure being reported.
+			RunCommand.TryKill(process, PartlyFailingKill);
+
+			// Only the injected kill ran, so the process is still alive for the finally block to end.
+			Assert.IsFalse(process.HasExited, "Expected only the failing kill to have run.");
+		}
+		finally
+		{
+			process.Kill(entireProcessTree: true);
+		}
+	}
+
 	[TestMethod]
 	public async Task CancelShouldNotRunTheCallersContinuationInline()
 	{

RunCommand/RunCommand.cs

@@ -23,7 +23,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 "
 		+ "containing spaces. Use the overload taking a file name and an argument list instead.")]
 	public static int Execute(string command) =>
 		ExecuteAsync(command).Result;
@@ -34,7 +34,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 "
 		+ "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;
@@ -45,7 +45,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 "
 		+ "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;
@@ -61,7 +61,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 "
 		+ "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;
@@ -108,7 +108,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 "
 		+ "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);
@@ -533,19 +533,26 @@
 		return process.ExitCode;
 	}
 
-	private static void TryKill(Process process)
+	private static void TryKill(Process process) => TryKill(process, Kill);
+
+	/// <summary>
+	/// Kills <paramref name="process"/> through <paramref name="kill"/>, treating every way the kill
+	/// can fail as best effort.
+	/// </summary>
+	/// <remarks>
+	/// This runs inside the cancellation token's registration, so anything it throws comes out of
+	/// the caller's <see cref="CancellationTokenSource.Cancel()"/>, and it runs in catch blocks,
+	/// where anything it throws replaces the exception being reported. It must never throw.
+	/// </remarks>
+	/// <param name="process">The process to kill.</param>
+	/// <param name="kill">Performs the kill. A parameter so tests can make it fail.</param>
+	internal static void TryKill(Process process, Action<Process> kill)
 	{
 		try
 		{
 			if (!process.HasExited)
 			{
-#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.
-				process.Kill();
-#else
-				process.Kill(entireProcessTree: true);
-#endif
+				kill(process);
 			}
 		}
 		catch (InvalidOperationException)
@@ -560,5 +567,19 @@
 		{
 			// Terminating a remote process is not supported.
 		}
+		catch (AggregateException)
+		{
+			// Kill(entireProcessTree) reports descendants it could not terminate, such as ones owned
+			// by another user or running elevated, this way. It carries on past each failure, so
+			// everything it could kill has already been killed.
+		}
 	}
+
+#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();
+#else
+	private static void Kill(Process process) => process.Kill(entireProcessTree: true);
+#endif
 }