Skip to content

Split each run's output into lines in a handler of its own [patch] - #132

Merged
matt-edmondson merged 1 commit into
mainfrom
fix/line-handler-per-run-state
Nov 4, 2026
Merged

matt-edmondson merged 1 commit into
mainfrom
fix/line-handler-per-run-state

Conversation

@matt-edmondson

Copy link
Copy Markdown
Contributor

A LineOutputHandler kept its line-splitting state, outputBuffer and errorBuffer, on the instance. When two runs shared one handler and overlapped:

  • The second run's Reset() threw away the first run's partial line.
  • Both runs then appended to the same StringBuilder from different threads. Their text came out joined ("BBBBAAAA" | "BBBB"), and the unsynchronised writes could corrupt the builder.

This takes the per-run fix the issue prefers:

  • OutputHandler gains internal virtual OutputHandler CreateRunHandler() => this.
  • LineOutputHandler overrides it to return a new LineOutputHandler with the same delegates and encoding, and empty buffers.
  • RunAsync feeds each run to outputHandler.CreateRunHandler(), so every run has its own line state and nothing one run leaves behind reaches another.

Reset() is still called on the run handler, so a stateful handler that returns itself keeps today's behaviour. The public API is unchanged.

The delegates are still serialised per run, by the existing deliveryLock, not across runs. I chose not to take a handler-wide lock: a delegate that starts a nested run on the same handler would deadlock on it. OutputHandler's remarks now say the never-at-the-same-time guarantee covers one run, and that state shared between concurrent runs needs its own lock, as the repro's lock (lines) already does.

Tests

  • New LineOutputHandlerSharedByOverlappingRunsShouldKeepEachRunsLinesApart: run A prints AAAA, waits, then finishes its line. Run B starts on the same handler while A is mid-line. The test expects exactly AAAAAAAA and BBBBBBBB.
    • With the library change stashed, it fails.
    • With the change, it passed 5 out of 5 repeated runs.
  • ReusedLineOutputHandlerShouldDropTheLineARunWhoseCallbackThrewLeftUnfinished had a precondition that the failed run's partial line sat in the shared handler's buffer. That partial line now lives in the run's own handler, so the precondition now asserts that the shared buffer is empty. The test's actual check, that the next run starts clean, is unchanged.
  • Full suite: 67 tests, all pass (2 skipped as before, elevation-only). Release build succeeds for every target framework.

Related: #129 (bare CR delivery) touches LineOutputHandler buffering too, but in ProcessDataByLine, not where the buffers are owned. Whichever lands second should rebase cleanly. #125 is a separate cancellation-timing issue and is not addressed here.

Fixes #130


Generated by Claude Code

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 #130

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017vfJW4ekcBueRdQ8TsyWTk
@sonarqubecloud

sonarqubecloud Bot commented Nov 1, 2026

Copy link
Copy Markdown

@matt-edmondson
matt-edmondson merged commit b8d0e00 into main Nov 4, 2026
15 checks passed
@matt-edmondson
matt-edmondson deleted the fix/line-handler-per-run-state branch November 4, 2026 00:55
matt-edmondson pushed a commit that referenced this pull request Nov 4, 2026
Resolves the conflict in LineOutputHandler.cs: keeps this branch's
immediate CR delivery with its per-stream skip-leading-LF state, and takes
main's CreateRunHandler (#132), its class remarks, and its narrowing of
IsLineBreak to LF only (#136).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017vfJW24pNCmCLDhz6z3ZdY
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

One LineOutputHandler shared by two concurrent ExecuteAsync runs drops one run's partial line and glues the two runs' output into one line

2 participants