What is the Correct Usage of SempahoreSlim as a Lock in Async Code?

Question

It seems like in async code these days, SemaphoreSlim is the recommended replacement for lock(obj) {}. I found this recommendation for how to use it: https://blog.cdemi.io/async-waiting-inside-c-sharp-locks/

In particular, this person suggest this code:

//Instantiate a Singleton of the Semaphore with a value of 1. This means that only 1 thread can be granted access at a time.
static SemaphoreSlim semaphoreSlim = new SemaphoreSlim(1,1);

//Asynchronously wait to enter the Semaphore. If no-one has been granted access to the Semaphore, code execution will proceed, otherwise this thread waits here until the semaphore is released 
await semaphoreSlim.WaitAsync();
try
{
    await Task.Delay(1000);
}
finally
{
    //When the task is ready, release the semaphore. It is vital to ALWAYS release the semaphore when we are ready, or else we will end up with a Semaphore that is forever locked.
    //This is why it is important to do the Release within a try...finally clause; program execution may crash or take a different path, this way you are guaranteed execution
    semaphoreSlim.Release();
}

To me it seems like this code violates the advice I used to see about how to lock, which is to keep in mind that your code could be interrupted at any time and code for that. If any exception is thrown just after await sempahoreSlim.WaitAsync() and before the try statement is entered, the semaphore will never be released. This kind of problem is exactly why I thought the lock statement and using statements were introduced with such great results.

Is there a reference somewhere that definitively explains that this code is valid? Perhaps try/finally statements are actually entered before the code can be interrupted, which is something I've never known about before? Or, is there a different pattern that would actually be the correct use of a semaphore as a lock, or some other locking mechanism for async .NET code?

Answer 1

Yes, your assumption is correct. If an exception is thrown before try is entered, the SemaphoreSlim will never be released. Although this is a really really rare event which I'd argue can be ignored in most cases. For example when the code is executed as a Task that can be cancelled and unfortunately happend to be cancelled exactly after the Wait() finished and before the try{} was entered. It is possible, but so unlikely you shouldn't have to worry about.

The code you have provided as an example is valid and even provided similiarly in the Microsoft Docs (https://learn.microsoft.com/en-us/dotnet/api/system.threading.semaphoreslim?view=net-6.0#examples)

In case you have a program where this event must not occur under any circumstances, you can implement a lock(){} alike pattern with using(){} however. This is the class I use in all my projects instead of manually handling SemaphoreSlim or the lock(){} statement.

public class MySemaphore : IDisposable
{
    private string semaphoreLockKey;
    private static Dictionary<string, SemaphoreSlim> internalSemaphoreSlimDict = new Dictionary<string, SemaphoreSlim>();

    /// <summary>
    /// <para>Creates a <see cref="MySemaphore"/> for the given <paramref name="key"/> and aquires the lock this <see cref="MySemaphore"/> represents.</para>
    /// <para>The task this method returns will await the lock for this <see cref="MySemaphore"/> if the semaphore with the key is already in use.
    /// Once the task aquired the lock, an instance of <see cref="MySemaphore"/> is returned, which will release the lock once <see cref="Dispose"/> is called (preferably via a using() statement)</para>
    /// </summary>
    /// <param name="key"></param>
    /// <returns>Returns a <see cref="MySemaphore"/> that holds the lock of the given <paramref name="key"/>. Dispose the returned instance to release the lock (preferably via a using() statement)</returns>
    /// <remarks>Wrap this into a using() to release the semaphore upon finishing your locked code</remarks>
    public static async Task<MySemaphore> WaitForLockAsync(string key)
    {
        var mySemaphore = new MySemaphore(key);

        await internalSemaphoreSlimDict[key].WaitAsync();
        return mySemaphore;
    }

    /// <summary>
    /// <para>Creates a <see cref="MySemaphore"/> for the given <paramref name="key"/> and aquires the lock this <see cref="MySemaphore"/> represents.</para>
    /// <para>The task this method returns will await the lock for this <see cref="MySemaphore"/> if the semaphore with the key is already in use.
    /// Once the task aquired the lock, an instance of <see cref="MySemaphore"/> is returned, which will release the lock once <see cref="Dispose"/> is called (preferably via a using() statement)</para>
    /// </summary>
    /// <param name="key"></param>
    /// <returns>Returns a <see cref="MySemaphore"/> that holds the lock of the given <paramref name="key"/>. Dispose the returned instance to release the lock (preferably via a using() statement)</returns>
    /// <remarks>Wrap this into a using() to release the semaphore upon finishing your locked code</remarks>
    public static MySemaphore WaitForLock(string key)
    {
        var mySemaphore = new MySemaphore(key);

        internalSemaphoreSlimDict[key].Wait();
        return mySemaphore;
    }

    /// <summary>
    /// Constructor using a key. If a key already exists and is currently used, it will lock the calling thread until the other thread has disposed his MySemaphore
    /// </summary>
    /// <param name="key"></param>
    private MySemaphore(string key)
    {
        this.semaphoreLockKey = key;
        if (!internalSemaphoreSlimDict.ContainsKey(key))
            internalSemaphoreSlimDict[key] = new SemaphoreSlim(1, 1);
    }

    /// <summary>
    /// Releases the Lock that is held by this instance
    /// </summary>
    public void Dispose()
    {
        internalSemaphoreSlimDict[semaphoreLockKey].Release();
    }
}

Using this class, we can define a (string)key for which a SemaphoreSlim will be created. Two Tasks that know nothing of each other can use the same key and thus wait for the other Task to finish. When we call MySemaphore.WaitForLock, a new instance of MySemaphore (that represents a SemaphoreSlim) will be created and SemaphoreSlim.Wait() will be called. Once the lock is aquired, the MySemaphore instance is returned.

Since we wrap this into a using(){} statement, whenever the using is exited (either by finishing the code or when any exception is thrown after entering the using block, the MySemaphore.Dispose() is called, which in turn releases the lock.

Usage then would be:

using (MySemaphore.WaitForLock("someLockKey"))
{
    //do something
}

Or

using (await MySemaphore.WaitForLockAsync("someLockKey"))
{
    await Task.Delay(1000);
}

Please note, this way you primarly create a lock(){} like pattern that supports usage in async methods and reduces the number of lines in your code. According to the Microsoft Docs, in older C# versions the using() statement is compiled to practically the same as if you were using try/finally. That means, MySemaphore will only be disposed once the using block has been entered - not when WaitForLock() is still being executed. This means, you still have the same problem that between Wait() aquiring the lock and the using block being entered, the SemaphoreSlim will not be released.

This however has changed in C# 8.0 when you use the using declaration (see https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/proposals/csharp-8.0/using#using-declaration), according to here: https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/using-statement

The newer using statement syntax translates to similar code. The try block opens where the variable is declared. The finally block is added at the close of the enclosing block, typically at the end of a method.

Since we declare the MySemaphore before the Wait() call (when using the using declaration, rather than the using statement), the SemaphoreSlim will be released under any circumstances. You must however handle the case that Dispose() may be called before Wait() was able to aquire the lock. This can be resolved by adding a new boolean field in the MySemaphore class and flipping it to true after SemaphoreSlim.Wait() is called and only call Release() when the field is true.

private bool hasLock;
public void Dispose()
{
    if(hasLock)
        internalSemaphoreSlimDict[semaphoreLockKey].Release();
}

You would need to change the usage of MySemaphore then however:

using MySemaphore ms = MySemaphore.WaitForLock("someLockKey");
//do something
//MySemaphore "ms" will be disposed when the current scope is exited

Or

using MySemaphore ms = await MySemaphore.WaitForLockAsync("someLockKey");
//do something
//MySemaphore "ms" will be disposed when the current scope is exited

With this, as soon as the "ms" instance is declared, whatever happens, the lock aquired will always guaranteed to be released.

Answer 2

Yes, it is true that in theory something could happen between the await semaphoreSlim.WaitAsync(); and the try, but in reality: in that scenario your app is already toast, as it is in the process of imploding call-stacks. In reality, while this is a theoretical concern, there isn't much that you can usefully do anyway, and your process is about to be put down ungracefully like the sickly thing that it is.

So my advice: don't worry too much about it :)

(in reality, the much bigger risk is a thread-pool spiral of death, usually caused by sync-over-async on thread-pool threads, meaning that even though you have acquired the semaphore semantically, there is no pool thread to give to you to actually do the thing, allowing you to get back to releasing it)