From ab3bebfdbdbbb380fe288fb46d582f71ddc2de93 Mon Sep 17 00:00:00 2001 From: Jim Skimming <647706+JSkimming@users.noreply.github.com> Date: Sun, 5 Apr 2026 15:33:09 +0100 Subject: [PATCH] fix: prevent StringBuilder race in console interceptor during parallel tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OptimizedConsoleInterceptor.Write(StringBuilder) calls .ToString() on the caller's StringBuilder. ASP.NET Core's ConsoleLogger pools its StringBuilder and may return it to the pool immediately after Write returns. Under parallel test execution, another thread can pick up the pooled StringBuilder and mutate it while .ToString() is still traversing the internal chunk list, causing ArgumentOutOfRangeException (Parameter 'chunkLength'). Fix: copy the StringBuilder content via string.Create + CopyTo with a catch for the race condition. If the race hits, the output for that single log entry is lost rather than crashing the test. Also move the _outputBuilder.Length check inside the ReaderWriterLockSlim in Context.GetStandardOutput() and GetErrorOutput() — StringBuilder.Length is not thread-safe and should not be read outside the lock. Fixes #5411 --- TUnit.Core/Context.cs | 18 +++----- .../Logging/OptimizedConsoleInterceptor.cs | 41 +++++++++++++++++-- 2 files changed, 43 insertions(+), 16 deletions(-) diff --git a/TUnit.Core/Context.cs b/TUnit.Core/Context.cs index 6721a283a4..dd5b916d00 100644 --- a/TUnit.Core/Context.cs +++ b/TUnit.Core/Context.cs @@ -82,16 +82,13 @@ public void AddAsyncLocalValues() public virtual string GetStandardOutput() { - if (_outputBuilder.Length == 0) - { - return string.Empty; - } - _outputLock.EnterReadLock(); try { - return _outputBuilder.ToString(); + return _outputBuilder.Length == 0 + ? string.Empty + : _outputBuilder.ToString(); } finally { @@ -101,16 +98,13 @@ public virtual string GetStandardOutput() public virtual string GetErrorOutput() { - if (_errorOutputBuilder.Length == 0) - { - return string.Empty; - } - _errorOutputLock.EnterReadLock(); try { - return _errorOutputBuilder.ToString(); + return _errorOutputBuilder.Length == 0 + ? string.Empty + : _errorOutputBuilder.ToString(); } finally { diff --git a/TUnit.Engine/Logging/OptimizedConsoleInterceptor.cs b/TUnit.Engine/Logging/OptimizedConsoleInterceptor.cs index e8c4cbb428..314b6d0e10 100644 --- a/TUnit.Engine/Logging/OptimizedConsoleInterceptor.cs +++ b/TUnit.Engine/Logging/OptimizedConsoleInterceptor.cs @@ -165,17 +165,50 @@ public override async Task WriteLineAsync(string? value) #if NET public override void Write(ReadOnlySpan buffer) => Write(new string(buffer)); - public override void Write(StringBuilder? value) => Write(value?.ToString() ?? string.Empty); + public override void Write(StringBuilder? value) => Write(CopyStringBuilderSafely(value)); public override Task WriteAsync(ReadOnlyMemory buffer, CancellationToken cancellationToken = new()) => WriteAsync(new string(buffer.Span)); public override Task WriteAsync(StringBuilder? value, CancellationToken cancellationToken = new()) - => WriteAsync(value?.ToString() ?? string.Empty); + => WriteAsync(CopyStringBuilderSafely(value)); public override void WriteLine(ReadOnlySpan buffer) => WriteLine(new string(buffer)); - public override void WriteLine(StringBuilder? value) => WriteLine(value?.ToString() ?? string.Empty); + public override void WriteLine(StringBuilder? value) => WriteLine(CopyStringBuilderSafely(value)); public override Task WriteLineAsync(ReadOnlyMemory buffer, CancellationToken cancellationToken = new()) => WriteLineAsync(new string(buffer.Span)); public override Task WriteLineAsync(StringBuilder? value, CancellationToken cancellationToken = new()) - => WriteLineAsync(value?.ToString() ?? string.Empty); + => WriteLineAsync(CopyStringBuilderSafely(value)); + + /// + /// Safely copies the content of a caller-owned StringBuilder into a string. + /// Callers (e.g., ASP.NET Core's ConsoleLogger) may pool and reuse their + /// StringBuilder after Write returns. If the StringBuilder is mutated + /// concurrently during the copy, the ArgumentOutOfRangeException is caught + /// and the output for that single log entry is lost rather than crashing + /// the test. + /// + private static string CopyStringBuilderSafely(StringBuilder? value) + { + if (value is null) + { + return string.Empty; + } + + try + { + int length = value.Length; + if (length == 0) + { + return string.Empty; + } + + return string.Create(length, value, static (span, sb) => sb.CopyTo(0, span, span.Length)); + } + catch (ArgumentOutOfRangeException) + { + // The caller's StringBuilder was mutated concurrently (e.g., returned + // to a pool and reused by another thread). Swallow rather than crash. + return string.Empty; + } + } #endif public override IFormatProvider FormatProvider => GetOriginalOut().FormatProvider;