fix: ensure thread-safe initialization of console line buffers

[thomhurst]

Problem

PR #4549 introduced per-context console line buffers to prevent output mixing between parallel tests. However, a post-merge review comment identified a thread-safety issue with the lazy initialization in Context.cs:

// NOT thread-safe
internal (StringBuilder Buffer, object Lock) GetConsoleStdOutLineBuffer()
{
    return (_consoleStdOutLineBuffer ??= new StringBuilder(), _consoleStdOutBufferLock);
}

Race condition: Multiple threads calling GetConsoleStdOutLineBuffer() simultaneously could each create their own StringBuilder before the null-coalescing assignment completes, undermining the per-context buffer isolation that #4549 was designed to provide.

Solution

Replace nullable StringBuilder? fields with Lazy<StringBuilder> for guaranteed thread-safe initialization:

TUnit.Core/Context.cs:

• Changed StringBuilder? _consoleStdOutLineBuffer → Lazy<StringBuilder> _consoleStdOutLineBuffer = new(() => new StringBuilder())
• Changed StringBuilder? _consoleStdErrLineBuffer → Lazy<StringBuilder> _consoleStdErrLineBuffer = new(() => new StringBuilder())
• Updated accessors to use .Value instead of ??= operator

Benefits

✅ Thread-safe by default - Lazy<T> guarantees single initialization even under concurrent access
✅ No manual locking - No need for double-checked locking pattern
✅ Idiomatic C# - Standard approach for lazy initialization
✅ AOT-compatible - Works with Native AOT compilation
✅ Zero behavior change - Transparent fix maintaining all existing functionality

Testing

• ✅ Builds successfully across all target frameworks (netstandard2.0, net8.0, net9.0, net10.0)
• ✅ No build warnings
• ✅ Existing ParallelConsoleOutputTests from fix: prevent console output mixing between parallel tests #4549 continue to validate correct behavior

Addresses feedback from: #4549 (comment)

🤖 Generated with Claude Code

[thomhurst]

Summary

Replaces nullable StringBuilder fields with Lazy for thread-safe initialization of console line buffers.

Critical Issues

None found ✅

Analysis

The PR correctly addresses a legitimate thread-safety concern in the null-coalescing initialization pattern. However, I have one observation about the implementation:

Observation: Lock objects may be redundant

The change replaces ??= with Lazy<T> for thread-safe initialization, but keeps the separate lock objects (_consoleStdOutBufferLock, _consoleStdErrBufferLock). Looking at the usage in OptimizedConsoleInterceptor.cs:68-76, these locks protect access to the buffer contents during flush operations:

var (buffer, bufferLock) = GetLineBuffer();
lock (bufferLock)
{
    if (buffer.Length > 0)
    {
        RouteToSinks(buffer.ToString());
        buffer.Clear();
    }
}

The locks are clearly necessary for protecting buffer operations. The Lazy<T> wrapper only handles initialization thread-safety. So the current approach is correct - both mechanisms serve different purposes and are both needed.

Correctness:
✅ Thread-safe initialization via Lazy<T>
✅ Preserves existing locking for buffer content protection
✅ AOT-compatible (no reflection used)
✅ No behavioral changes
✅ Minimal allocation overhead (Lazy<T> wrapper is tiny, one-time cost)

Performance consideration:
The PR description in #4549 mentions this is a hot path (called on every Console.Write operation). Lazy<T>.Value access after initialization is extremely fast (just a field read + simple check), so there's no meaningful performance impact compared to the previous direct field access.

Verdict

✅ APPROVE - Clean fix that properly addresses the thread-safety issue without introducing new problems.

🤖 Generated with Claude Code