1

I have the following code:

public const int ThreadLimitMax = 128;
private static object setThreadLimitLock = new object();
private static SemaphoreSlim totalThreadLimiter = new SemaphoreSlim(ThreadLimit, ThreadLimitMax);
        
public static int ThreadLimit { get; private set; } = 128;

public static async Task SetThreadLimit(int max)
{
    if (max > ThreadLimitMax)
        throw new ArgumentOutOfRangeException(nameof(max), $"Cannot have more than {ThreadLimitMax} threads.");
    if (max < 1)
        throw new ArgumentOutOfRangeException(nameof(max), $"Cannot have less than 1 threads.");

    lock (setThreadLimitLock)
    {
        int difference = Math.Abs(ThreadLimit - max);
        if (max < ThreadLimit)
        {
            for (int i = 0; i < difference; i++)
            {
                await totalThreadLimiter.WaitAsync().ConfigureAwait(false);
            }
        }
        else if (max > ThreadLimit)
        {
            totalThreadLimiter.Release(difference);
        }
        ThreadLimit = max;
    }
}

I am trying to make a method that will modify the amount of available threads in totalThreadLimiter. I keep the maximum number of threads in the ThreadMaxLimit integer.

To change the amount of threads, I need to ensure the ThreadLimit is not accessed until the operation of changing max threads is complete. I also need to ensure that the method is blocked until the totalThreadLimiter has completed with all WaitAsync() calls.

How can I do that?

Eduard G
  • 443
  • 5
  • 21
  • 2
    Use a lock which supports `await`, such as `SemaphoreSlim` (which has a `WaitAsync` method, and also supports being unlocked on a different thread to the one which locked it) – canton7 Jul 12 '21 at 13:10

1 Answers1

9

lock is a helper API around Monitor, which is a thread-bound synchronization primitive, which means it isn't suitable for use with await, because there is no guarantee what thread you'll be on when you come back from an incomplete asynchronous operation.

Ultimately, you need an async-aware synchronization primitive; the most readily available would be SemaphoreSlim, which has the WaitAsync() API that you would use to acquire the lock, with a try/finally that calls Release().

In the code in the question, depending on the code branch you either acquire (only) the semaphore, or release the semaphore; that is almost certainly wrong. Correct usage would be something more like:

await totalThreadLimiter.WaitAsync();
try
{
    // some code with "await" here
}
finally
{
    totalThreadLimiter.Release();
}
Marc Gravell
  • 1,026,079
  • 266
  • 2,566
  • 2,900
  • Ah I see. Thank you! Now I understand user1639030's answer in https://stackoverflow.com/questions/7612602/why-cant-i-use-the-await-operator-within-the-body-of-a-lock-statement , it wasn't clear to me that the SemaphoreSlim does not suffer from the same issue as lock. – Eduard G Jul 12 '21 at 13:15
  • 1
    It would be useful to show the initialization of the semaphore, as the parameters are not obvious. – Xavier Poinas Nov 11 '22 at 14:56