From f2089831fa684f1626ae903134dee6f16880107c Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 00:35:11 +0000 Subject: [PATCH] Split each run's output into lines in a handler of its own A LineOutputHandler shared by two overlapping ExecuteAsync runs kept one pair of line buffers for both: the second run's Reset discarded the first run's partial line, and both runs then appended to the same StringBuilder from different threads, joining their text into one line. RunAsync now asks the handler for a per-run handler (CreateRunHandler); LineOutputHandler returns a fresh one with the same delegates and encoding, so each run's line state is its own. Fixes ktsu-dev/RunCommand#130 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017vfJW4ekcBueRdQ8TsyWTk --- RunCommand.Test/RunCommandTests.cs | 45 +++++++++++++++++++++++++++++- RunCommand/LineOutputHandler.cs | 13 +++++++++ RunCommand/OutputHandler.cs | 11 ++++++++ RunCommand/RunCommand.cs | 6 ++-- 4 files changed, 72 insertions(+), 3 deletions(-) diff --git a/RunCommand.Test/RunCommandTests.cs b/RunCommand.Test/RunCommandTests.cs index 64562b3..dee9663 100644 --- a/RunCommand.Test/RunCommandTests.cs +++ b/RunCommand.Test/RunCommandTests.cs @@ -1421,6 +1421,48 @@ public async Task ReusedLineOutputHandlerShouldDropTheLineACancelledRunLeftUnfin 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 LineOutputHandlerSharedByOverlappingRunsShouldKeepEachRunsLinesApart() + { + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + Assert.Inconclusive("Needs sh to print part of a line, pause, then finish it. The per-run splitting this covers is platform independent, so the other legs cover it."); + } + + // ktsu-dev/RunCommand#130: the second run's Reset threw away the first run's partial line, and + // both runs then appended to the same buffer, so their text came out joined into one line. + List lines = []; + using SemaphoreSlim firstPartArrived = new(0); + LineOutputHandler handler = new(line => + { + lock (lines) + { + lines.Add(line); + } + }); + + Task first = RunCommand.ExecuteAsync( + "sh", + ["-c", "printf AAAA; sleep 1; printf 'AAAA\\n'"], + new SignallingLineOutputHandler(handler, firstPartArrived)); + + Assert.IsTrue(await firstPartArrived.WaitAsync(TimeSpan.FromSeconds(10)).ConfigureAwait(false), "Expected the first run's partial line to arrive."); + + Task second = RunCommand.ExecuteAsync("sh", ["-c", "printf BBBB; sleep 0.2; printf 'BBBB\\n'"], handler); + + int[] exitCodes = await Task.WhenAll(first, second).ConfigureAwait(false); + + Assert.AreEqual(0, exitCodes[0], "The first run should succeed."); + Assert.AreEqual(0, exitCodes[1], "The second run should succeed."); + List sorted; + lock (lines) + { + sorted = [.. lines.Order(StringComparer.Ordinal)]; + } + + Assert.AreEqual("AAAAAAAA | BBBBBBBB", string.Join(" | ", sorted)); + } + [TestMethod] public async Task ReusedLineOutputHandlerShouldDropTheLineARunWhoseCallbackThrewLeftUnfinished() { @@ -1445,7 +1487,8 @@ public async Task ReusedLineOutputHandlerShouldDropTheLineARunWhoseCallbackThrew await Assert.ThrowsAsync( () => 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."); + // The failed run's partial line stays in that run's own handler (#130), so nothing is left on this one. + Assert.AreEqual(string.Empty, handler.outputBuffer.ToString(), "A run should split lines in its own handler, not the shared one."); throwOnError = false; int exitCode = await RunCommand.ExecuteAsync("sh", ["-c", "echo hello"], handler).ConfigureAwait(false); diff --git a/RunCommand/LineOutputHandler.cs b/RunCommand/LineOutputHandler.cs index d04223a..410cb0b 100644 --- a/RunCommand/LineOutputHandler.cs +++ b/RunCommand/LineOutputHandler.cs @@ -8,6 +8,10 @@ namespace ktsu.RunCommand; /// Handles the output from a command process, processing both standard output and standard error streams. /// It processes the data line-by-line, calling the specified actions for each line received. /// +/// +/// One handler can serve several runs at once. Each run splits its own output into lines, so a partial +/// line from one run is never joined to another run's output. +/// public class LineOutputHandler : OutputHandler { /// @@ -77,6 +81,15 @@ internal override void Reset() errorBuffer = new(); } + /// + /// Returns a new handler with the same delegates and encoding and empty buffers, so each run splits + /// its own output into lines. Two runs sharing this handler's buffers lost one run's partial line and + /// joined the two runs' text into one line (ktsu-dev/RunCommand#130). + /// + /// A handler for a single run. + internal override OutputHandler CreateRunHandler() => + new LineOutputHandler(OnStandardOutput, OnStandardError, Encoding); + /// /// Invokes with the buffered final line, if there is one, and clears the buffer. /// diff --git a/RunCommand/OutputHandler.cs b/RunCommand/OutputHandler.cs index 647835e..339259f 100644 --- a/RunCommand/OutputHandler.cs +++ b/RunCommand/OutputHandler.cs @@ -12,6 +12,9 @@ namespace ktsu.RunCommand; /// The two streams are read concurrently, but the actions are never run at the same time: each one /// returns before the other is called, so both may share state, such as one list collecting all of a /// command's output, without locking. Each stream's chunks arrive in the order they were written. +/// That guarantee covers one run. A handler passed to several runs at once, for example under +/// , may have its actions called by those runs at the same time, +/// so state shared between them needs its own lock. /// public class OutputHandler { @@ -78,4 +81,12 @@ internal virtual void Complete() { } /// and its leftover text is not a real line, so it is dropped rather than delivered. /// internal virtual void Reset() { } + + /// + /// Returns the handler that one run feeds its output to. A handler that keeps per-run state, such as a + /// partial line, returns a fresh handler sharing its delegates and encoding, so runs that overlap on + /// this handler each keep their own state. A stateless handler returns itself. + /// + /// The handler for a single run. + internal virtual OutputHandler CreateRunHandler() => this; } diff --git a/RunCommand/RunCommand.cs b/RunCommand/RunCommand.cs index 9d41adb..85c9e3f 100644 --- a/RunCommand/RunCommand.cs +++ b/RunCommand/RunCommand.cs @@ -460,8 +460,10 @@ private static async Task RunAsync(ProcessStartInfo startInfo, OutputHandle { 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. + // A handler can be reused, including by runs that overlap, so each run feeds a handler of its + // own: a partial line one run leaves behind, whether it was cancelled, failed or is still going, + // is never glued onto another run's output. Reset covers a stateful handler that returns itself. + outputHandler = outputHandler.CreateRunHandler(); outputHandler.Reset(); using Process process = new() { StartInfo = startInfo };