Why can't I use the 'await' operator within the body of a lock statement?

Question

The await keyword in C# (.NET Async CTP) is not allowed from within a lock statement.

From MSDN:

An await expression cannot be used in a synchronous function, in a query expression, in the catch or finally block of an exception handling statement, in the block of a lock statement, or in an unsafe context.

I assume this is either difficult or impossible for the compiler team to implement for some reason.

I attempted a work around with the using statement:

class Async
{
    public static async Task<IDisposable> Lock(object obj)
    {
        while (!Monitor.TryEnter(obj))
            await TaskEx.Yield();

        return new ExitDisposable(obj);
    }

    private class ExitDisposable : IDisposable
    {
        private readonly object obj;
        public ExitDisposable(object obj) { this.obj = obj; }
        public void Dispose() { Monitor.Exit(this.obj); }
    }
}

// example usage
using (await Async.Lock(padlock))
{
    await SomethingAsync();
}

However this does not work as expected. The call to Monitor.Exit within ExitDisposable.Dispose seems to block indefinitely (most of the time) causing deadlocks as other threads attempt to acquire the lock. I suspect the unreliability of my work around and the reason await statements are not allowed in lock statement are somehow related.

Does anyone know why await isn't allowed within the body of a lock statement?

Answer 1

I assume this is either difficult or impossible for the compiler team to implement for some reason.

No, it is not at all difficult or impossible to implement -- the fact that you implemented it yourself is a testament to that fact. Rather, it is an incredibly bad idea and so we don't allow it, so as to protect you from making this mistake.

call to Monitor.Exit within ExitDisposable.Dispose seems to block indefinitely (most of the time) causing deadlocks as other threads attempt to acquire the lock. I suspect the unreliability of my work around and the reason await statements are not allowed in lock statement are somehow related.

Correct, you have discovered why we made it illegal. Awaiting inside a lock is a recipe for producing deadlocks.

I'm sure you can see why: arbitrary code runs between the time the await returns control to the caller and the method resumes. That arbitrary code could be taking out locks that produce lock ordering inversions, and therefore deadlocks.

Worse, the code could resume on another thread (in advanced scenarios; normally you pick up again on the thread that did the await, but not necessarily) in which case the unlock would be unlocking a lock on a different thread than the thread that took out the lock. Is that a good idea? No.

I note that it is also a "worst practice" to do a yield return inside a lock, for the same reason. It is legal to do so, but I wish we had made it illegal. We're not going to make the same mistake for "await".

Answer 2

Use SemaphoreSlim.WaitAsync method.

 await mySemaphoreSlim.WaitAsync();
 try {
     await Stuff();
 } finally {
     mySemaphoreSlim.Release();
 }

Answer 3

Basically it would be the wrong thing to do.

There are two ways this could be implemented:

• Keep hold of the lock, only releasing it at the end of the block.
  This is a really bad idea as you don't know how long the asynchronous operation is going to take. You should only hold locks for minimal amounts of time. It's also potentially impossible, as a thread owns a lock, not a method - and you may not even execute the rest of the asynchronous method on the same thread (depending on the task scheduler).

• Release the lock in the await, and reacquire it when the await returns
  This violates the principle of least astonishment IMO, where the asynchronous method should behave as closely as possible like the equivalent synchronous code - unless you use Monitor.Wait in a lock block, you expect to own the lock for the duration of the block.

So basically there are two competing requirements here - you shouldn't be trying to do the first here, and if you want to take the second approach you can make the code much clearer by having two separated lock blocks separated by the await expression:

// Now it's clear where the locks will be acquired and released
lock (foo)
{
}
var result = await something;
lock (foo)
{
}

So by prohibiting you from awaiting in the lock block itself, the language is forcing you to think about what you really want to do, and making that choice clearer in the code that you write.

Answer 4

This is just an extension to this answer.

using System;
using System.Threading;
using System.Threading.Tasks;

public class SemaphoreLocker
{
    private readonly SemaphoreSlim _semaphore = new SemaphoreSlim(1, 1);

    public async Task LockAsync(Func<Task> worker)
    {
        await _semaphore.WaitAsync();
        try
        {
            await worker();
        }
        finally
        {
            _semaphore.Release();
        }
    }

    // overloading variant for non-void methods with return type (generic T)
    public async Task<T> LockAsync<T>(Func<Task<T>> worker)
    {
        await _semaphore.WaitAsync();
        try
        {
            return await worker();
        }
        finally
        {
            _semaphore.Release();
        }
    }
}

Usage:

public class Test
{
    private static readonly SemaphoreLocker _locker = new SemaphoreLocker();

    public async Task DoTest()
    {
        await _locker.LockAsync(async () =>
        {
            // [async] calls can be used within this block 
            // to handle a resource by one thread. 
        });
        // OR
        var result = await _locker.LockAsync(async () =>
        {
            // [async] calls can be used within this block 
            // to handle a resource by one thread. 
        });
    }
}

Answer 5

This referes to Building Async Coordination Primitives, Part 6: AsyncLock , http://winrtstoragehelper.codeplex.com/ , Windows 8 app store and .net 4.5

Here is my angle on this:

The async/await language feature makes many things fairly easy but it also introduces a scenario that was rarely encounter before it was so easy to use async calls: reentrance.

This is especially true for event handlers, because for many events you don't have any clue about whats happening after you return from the event handler. One thing that might actually happen is, that the async method you are awaiting in the first event handler, gets called from another event handler still on the same thread.

Here is a real scenario I came across in a windows 8 App store app: My app has two frames: coming into and leaving from a frame I want to load/safe some data to file/storage. OnNavigatedTo/From events are used for the saving and loading. The saving and loading is done by some async utility function (like http://winrtstoragehelper.codeplex.com/). When navigating from frame 1 to frame 2 or in the other direction, the async load and safe operations are called and awaited. The event handlers become async returning void => they cant be awaited.

However, the first file open operation (lets says: inside a save function) of the utility is async too and so the first await returns control to the framework, which sometime later calls the other utility (load) via the second event handler. The load now tries to open the same file and if the file is open by now for the save operation, fails with an ACCESSDENIED exception.

A minimum solution for me is to secure the file access via a using and an AsyncLock.

private static readonly AsyncLock m_lock = new AsyncLock();
...

using (await m_lock.LockAsync())
{
    file = await folder.GetFileAsync(fileName);
    IRandomAccessStream readStream = await file.OpenAsync(FileAccessMode.Read);
    using (Stream inStream = Task.Run(() => readStream.AsStreamForRead()).Result)
    {
        return (T)serializer.Deserialize(inStream);
    }
}

Please note that his lock basically locks down all file operation for the utility with just one lock, which is unnecessarily strong but works fine for my scenario.

Here is my test project: a windows 8 app store app with some test calls for the original version from http://winrtstoragehelper.codeplex.com/ and my modified version that uses the AsyncLock from Stephen Toub.

May I also suggest this link: http://www.hanselman.com/blog/ComparingTwoTechniquesInNETAsynchronousCoordinationPrimitives.aspx

Answer 6

Stephen Taub has implemented a solution to this question, see Building Async Coordination Primitives, Part 7: AsyncReaderWriterLock.

Stephen Taub is highly regarded in the industry, so anything he writes is likely to be solid.

I won't reproduce the code that he posted on his blog, but I will show you how to use it:

/// <summary>
///     Demo class for reader/writer lock that supports async/await.
///     For source, see Stephen Taub's brilliant article, "Building Async Coordination
///     Primitives, Part 7: AsyncReaderWriterLock".
/// </summary>
public class AsyncReaderWriterLockDemo
{
    private readonly IAsyncReaderWriterLock _lock = new AsyncReaderWriterLock(); 

    public async void DemoCode()
    {           
        using(var releaser = await _lock.ReaderLockAsync()) 
        { 
            // Insert reads here.
            // Multiple readers can access the lock simultaneously.
        }

        using (var releaser = await _lock.WriterLockAsync())
        {
            // Insert writes here.
            // If a writer is in progress, then readers are blocked.
        }
    }
}

If you want a method that's baked into the .NET framework, use SemaphoreSlim.WaitAsync instead. You won't get a reader/writer lock, but you will get tried and tested implementation.

Answer 7

Hmm, looks ugly, seems to work.

static class Async
{
    public static Task<IDisposable> Lock(object obj)
    {
        return TaskEx.Run(() =>
            {
                var resetEvent = ResetEventFor(obj);

                resetEvent.WaitOne();
                resetEvent.Reset();

                return new ExitDisposable(obj) as IDisposable;
            });
    }

    private static readonly IDictionary<object, WeakReference> ResetEventMap =
        new Dictionary<object, WeakReference>();

    private static ManualResetEvent ResetEventFor(object @lock)
    {
        if (!ResetEventMap.ContainsKey(@lock) ||
            !ResetEventMap[@lock].IsAlive)
        {
            ResetEventMap[@lock] =
                new WeakReference(new ManualResetEvent(true));
        }

        return ResetEventMap[@lock].Target as ManualResetEvent;
    }

    private static void CleanUp()
    {
        ResetEventMap.Where(kv => !kv.Value.IsAlive)
                     .ToList()
                     .ForEach(kv => ResetEventMap.Remove(kv));
    }

    private class ExitDisposable : IDisposable
    {
        private readonly object _lock;

        public ExitDisposable(object @lock)
        {
            _lock = @lock;
        }

        public void Dispose()
        {
            ResetEventFor(_lock).Set();
        }

        ~ExitDisposable()
        {
            CleanUp();
        }
    }
}

Answer 8

I did try using a Monitor (code below) which appears to work but has a GOTCHA... when you have multiple threads it will give... System.Threading.SynchronizationLockException Object synchronization method was called from an unsynchronized block of code.

using System;
using System.Threading;
using System.Threading.Tasks;

namespace MyNamespace
{
    public class ThreadsafeFooModifier : 
    {
        private readonly object _lockObject;

        public async Task<FooResponse> ModifyFooAsync()
        {
            FooResponse result;
            Monitor.Enter(_lockObject);
            try
            {
                result = await SomeFunctionToModifyFooAsync();
            }
            finally
            {
                Monitor.Exit(_lockObject);
            }
            return result;
        }
    }
}

Prior to this I was simply doing this, but it was in an ASP.NET controller so it resulted in a deadlock.

public async Task<FooResponse> ModifyFooAsync() { lock(lockObject) { return SomeFunctionToModifyFooAsync.Result; } }