Locking with nested async calls

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
}

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
     }
}

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.

• https://github.com/neosmart/AsyncLock
• https://github.com/mysteryjeans/Flettu/

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 reëntrant call and go through
           using (await _lock.LockAsync()) // no deadlock
           {
               // do work
           }
     }
}

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

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();