Fix ProcessRunner StringBuilder race condition causing assertion failure

[Copilot]

Description

Closes #124067

ProcessRunner's stdout and stderr handlers concurrently appended to a shared StringBuilder without synchronization, corrupting internal state and triggering assertion failure minBlockCharCount > 0 in ExpandByABlock.

Changes

• Lock on _outputCapture StringBuilder in both StandardOutputEventHandler and StandardErrorEventHandler
• No lock needed for ToString() call in IsAvailable - process has already exited, no more events will fire

private void StandardOutputEventHandler(object sender, DataReceivedEventArgs eventArgs)
{
    string data = eventArgs?.Data;
    if (!string.IsNullOrEmpty(data))
    {
        WriteLog(data);
        lock (_outputCapture)
        {
            _outputCapture.AppendLine("  " + data);
        }
    }
}

Original prompt

---

This section details on the original issue you should resolve

<issue_title>Assertion failed: minBlockCharCount > 0</issue_title>
<issue_description>Failed in: runtime-coreclr pgo 20260205.1

Failed tests:

coreclr linux arm Checked defaultpgo @ (Debian.13.Arm32.Open)AzureLinux.3.Arm64.Open@mcr.microsoft.com/dotnet-buildtools/prereqs:debian-13-helix-arm32v7
- readytorun/coreroot_determinism/coreroot_determinism/coreroot_determinism.cmd

Error message:

Process terminated.
Assertion failed.
minBlockCharCount > 0
   at System.Text.StringBuilder.ExpandByABlock(Int32 minBlockCharCount)
   at System.Text.StringBuilder.AppendWithExpansion(Char& value, Int32 valueCount)
   at System.Text.StringBuilder.Append(String value)
   at System.Text.StringBuilder.AppendLine(String value)
   at ProcessRunner.StandardErrorEventHandler(Object sender, DataReceivedEventArgs eventArgs) in /_/src/coreclr/tools/r2rtest/ProcessRunner.cs:line 297
   at System.Diagnostics.Process.ErrorReadNotifyUser(String data) in /_/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs:line 1731
   at System.Diagnostics.AsyncStreamReader.FlushMessageQueue(Boolean rethrowInNewThread) in /_/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/AsyncStreamReader.cs:line 237
   at System.Diagnostics.AsyncStreamReader.ReadBufferAsync() in /_/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/AsyncStreamReader.cs:line 123
   at System.Runtime.CompilerServices.AsyncTaskMethodBuilder`1.AsyncStateMachineBox`1.ExecutionContextCallback(Object s)
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
   at System.Runtime.CompilerServices.AsyncTaskMethodBuilder`1.AsyncStateMachineBox`1.MoveNext(Thread threadPoolThread)
   at System.Runtime.CompilerServices.AsyncTaskMethodBuilder`1.AsyncStateMachineBox`1.MoveNext()
   at System.Threading.Tasks.AwaitTaskContinuation.RunOrScheduleAction(IAsyncStateMachineBox box, Boolean allowInlining)
   at System.Threading.Tasks.Task.RunContinuations(Object continuationObject)
   at System.Threading.Tasks.Task`1.TrySetResult(TResult result)
   at System.Runtime.CompilerServices.AsyncTaskMethodBuilder`1.SetExistingTaskResult(Task`1 task, TResult result)
   at System.Runtime.CompilerServices.AsyncValueTaskMethodBuilder`1.SetResult(TResult result)
   at System.IO.Pipes.PipeStream.ReadAsyncCore(Memory`1 destination, CancellationToken cancellationToken) in /_/src/libraries/System.IO.Pipes/src/System/IO/Pipes/PipeStream.Unix.cs:line 313
   at System.Runtime.CompilerServices.AsyncTaskMethodBuilder`1.AsyncStateMachineBox`1.ExecutionContextCallback(Object s)
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
   at System.Runtime.CompilerServices.AsyncTaskMethodBuilder`1.AsyncStateMachineBox`1.MoveNext(Thread threadPoolThread)
   at System.Runtime.CompilerServices.AsyncTaskMethodBuilder`1.AsyncStateMachineBox`1.MoveNext()
   at System.Threading.ThreadPool.<>c.<.cctor>b__47_0(Object state)
   at System.Threading.Tasks.Sources.ManualResetValueTaskSourceCore`1.SignalCompletion()
   at System.Threading.Tasks.Sources.ManualResetValueTaskSourceCore`1.SetResult(TResult result)
   at System.Net.Sockets.Socket.AwaitableSocketAsyncEventArgs.OnCompleted(SocketAsyncEventArgs _) in /_/src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.Tasks.cs:line 1013
   at System.Net.Sockets.SocketAsyncEventArgs.OnCompletedInternal() in /_/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncEventArgs.cs:line 214
   at System.Net.Sockets.SocketAsyncEventArgs.FinishOperationAsyncSuccess(Int32 bytesTransferred, SocketFlags flags) in /_/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncEventArgs.cs:line 1130
   at System.Net.Sockets.SocketAsyncEventArgs.CompletionCallback(Int32 bytesTransferred, SocketFlags flags, SocketError socketError) in /_/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncEventArgs.Unix.cs:line 389
   at System.Net.Sockets.SocketAsyncEventArgs.TransferCompletionCallbackCore(Int32 bytesTransferred, Memory`1 socketAddress, SocketFlags receivedFlags, SocketError socketError) in /_/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncEventArgs.Unix.cs:line 108
   at System.Net.Sockets.SocketAsyncContext.BufferMemoryReceiveOperation.InvokeCallback(Boolean all

Stack trace:

   at Xunit.Assert.True(Nullable`1 condition, String userMessage) in /_/src/arcade/src/Microsoft.DotNet.XUnitAssert/src/BooleanAsserts.cs:line 141
   at Xunit.Assert.True(Boolean condition, String userMessage) in /_/src/arcade/src/Microsoft.DotNet.XUnitAssert/src/BooleanAsserts.cs:line 123
   at TestLibrary.OutOfProcessTest.RunOutOfProcessTest(String assemblyPath, String ...

</details>



<!-- START COPILOT CODING AGENT SUFFIX -->

- Fixes dotnet/runtime#124067

<!-- START COPILOT CODING AGENT TIPS -->
---

✨ Let Copilot coding agent [set things up for you](https://github.com/dotnet/runtime/issues/new?title=✨+Set+up+Copilot+instructions&body=Configure%20instructions%20for%20this%20repository%20as%20documented%20in%20%5BBest%20practices%20for%20Copilot%20coding%20agent%20in%20your%20repository%5D%28https://gh.io/copilot-coding-agent-tips%29%2E%0A%0A%3COnboard%20this%20repo%3E&assignees=copilot) — coding agent works faster and does higher quality work when set up for your repo.

[stephentoub]

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

[Copilot]

Pull request overview

This PR addresses a race condition in ProcessRunner’s stdout/stderr capture that can corrupt the shared StringBuilder and trigger StringBuilder.ExpandByABlock assertions during concurrent appends.

Changes:

• Adds synchronization around _outputCapture.AppendLine(...) in both stdout and stderr event handlers.
• Switches synchronization to lock on the _outputCapture instance itself (instead of a separate lock object).

[Copilot]

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

🤖 Copilot Code Review — PR #124225

Holistic Assessment

Motivation: The bug is real — StandardOutputEventHandler and StandardErrorEventHandler are invoked on separate thread pool threads (via AsyncStreamReader) and both append to the shared _outputCapture StringBuilder without synchronization, causing internal state corruption. The linked CI failure confirms this.

Approach: Adding lock around the concurrent AppendLine calls is the minimal correct fix. Locking on the _outputCapture instance itself (a private readonly field) is acceptable and consistent with the existing pattern in this file where _logWriter is also used as its own lock object (lines 240, 270, 376). Not locking the ToString() call in IsAvailable is correct — by that point, the process has exited and WaitForExit(0) has been called (line 324), so stdout/stderr streams have been fully drained and no more events will fire.

Summary: ✅ LGTM. The fix is minimal, correct, and consistent with the existing file conventions.

---

Detailed Findings

✅ Thread-safety fix is correct and sufficient

The two event handlers (StandardOutputEventHandler at line 281, StandardErrorEventHandler at line 294) are fired from AsyncStreamReader.ReadBufferAsync on thread pool threads — one for stdout, one for stderr — so they can run concurrently. The lock correctly serializes access to the shared _outputCapture StringBuilder, preventing the internal state corruption that caused the minBlockCharCount > 0 assertion failure.

✅ No lock needed on ToString() in IsAvailable

The _outputCapture.ToString() call (line 369) occurs after _process.WaitForExit(0) (line 324) returns. By that point, the async stream readers have completed and no more DataReceived events will fire, so there is no concurrent access to guard against. This is consistent with the Process contract where WaitForExit ensures all async output has been flushed.

✅ Locking on _outputCapture itself is consistent with existing file patterns

The file already uses the _logWriter object as its own lock (lines 240, 270, 376). Using _outputCapture as the lock object follows the same convention and avoids an unnecessary extra allocation. The field is private readonly so there's no risk of external code locking on the same instance.

[MihaZupan]

/ba-g DeadLetter on Apple TV, the rest are known