Asynchronous Lock Primitives suggestion

Jun 18, 2013 at 2:49 PM
I would like to suggest a change to the Asynchronous Lock Primitives that should not break the public API (at least the way it should be used) and could help prevent incorrect usage at compile time.

Lets say I have some code that wants to use the AsyncLock primitive to coordinate the access to a shared resource. The code will look something like this:
public async Task SomthingAsync()
{
    using (await this.lock.LockAsync())
    {
        // TODO: Access a shared resource
        await Task.Delay(100);
    }
}
However, the following code will also compile without error (but seriously breaks the lock pattern; notice the missing await in the "using" line)
public async Task SomthingAsync()
{
    using (this.lock.LockAsync())
    {
        // TODO: Access a shared resource
        await Task.Delay(100);
    }
}
The reason the last piece of code compiles, is because the compiler expects an object that implements IDisposable to be passed to the "using" and since "LockAsync()" returns a Task<IDisposable> there is no problem to the compiler, since System.Threading.Tasks.Task<T> implements IDisposable it self. But in fact what the code expects is the IDisposable result of the returned task to be passed to the using (not the task it self).

So my suggestion is that you change the AsyncLock.LockAsync() method (and any other async lock methods of other Asynchronous Lock Primitives) to return a TaskAwaiter<IDisposable> structure (or any other custom awaitable structure that does not implement IDisposable). With this change the the second example (above) will not compile and then prevent the incorrect use of the Asynchronous Lock Primitives, but the first example will still compile (as expected).
Coordinator
Jun 18, 2013 at 4:09 PM
That's an interesting suggestion! I will take it under consideration.

This would definitely be a breaking change; there are (rare, but valid) use cases that require a Task, e.g., when using a lock in a Task.WhenAll.

-Steve
Nov 5, 2013 at 1:51 PM
StephenCleary wrote:
That's an interesting suggestion! I will take it under consideration.

This would definitely be a breaking change; there are (rare, but valid) use cases that require a Task, e.g., when using a lock in a Task.WhenAll.

-Steve
Since this is a breaking change, have you considered implementing it in version 2 which you plan to release soon?
Coordinator
Nov 6, 2013 at 8:16 PM
I've already released 2.0.0, and this is still on the "back burner" for now. Copying this to a work item to ensure it'll get in the next major revision.
Coordinator
Nov 6, 2013 at 8:16 PM
This discussion has been copied to a work item. Click here to go to the work item and continue the discussion.