Search code examples
c#multithreadingtimerthreadpool

lock-statement in a Timer callback = potential thread starvation?


My library has multiple Timers (can be hundreds), that all call the same logic (it's a cache cleanup but that's not important).

This logic is pretty CPU-intensive, so I introduced a lock statement, to prevent multiple threads loading CPU at once:

_timer = new Timer(EvictExpiredItems, null, interval, interval);

private void EvictExpiredItems(object state)
{
    lock (_globalStaticLock)
    {
        DoStuff();
    }
}

However my concern is - because timer-callbacks are executed on thread-pool threads (?), and I'm actually blocking the thread, this can lead to thread starvation (for example, in an ASP.NET app).

Is this a valid concern?

My solution is to make the timer-callback async and use SempahoreSlim instead of a lock.

private async void EvictExpiredItems(object state)
{
    await _semaphoreSlim.WaitAsync(); //static semaphore var
    try
    {
        DoStuff();
    }
    finally
    {
         _semaphoreSlim.Release();
    }
}

Is this a valid concern and is this a valid solution?


Solution

  • Yes, using the lock in the timer's callback you are risking thread pool starvation. The Timer component invokes to callback on the ThreadPool, so in case the DoStuff takes a lot of time, and during this time the callback invocations are more than the threads available in the ThreadPool, the ThreadPool will become saturated. This means that new requests for work will not be satisfied immediately, and instead they'll satisfied only when a thread becomes available. The ThreadPool has an algorithm that creates or destroys threads when it has too few or too many, so a saturation incident rarely results in a deadlock, but the performance and responsiveness of your application might take an observable hit. If we are talking about an ASP.NET application, the scalability of your application might be seriously harmed too.

    On the contrary using the await _semaphoreSlim.WaitAsync() approach can achieve the same goal (prevent overlapping invocations of the DoStuff), without causing thread pool starvation. From this aspect the semaphore is better than the lock. Both are unlikely to be the optimal solutions for whatever feature you are trying to implement though.