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://learn.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

score 0 · Answer 4 · answered Jun 22 '22 at 14:00

Disclaimer: I'm the author of the NuGet package I mention here.

There have been several attempts at a recursive/reentrant async lock (some are listed below) but only one of them succeeds in providing all three of these at once:

Asynchronicity
Reentrance
Mutual exclusion

As of this writing, the only correct implementation that I know of is:

https://www.nuget.org/packages/ReentrantAsyncLock/

The package documentation shows how to use it. Using it in your code would look like this:

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

    public async Task Bar()
    {
        await using (await _lock.LockAsync(CancellationToken.None))
        {
            await BarInternal();
        }
     }

    public async Task BarInternal()
    {
        await using (await _lock.LockAsync(CancellationToken.None)) // No deadlock
        {
            // DO work
        }
     }
}

I'm sure everyone knows what asynchronicity is.

Your code is an example of reentrance.

This is an example of mutual exclusion:

var gate = new object();
var value = 0;
var tasks = new List<Task>();
for (var i = 0; i < 1000; i++)
{
    var task = Task.Run(() =>
    {
        lock (gate)
        {
            value++; // Without the lock this is a race condition
        }
    });
    tasks.Add(task);
}
Task.WhenAll(tasks).Wait();
Debug.Assert(value == 1000);

The regular lock keyword in C# gives reentrance and mutual exclusion.

SemaphoreSlim and a dozen other things give asynchronicity and mutual exclusion.

But it has been difficult for people to get all three together at once.

For example, Stephen Cleary linked to his proof of concept in his answer. But his fails these tests:

https://github.com/matthew-a-thomas/cs-reentrant-async-lock/blob/ece6e461c26f005da2122185cb9c5b884968f98a/ReentrantAsyncLock.Tests/ReentrantAsyncLockClass.cs

(Keep in mind those tests were originally written for the ReentrantAsyncLock NuGet package, that's why some things are commented out that don't make sense for Stephen Cleary's RecursiveAsyncLock, and that's why the test file has ReentrantAsyncLock in its name. Compare that test file to the equivalent on the main branch and you'll see what I mean.)

Of course he has never claimed that it would but has only ever cautioned people against using it. So this isn't a ding against Stephen. I'm just giving an example of how someone can make an async lock and at first glance it looks like it gives all three of the things I listed above when in fact it has trouble putting the second two things together.

Similar things can be said for all of these:

https://github.com/neosmart/AsyncLock - these two tests deadlock
https://github.com/mysteryjeans/Flettu/ - this test sometimes deadlocks, sometimes fails with a mismatched count, and sometimes fails with a semaphore count exception
https://github.com/jasonkuo41/CellWars.Threading.AsyncLock - both of these tests fail with mismatched values

Shahar Shokrani · Answer 5 · 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

5 Answers5

Linked