Locking with nested async calls

Question

I am working on a multi threaded WindowsPhone8 app that has critical sections within async methods.

Does anyone know of a way to properly use semaphores / mutexes in C# where you are using nested async calls where the inner method may be acquiring the same lock that it already acquired up the callstack? I thought the SemaphoreSlim might be the answer, but it looks like it causes a deadlock.

public class Foo
{
    SemaphoreSlim _lock = new SemaphoreSlim(1);

    public async Task Bar()
    {
        await _lock.WaitAsync();

        await BarInternal();

        _lock.Release();
     }

    public async Task BarInternal()
    {
        await _lock.WaitAsync();  // deadlock

        // DO work

        _lock.Release();
     }

}

Recursive locking is often considered to be a bad practice. Can't you just restructure your code so that this doesn't happen? — svick, Nov 06 '13 at 22:49
It does not depend on async/await in this particular case. This piece of code will fall to deadlock in any case just because it tries to acquire lock **two times** one after another. Yes, they can be executed in different threads (because async/await are executed on the thread pool), but they are executed **consequentially**. — Stanislav, Jul 21 '15 at 07:55

score 14 · Answer 1 · answered Nov 06 '13 at 23:30

Recursive locks are a really bad idea (IMO; link is to my own blog). This is especially true for async code. It's wicked difficult to get async-compatible recursive locks working. I have a proof-of-concept here but fair warning: I do not recommend using this code in production, this code will not be rolled into AsyncEx, and it is not thoroughly tested.

What you should do instead is restructure your code as @svick stated. Something like this:

public async Task Bar()
{
    await _lock.WaitAsync();

    await BarInternal_UnderLock();

    _lock.Release();
}

public async Task BarInternal()
{
    await _lock.WaitAsync();

    await BarInternal_UnderLock();

    _lock.Release();
}

private async Task BarInternal_UnderLock()
{
    // DO work
}

@PoulBak: Just like the original code. Presumably, a real-world `Bar` would do something different than `BarInternal`, but since the code in the question was identical, the code in my answer matches. — Stephen Cleary, Aug 15 '18 at 12:35

Knickedi · Answer 2 · 2013-11-06T23:33:36.367

Here's what I did in such a situation (still, I'm not experienced with tasks, so don't beat me ;-)
So basically you have move the actual implementation to non locking methods and use these in all methods which acquire a lock.

public class Foo
{
    SemaphoreSlim _lock = new SemaphoreSlim(1);

    public async Task Bar()
    {
        await _lock.WaitAsync();
        await BarNoLock();
        _lock.Release();
     }

    public async Task BarInternal()
    {
        await _lock.WaitAsync(); // no deadlock
        await BarNoLock();
        _lock.Release();
     }

     private async Task BarNoLock()
     {
         // do the work
     }
}

vlee · Answer 3 · 2021-01-28T22:21:05.520

First, read through Stephen Cleary's blog post, which he linked to in his answer. He mentions multiple reasons, such as uncertain lock state and inconsistent invariants, which are associated with recursive locks (not to mention recursive async locks). If you can do the refactoring he and Knickedi describe in their answers, that would be great.

However, there are some cases where that type of refactoring is just not possible. Fortunately, there are now multiple libraries which support nested async calls (lock reentrance). Here are two. The author of the first has a blog post where he talks more about it.

You can incorporate it into your code as such (using the first library in this example):

public class Foo
{
    AsyncLock _lock = new AsyncLock();

    public async Task Bar()
    {
           // This first LockAsync() call should not block
           using (await _lock.LockAsync())
           {
               await BarInternal();
           }
     }

    public async Task BarInternal()
    {
           // This second call to LockAsync() will be recognized
           // as being a reëntrant call and go through
           using (await _lock.LockAsync()) // no deadlock
           {
               // do work
           }
     }
}

I do agree that one should carefully examine if existing nested locks are necessary or just a code path convenience. That said, I highly recommend the Flettu.AsyncLock class you mention. It is clean and simple and he uses [AsyncLocal](https://docs.microsoft.com/en-us/dotnet/api/system.threading.asynclocal-1) which is native to the framework and carries state through an async flow the way `ThreadLocal` can do for synchronous code. — mdisibio, Feb 22 '21 at 20:33

Shahar Shokrani · Answer 4 · 2020-03-22T18:40:20.173

You can use the System.Threading.ReaderWriterLockSlim (doc), which has a support recursion flag:

ReaderWriterLockSlim _lock = new ReaderWriterLockSlim(LockRecursionPolicy.SupportsRecursion);

async Task Bar()
{
    try
    {
        _lock.EnterReadLock();
        await BarInternal();
    }
    finally
    {
        if (_lock.IsReadLockHeld)
            _lock.ExitReadLock();
    }
}

async Task BarInternal()
{
    try
    {
        _lock.EnterReadLock();
        await Task.Delay(1000);
    }
    finally
    {
        if (_lock.IsReadLockHeld)
            _lock.ExitReadLock();
    }
}

Still you should be very careful with recursion because it is very difficult to control which thread took a lock and when.

The code in the question will be result in a deadlock because it tries to acquire the lock twice, something like:

await _lock.WaitAsync();
await _lock.WaitAsync(); --> Will result in exception.

While flagging the ReaderWriterLockSlim in SupportsRecursion will not throw an exception for this weird code:

 _lock.EnterReadLock();
 _lock.EnterReadLock();

Locking with nested async calls

4 Answers4

Linked