[patch] Drop a reused handler's leftover partial line at the start of each run

RunCommand.Test/RunCommandTests.cs

@@ -1216,4 +1216,117 @@
 			File.Delete(second);
 		}
 	}
+
+	[TestMethod]
+	public async Task ReusedLineOutputHandlerShouldDropTheLineACancelledRunLeftUnfinished()
+	{
+		if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
+		{
+			Assert.Inconclusive("Needs sh to print a line without a line break. The reset this covers is in platform independent code, so the other legs cover it.");
+		}
+
+		List<string> lines = [];
+		using SemaphoreSlim partialArrived = new(0);
+		LineOutputHandler handler = new(lines.Add);
+
+		using (CancellationTokenSource cancellationTokenSource = new())
+		{
+			Task<int> cancelled = RunCommand.ExecuteAsync(
+				"sh",
+				["-c", "printf partial; sleep 30"],
+				new SignallingLineOutputHandler(handler, partialArrived),
+				cancellationTokenSource.Token);
+
+			Assert.IsTrue(await partialArrived.WaitAsync(TimeSpan.FromSeconds(10)).ConfigureAwait(false), "Expected the partial line to arrive.");
+			await cancellationTokenSource.CancelAsync().ConfigureAwait(false);
+			await Assert.ThrowsAsync<OperationCanceledException>(() => cancelled).ConfigureAwait(false);
+		}
+
+		Assert.AreEqual("partial", handler.outputBuffer.ToString(), "The cancelled run should have left its partial line buffered.");
+
+		int exitCode = await RunCommand.ExecuteAsync("sh", ["-c", "echo hello"], handler).ConfigureAwait(false);
+
+		Assert.AreEqual(0, exitCode);
+		Assert.AreEqual("hello", string.Join(" | ", lines), "The next run's first line should not carry the cancelled run's leftover text.");
+	}
+
+	[TestMethod]
+	public async Task ReusedLineOutputHandlerShouldDropTheLineARunWhoseCallbackThrewLeftUnfinished()
+	{
+		if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
+		{
+			Assert.Inconclusive("Needs sh to print a line without a line break. The reset this covers is in platform independent code, so the other legs cover it.");
+		}
+
+		List<string> lines = [];
+		bool throwOnError = true;
+		LineOutputHandler handler = new(
+			lines.Add,
+			line =>
+			{
+				if (throwOnError)
+				{
+					throw new InvalidOperationException("handler failed");
+				}
+			});
+
+		// Standard output's partial line is buffered before standard error's line makes the callback throw.
+		await Assert.ThrowsAsync<InvalidOperationException>(
+			() => RunCommand.ExecuteAsync("sh", ["-c", "printf partial; sleep 0.5; echo boom >&2; sleep 30"], handler)).ConfigureAwait(false);
+
+		Assert.AreEqual("partial", handler.outputBuffer.ToString(), "The failed run should have left its partial line buffered.");
+
+		throwOnError = false;
+		int exitCode = await RunCommand.ExecuteAsync("sh", ["-c", "echo hello"], handler).ConfigureAwait(false);
+
+		Assert.AreEqual(0, exitCode);
+		Assert.AreEqual("hello", string.Join(" | ", lines), "The next run's first line should not carry the failed run's leftover text.");
+	}
+
+	[TestMethod]
+	public async Task AbandonedReaderShouldNotDeliverIntoALaterRunOnTheSameHandler()
+	{
+		if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
+		{
+			Assert.Inconclusive("Needs a shell that can orphan a child out of its own process tree while that child keeps the pipe it inherited. That a cancelled run's reads stop delivering is platform independent, so the other legs cover it.");
+		}
+
+		List<string> lines = [];
+		using SemaphoreSlim partialArrived = new(0);
+		LineOutputHandler handler = new(lines.Add);
+
+		// The orphaned subshell escapes the kill and keeps the cancelled run's standard output open, then
+		// writes to it a second later, while the next run on the same handler is still going. The
+		// cancelled run gave up on that read, so what arrives there must not reach the handler.
+		using (CancellationTokenSource cancellationTokenSource = new())
+		{
+			Task<int> cancelled = RunCommand.ExecuteAsync(
+				"sh",
+				["-c", "sh -c '(sleep 1; printf late) &'; printf partial; sleep 30"],
+				new SignallingLineOutputHandler(handler, partialArrived),
+				cancellationTokenSource.Token);
+
+			Assert.IsTrue(await partialArrived.WaitAsync(TimeSpan.FromSeconds(10)).ConfigureAwait(false), "Expected the partial line to arrive.");
+			await cancellationTokenSource.CancelAsync().ConfigureAwait(false);
+			await Assert.ThrowsAsync<OperationCanceledException>(() => cancelled).ConfigureAwait(false);
+		}
+
+		int exitCode = await RunCommand.ExecuteAsync("sh", ["-c", "sleep 2; echo hello"], handler).ConfigureAwait(false);
+
+		Assert.AreEqual(0, exitCode);
+		Assert.AreEqual("hello", string.Join(" | ", lines), "Output the cancelled run's abandoned reader received should not reach the later run.");
+	}
+
+	/// <summary>
+	/// Forwards standard output to <paramref name="inner"/> and signals <paramref name="received"/> after each chunk,
+	/// so a test can wait for output to arrive before it cancels.
+	/// </summary>
+	private sealed class SignallingLineOutputHandler(LineOutputHandler inner, SemaphoreSlim received) : OutputHandler
+	{
+		internal override void HandleStandardOutputData(string data)
+		{
+			inner.HandleStandardOutputData(data);
+			received.Release();
+		}
+	}
 }

RunCommand/LineOutputHandler.cs

@@ -13,12 +13,17 @@ public class LineOutputHandler : OutputHandler
 	/// <summary>
 	/// Buffer to store incomplete lines from standard output.
 	/// </summary>
-	internal readonly StringBuilder outputBuffer = new();
+	/// <remarks>
+	/// Replaced rather than cleared by <see cref="Reset"/>, so a delivery still in progress from a
+	/// cancelled run finishes into the old buffer instead of the new run's.
+	/// </remarks>
+	internal StringBuilder outputBuffer = new();
 
 	/// <summary>
 	/// Buffer to store incomplete lines from standard error.
 	/// </summary>
-	internal readonly StringBuilder errorBuffer = new();
+	/// <remarks>See <see cref="outputBuffer"/>.</remarks>
+	internal StringBuilder errorBuffer = new();
 
 	/// <summary>
 	/// Initializes a new instance of the <see cref="LineOutputHandler"/> class.
@@ -62,6 +67,16 @@ internal override void Complete()
 		FlushBuffer(errorBuffer, OnStandardError);
 	}
 
+	/// <summary>
+	/// Discards any partial line, including a CR still waiting to see whether an LF follows, that a
+	/// cancelled or failed run left in the buffers.
+	/// </summary>
+	internal override void Reset()
+	{
+		outputBuffer = new();
+		errorBuffer = new();
+	}
+
 	/// <summary>
 	/// Invokes <paramref name="onLineReceived"/> with the buffered final line, if there is one, and clears the buffer.
 	/// </summary>

RunCommand/OutputHandler.cs

@@ -66,4 +66,11 @@ internal virtual void HandleStandardErrorData(string data)
 	/// so a handler that holds back partial data can deliver it. Not called when the run is cancelled.
 	/// </summary>
 	internal virtual void Complete() { }
+
+	/// <summary>
+	/// Called at the start of every run, so a handler that holds back partial data can discard what an
+	/// earlier run left behind. A run that was cancelled or failed never reaches <see cref="Complete"/>,
+	/// and its leftover text is not a real line, so it is dropped rather than delivered.
+	/// </summary>
+	internal virtual void Reset() { }
 }

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);
@@ -460,6 +460,10 @@
 	{
 		cancellationToken.ThrowIfCancellationRequested();
 
+		// A handler can be reused, and a run that was cancelled or failed never reached Complete, so
+		// whatever partial line it left behind would otherwise be glued onto this run's first line.
+		outputHandler.Reset();
+
 		using Process process = new() { StartInfo = startInfo };
 
 		process.Start();