Add ProcessOutputLine struct and Process.ReadAllLinesAsync

[Copilot]

Description

Implements ProcessOutputLine and Process.ReadAllLinesAsync from the approved API in #125838.

ReadAllLinesAsync streams stdout/stderr lines as an IAsyncEnumerable<ProcessOutputLine>, interleaving them as they become available. The implementation uses a Channel<ProcessOutputLine> with rendezvous semantics (capacity 0) to multiplex two concurrent StreamReader.ReadLineAsync tasks into a single async enumerable. The consumer reads via a WaitToReadAsync/TryRead loop for efficient channel consumption. If the consumer stops enumerating early (e.g., breaks out of await foreach), in-flight reads are canceled via a linked CancellationTokenSource in a try/finally block, and both reader tasks are awaited before the CTS is disposed.

using Process process = Process.Start(startInfo);
await foreach (ProcessOutputLine line in process.ReadAllLinesAsync())
{
    if (line.StandardError)
        Console.ForegroundColor = ConsoleColor.Red;
    Console.WriteLine(line.Content);
    Console.ResetColor();
}

Changes

• ProcessOutputLine.cs — new readonly struct with Content and StandardError properties; constructor throws ArgumentNullException when content is null
• Process.Multiplexing.cs — ReadAllLinesAsync implementation using a Channel-based approach: two ReadToChannelAsync local tasks read from stdout/stderr and write to a Channel.CreateBounded<ProcessOutputLine>(0) rendezvous channel, with a WaitToReadAsync/TryRead loop for consumption. Channel completion is tracked via Interlocked.Exchange on a bool firstCompleted flag — the second writer to finish completes the channel. A linked CancellationTokenSource with try/finally cancels (synchronously via Cancel()) and awaits both reader tasks on early enumeration stop before disposing the CTS. Removed unused System.Runtime.InteropServices using.
• System.Diagnostics.Process.cs (ref) — public API surface for ProcessOutputLine (with _dummy/_dummyPrimitive fields for proper struct layout) and ReadAllLinesAsync, sorted alphabetically per ref file conventions
• ProcessStreamingTests.cs — 11 test methods (17 cases) mirroring ProcessMultiplexingTests: dispose/redirect/mode validation, interleaved reads, large output, cancellation, property verification with Assert.Single, and early enumeration stop (ReadAllLinesAsync_StopsCleanlyWhenConsumerBreaksEarly). Structured for future ReadAllLines (sync) extension.
• csproj updates — registered new source and test files; added System.Threading.Channels project reference

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-diagnostics-process
See info in area-owners.md if you want to be subscribed.

[adamsitnik]

@copilot please address my feedback

[Copilot]

@copilot please address my feedback

Addressed both items in a29272a:

• ReadAllLinesAsync sorted alphabetically in the ref file
• Assert.Single used instead of Assert.Contains in the test
• Also fixed an IDE0059 analyzer error for the isError variable declaration

[adamsitnik]

@copilot please invoke the code-review skill and post your analysis/results as a comment on this PR

[Copilot]

Pull request overview

Adds a new line-oriented, streaming output API to System.Diagnostics.Process, enabling consumers to asynchronously iterate interleaved stdout/stderr lines via a lightweight ProcessOutputLine value type.

Changes:

• Introduces ProcessOutputLine (Content, StandardError) as a public readonly struct.
• Implements Process.ReadAllLinesAsync as an IAsyncEnumerable<ProcessOutputLine> in the multiplexing implementation.
• Adds a dedicated test suite covering validation, interleaving, large output, cancellation, and property semantics; registers new source/test files in csproj/ref.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj	Registers the new ProcessStreamingTests.cs test file for compilation.
src/libraries/System.Diagnostics.Process/tests/ProcessStreamingTests.cs	Adds test coverage for ReadAllLinesAsync behavior and argument/state validation.
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessOutputLine.cs	Adds the new public ProcessOutputLine struct used by the streaming API.
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.cs	Implements Process.ReadAllLinesAsync using concurrent ReadLineAsync + Task.WhenAny.
src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj	Registers ProcessOutputLine.cs in the library project.
src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs	Updates public API surface to include ProcessOutputLine and ReadAllLinesAsync.

[Copilot]

@copilot please invoke the code-review skill and post your analysis/results as a comment on this PR

🤖 Copilot Code Review — PR #126987

Holistic Assessment

Motivation: The PR implements ProcessOutputLine and Process.ReadAllLinesAsync from the approved API in #125838. The motivation is clear and well-justified — providing a deadlock-free way to interleave stdout/stderr line reading from child processes.

Approach: The implementation uses Task.WhenAny over concurrent StreamReader.ReadLineAsync calls with an inner loop optimization to drain buffered data synchronously. This is the standard pattern for multiplexing two async streams and is consistent with the existing ReadAllText/ReadAllBytes methods in the same file.

Summary: ⚠️ Needs Human Review. The implementation is largely correct and well-structured, but there are a few items that warrant maintainer attention around the isError priority logic semantics. No blocking bugs were found.

---

Detailed Findings

✅ API Compliance — Matches approved shape exactly

The implementation matches the approved API from #125838 (bartonjs's comment). All type names, method signatures, parameter names, and return types are correct. The [EnumeratorCancellation] attribute is correctly present in src but not ref (standard practice).

✅ Core Algorithm Correctness — Verified through manual trace

Traced through 6 scenarios:

1. Both streams have data ready: isError = true (stderr priority) — stderr consumed first, stdout's completed task handled on next iteration ✅
2. stderr returns null first: Drain correctly awaits readOutput, continues reading outputReader ✅
3. stdout returns null first: Drain correctly awaits readError, continues reading errorReader ✅
4. Both streams return null simultaneously: stderr null detected, drain awaits stdout which also returns null, loop ends ✅
5. Inner loop EOF: line becomes null, outer loop breaks, drain handles opposite stream's pending task ✅
6. Inner loop async exit: Pending ValueTask stored as appropriate task, returns to WhenAny ✅

The line variable reuse between inner/outer loops is intentional and correct — it signals EOF of the active stream to the outer loop.

✅ Drain Logic — Correct direction verified

When isError = true (stderr ended): drains readOutput → outputReader → remainingIsError = false ✅
When isError = false (stdout ended): drains readError → errorReader → remainingIsError = true ✅

⚠️ isError Priority — Semantic subtlety worth documenting

isError = completedTask == readError || (readOutput.IsCompleted && readError.IsCompleted);

When WhenAny returns readOutput but readError has also completed by that time, isError becomes true — overriding WhenAny's choice to prioritize stderr. Stdout's completed task remains unconsumed until the next iteration. The behavior is correct (no data loss), but this priority guarantee is non-obvious. A human reviewer should confirm this is the desired semantic.

⚠️ Task.WhenAny Allocation Per Iteration

Each outer loop iteration allocates via Task.WhenAny. The inner loop optimization mitigates this for buffered data. Acceptable for a process I/O API but worth noting.

✅ Ref File — Alphabetically sorted

ReadAllLinesAsync between ReadAllBytesAsync and ReadAllText ✅. ProcessOutputLine between ProcessExitStatus and ProcessPriorityClass ✅.

✅ Using Directives — Correctly updated

Removed unused System.Runtime.InteropServices (confirmed unused in base). Added System.Collections.Generic and System.Runtime.CompilerServices ✅.

✅ ProcessOutputLine Struct — Clean implementation

readonly struct, no null validation (consistent with ProcessExitStatus), XML docs on all members, parameter names match ref ✅.

✅ Test Coverage — Comprehensive (10 methods, 16 cases)

Covers: disposed process, no redirect, partial redirect, sync/async mode conflicts, both output+error (4 combos), interleaved output, large output (1000 lines), cancellation, property verification with Assert.Single.

💡 Potential future addition: asymmetric output test (stderr >> stdout) to exercise drain logic more.

💡 Minor — ValidateReadAllState sets SyncMode for async method

Consistent with ReadAllTextAsync/ReadAllBytesAsync. The mode flag prevents concurrent access from event-based APIs, not about sync/async distinction. Correct but could benefit from a clarifying comment.

---

Review performed with multi-model analysis (Gemini 2.5 Pro, Claude Sonnet 4). Drain logic and line variable reuse confirmed correct across all models.

[adamsitnik]

⚠️ isError Priority — Semantic subtlety worth documenting

isError = completedTask == readError || (readOutput.IsCompleted && readError.IsCompleted);

When WhenAny returns readOutput but readError has also completed by that time, isError becomes true — overriding WhenAny's choice to prioritize stderr. Stdout's completed task remains unconsumed until the next iteration. The behavior is correct (no data loss), but this priority guarantee is non-obvious. A human reviewer should confirm this is the desired semantic.

@copilot document this in the XML docs (this is on purpose)

[Copilot]

⚠️ isError Priority — Semantic subtlety worth documenting

isError = completedTask == readError || (readOutput.IsCompleted && readError.IsCompleted);...

Documented in a79f653 — added a <remarks> section to the ReadAllLinesAsync XML docs explaining that stderr is intentionally prioritized when both streams have data available simultaneously.

[adamsitnik]

LGTM.

I wish I could make the implementation simpler around cancelling the pending tasks when enumeration is stopped early, but I simple don't have any ideas right now.

[adamsitnik]

@copilot please address my feedback

[adamsitnik]

LGTM!