Skip to content

[Code Quality] RotatingStreamWriter — Thread.Sleep called while holding _lock blocks all writers for up to 100ms during rotation retries #1066

@Christophe-Rogiers

Description

@Christophe-Rogiers

Severity: Warning

File: src/Servy.Core/IO/RotatingStreamWriter.cs

Lines: 449-468 (Rotate method retry loop), called transitively from WriteLine/Write inside lock (_lock)

Code snippet:

public void WriteLine(string line)
{
    lock (_lock)
    {
        ...
        CheckRotation();   // -> Rotate() -> may Thread.Sleep below
    }
}

private void Rotate()
{
    ...
    for (int attempt = 0; attempt < MaxSyncRotationRetries; attempt++)
    {
        try
        {
            File.Move(_file.FullName, rotatedPath);
            success = true;
            break;
        }
        catch (IOException ex)
        {
            if (attempt < MaxSyncRotationRetries - 1)
            {
                Thread.Sleep(SyncRotationRetryDelayMs);   // <-- inside lock
            }
            ...
        }
    }
}

Explanation:
Both WriteLine(string) and Write(string) enter lock (_lock) and then call CheckRotationRotate. When File.Move throws IOException (file lock contention is exactly the scenario the retry exists for), Rotate calls Thread.Sleep(50) up to twice while still holding _lock. Every other thread that calls Write/WriteLine on the same writer is parked for up to 100 ms per rotation attempt.

For services whose child process produces high-volume stdout/stderr bursts (the very condition that triggers size-based rotation), this is a self-inflicted backpressure spike: the redirected pipe stalls on the very write that triggered the rotation, plus every other concurrent log write.

Suggested fix:
Move the timestamp+File.Move work outside the public Write lock — e.g. swap the StreamWriter atomically under the lock, then release the lock and perform the rename + retries on the now-detached file path. The cooldown / circuit-breaker fields are already structured for that, but the synchronous Thread.Sleep inside the critical section defeats them.

🤖 Generated with Claude Code

Metadata

Metadata

Assignees

Labels

enhancementNew feature or request

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions