Skip to content

Conversation

@thomhurst
Copy link
Owner

Problem

When running multiple tests in parallel in Visual Studio Test Explorer (or via dotnet test), console output from Console.Write() and Console.WriteLine() was being mixed between tests or missing entirely.

Reported in #4545

Root Cause

The OptimizedConsoleInterceptor class used an instance field _lineBuffer to buffer partial console writes (e.g., Console.Write() without a newline). Since StandardOutConsoleInterceptor and StandardErrorConsoleInterceptor are singletons, this buffer was shared across all parallel tests.

Example of the bug:

  1. Test A calls Console.Write("Test1-Start") → buffered in shared _lineBuffer
  2. Test B calls Console.WriteLine("-Test2-End") → appends to same buffer
  3. Combined "Test1-Start-Test2-End" gets routed to Test B's context via Context.Current
  4. Result: Test B sees Test A's output, Test A loses its output

Solution

Moved console line buffers into Context itself, leveraging the existing robust Context.Current AsyncLocal mechanism:

Changes:

TUnit.Core/Context.cs:

  • Added per-context console line buffers (_consoleStdOutLineBuffer, _consoleStdErrLineBuffer)
  • Added thread safety locks for each buffer
  • Exposed via GetConsoleStdOutLineBuffer() and GetConsoleStdErrLineBuffer() methods

TUnit.Engine/Logging/OptimizedConsoleInterceptor.cs:

  • Made GetLineBuffer() abstract, returning (StringBuilder Buffer, object Lock)
  • Added lock blocks around all buffer operations

Console interceptor implementations:

  • Updated to retrieve buffers from current context

Benefits

Per-test isolation: Each test context has its own console line buffers
Thread safety: Locks protect concurrent access within a single test
Consistency: Follows same pattern as OutputWriter/ErrorOutputWriter
No new AsyncLocal: Reuses existing Context.Current mechanism

Testing

  • ✅ All 80 existing CaptureOutputTests pass
  • ✅ Added 33 new ParallelConsoleOutputTests for regression testing
  • ✅ Tests verify output isolation between parallel tests with partial writes

Closes #4545

🤖 Generated with Claude Code

## Problem
When running multiple tests in parallel, console output (Console.Write/WriteLine)
was being mixed between tests or missing entirely. This occurred because the
console interceptor's line buffer was a singleton instance field shared across
all parallel tests.

## Root Cause
OptimizedConsoleInterceptor used `private readonly StringBuilder _lineBuffer`
which was shared by all tests. When Test A called Console.Write() without a
newline, it buffered text in the shared buffer. If Test B then called
Console.WriteLine(), it would append to the same buffer and route the combined
output to Test B's context via Context.Current.

## Solution
Move console line buffers into Context itself, leveraging the existing robust
Context.Current AsyncLocal mechanism:

- Added per-context console buffers in Context.cs with thread safety locks
- Made GetLineBuffer() abstract, returning (StringBuilder Buffer, object Lock)
- Added locking around all buffer operations for thread safety within tests
- Updated StandardOutConsoleInterceptor and StandardErrorConsoleInterceptor

## Benefits
- Per-test isolation: each test context has its own console line buffers
- Thread safety: locks protect concurrent access within a single test
- Consistency: follows same pattern as OutputWriter/ErrorOutputWriter
- No new AsyncLocal: reuses existing Context.Current mechanism

## Testing
- All 80 existing CaptureOutputTests pass
- Added 33 new ParallelConsoleOutputTests for regression testing

Fixes #4545

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@thomhurst thomhurst self-assigned this Jan 24, 2026
@thomhurst thomhurst enabled auto-merge (squash) January 24, 2026 12:37
@thomhurst
Copy link
Owner Author

Summary

Fixes console output mixing between parallel tests by moving line buffers from singleton interceptor instance to per-test Context.

Critical Issues

Thread-safe lazy initialization needed in Context.cs:30-34

The lazy initialization in GetConsoleStdOutLineBuffer() and GetConsoleStdErrLineBuffer() is not thread-safe:

internal (StringBuilder Buffer, object Lock) GetConsoleStdOutLineBuffer()
{
    return (_consoleStdOutLineBuffer ??= new StringBuilder(), _consoleStdOutBufferLock);
}

Problem: Multiple threads calling this simultaneously could each create their own StringBuilder before assignment completes, defeating the purpose of the fix.

Solution: Either:

  1. Ensure Context is only accessed from a single thread (document this requirement)
  2. Use double-checked locking:
internal (StringBuilder Buffer, object Lock) GetConsoleStdOutLineBuffer()
{
    if (_consoleStdOutLineBuffer == null)
    {
        lock (_consoleStdOutBufferLock)
        {
            if (_consoleStdOutLineBuffer == null)
            {
                _consoleStdOutLineBuffer = new StringBuilder();
            }
        }
    }
    return (_consoleStdOutLineBuffer, _consoleStdOutBufferLock);
}
  1. Or use Lazy<StringBuilder> for guaranteed thread-safe initialization

Question: Can you clarify whether a single Context instance is guaranteed to only be accessed from one thread at a time, or if concurrent access is possible?

Suggestions

None - the approach is sound once the thread safety issue is resolved.

Verdict

⚠️ REQUEST CHANGES - Thread safety issue must be addressed

@thomhurst thomhurst merged commit c01a6fc into main Jan 24, 2026
12 of 13 checks passed
@thomhurst thomhurst deleted the fix/issue-4545-console-output-mixing branch January 24, 2026 13:11
thomhurst added a commit that referenced this pull request Jan 24, 2026
Addresses post-merge feedback from PR #4549 regarding thread-safety
concerns in console buffer lazy initialization.

Replaced nullable StringBuilder fields with Lazy<StringBuilder> to
guarantee thread-safe initialization when multiple threads access
GetConsoleStdOutLineBuffer() or GetConsoleStdErrLineBuffer() concurrently.

The previous implementation using ??= was not thread-safe and could
result in multiple StringBuilder instances being created before
assignment completes, undermining per-context buffer isolation.

Benefits:
- Thread-safe by default with Lazy<T>
- No manual double-checked locking needed
- Cleaner, more idiomatic C# code
- AOT-compatible

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
thomhurst added a commit that referenced this pull request Jan 24, 2026
Addresses post-merge feedback from PR #4549 regarding thread-safety
concerns in console buffer lazy initialization.

Replaced nullable StringBuilder fields with Lazy<StringBuilder> to
guarantee thread-safe initialization when multiple threads access
GetConsoleStdOutLineBuffer() or GetConsoleStdErrLineBuffer() concurrently.

The previous implementation using ??= was not thread-safe and could
result in multiple StringBuilder instances being created before
assignment completes, undermining per-context buffer isolation.

Benefits:
- Thread-safe by default with Lazy<T>
- No manual double-checked locking needed
- Cleaner, more idiomatic C# code
- AOT-compatible

Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
This was referenced Jan 26, 2026
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.

[Bug]: Log outputs when running multiple tests in parallel are either missing or incomplete

2 participants