AsyncReaderWriterLock.ReaderLock - Check If Lock Applied about asyncex HOT 8 CLOSED

Interesting find.

From what testing I've done in LinqPad, if there is no lock contention, you can acquire the lock with an already cancelled CancellationToken. If there is contention then the CancellationToken is respected.

Dodgy script with 4x cases building upon each other and interacting...

var l = new AsyncLock();

var cts = new CancellationTokenSource();
cts.Cancel();

Task.Run(async () =>
{
    using (await l.LockAsync(cts.Token))
    {
                //I SEE THIS DUE TO THERE BEING NO CONTENTION - UNEXPECTED??
        Console.WriteLine("In cancelled lock - begin delay");   
        await Task.Delay(2000);
        Console.WriteLine("In cancelled lock - end delay");
    }
});

Task.Run(async () =>
{
    Console.WriteLine("Waiting before try lock no cancel token");
    await Task.Delay(500);
    Console.WriteLine("About to try lock no cancel token");
    using (await l.LockAsync())
    {
                //I SEE THIS AS EXPECTED AS WE ARE NOT USING THE TOKEN
        Console.WriteLine("In lock - begin delay");
        await Task.Delay(2000);
        Console.WriteLine("In lock - end delay");
    }
});


Task.Run(async () =>
{
    Console.WriteLine("Waiting before try delayed lock");
    await Task.Delay(1000);
    Console.WriteLine("About to try delayed lock");
    using (await l.LockAsync(cts.Token))
    {
                //I DO NOT SEE THIS AS WE HAVE A CANCELLED TOKEN AND THERE IS CONTENTION
        Console.WriteLine("In delayed lock - begin delay");  
        await Task.Delay(2000);
        Console.WriteLine("In delayed lock - end delay");
    }
});


Task.Run(async () =>
{
    Console.WriteLine("Waiting before try really delayed lock");
    await Task.Delay(6000);
    Console.WriteLine("About to try really delayed lock");
    using (await l.LockAsync(cts.Token))
    {
                //I SEE THIS DUE TO THERE BEING NO CONTENTION - UNEXPECTED??
        Console.WriteLine("In really delayed lock - begin delay");
        await Task.Delay(2000);
        Console.WriteLine("In really delayed lock - end delay");
    }
});

from asyncex.

Output I receive...

In cancelled lock - begin delay
Waiting before try delayed lock
Waiting before try lock no cancel token
Waiting before try really delayed lock
About to try lock no cancel token
About to try delayed lock
In cancelled lock - end delay
In lock - begin delay
In lock - end delay
About to try really delayed lock
In really delayed lock - begin delay
In really delayed lock - end delay

from asyncex.

And if I add some try/catch blocks in those Task.Run statements I do catch the one TaskCanceledException for the time when there's contention.

var l = new AsyncLock();

var cts = new CancellationTokenSource();
cts.Cancel();

Task.Run(async () =>
{
    using (await l.LockAsync(cts.Token))
    {
        try
        {
            //I SEE THIS DUE TO THERE BEING NO CONTENTION - UNEXPECTED??
            Console.WriteLine("In cancelled lock - begin delay");
            await Task.Delay(2000);
            Console.WriteLine("In cancelled lock - end delay");
        }
        catch (Exception ex)
        {
            ex.Dump("Ex in cancelled lock");
        }
    }
});

Task.Run(async () =>
{
    try
    {
        Console.WriteLine("Waiting before try lock no cancel token");
        await Task.Delay(500);
        Console.WriteLine("About to try lock no cancel token");
        using (await l.LockAsync())
        {
            //I SEE THIS AS EXPECTED AS WE ARE NOT USING THE TOKEN
            Console.WriteLine("In lock - begin delay");
            await Task.Delay(2000);
            Console.WriteLine("In lock - end delay");
        }
    }
    catch (Exception ex)
    {
        ex.Dump("ex in lock no cancel token");
    }
});


Task.Run(async () =>
{
    try
    {
        Console.WriteLine("Waiting before try delayed lock");
        await Task.Delay(1000);
        Console.WriteLine("About to try delayed lock");
        using (await l.LockAsync(cts.Token))
        {
            //I DO NOT SEE THIS AS WE HAVE A CANCELLED TOKEN AND THERE IS CONTENTION
            Console.WriteLine("In delayed lock - begin delay");
            await Task.Delay(2000);
            Console.WriteLine("In delayed lock - end delay");
        }
        Console.WriteLine("Never seen!!");
    }
    catch (Exception ex)
    {
        ex.Dump("Ex in delayed lock");
    }
});


Task.Run(async () =>
{
    try
    {
        Console.WriteLine("Waiting before try really delayed lock");
        await Task.Delay(6000);
        Console.WriteLine("About to try really delayed lock");
        using (await l.LockAsync(cts.Token))
        {
            //I SEE THIS DUE TO THERE BEING NO CONTENTION - UNEXPECTED??
            Console.WriteLine("In really delayed lock - begin delay");
            await Task.Delay(2000);
            Console.WriteLine("In really delayed lock - end delay");
        }
    }
    catch (Exception ex)
    {
        ex.Dump("Ex in really delayed lock");
    }
});

System.Threading.Tasks.TaskScheduler.UnobservedTaskException += (object sender, UnobservedTaskExceptionEventArgs args) =>
{
    //I don't get any unobserved exceptions.  
    //Note that even without the try/catch blocks I didn't get any.
    args.Dump();
};

That gives

Waiting before try lock no cancel token
Waiting before try really delayed lock
In cancelled lock - begin delay
Waiting before try delayed lock
About to try lock no cancel token
About to try delayed lock
 EXCEPTION
In cancelled lock - end delay
In lock - begin delay
In lock - end delay
About to try really delayed lock
In really delayed lock - begin delay
In really delayed lock - end delay

from asyncex.

Right. If you pass in an already-cancelled CT, then the returned task will be already completed. It will be successfully completed if the lock was taken, and it will be canceled if not.

I have more detail on this design written up on my blog.

Question for you two: do you think this is a good API design?

Good points:

• It is consistent.
• It keeps the API overloads short and clean; it doesn't complicate any APIs with boolean/null return values.

Bad points:

• It's only consistent within the AsyncEx library. No one else does this.
• It can encourage consumers to use exceptions for control flow.
• It is hard to find.

I'm re-examining all APIs right now as I'm doing a major version (breaking change) update as soon as .NET Core RTMs. This "special behavior for cancelled tokens" design is one thing I've never been perfectly happy with.

from asyncex.

Thanks for the blog link - I thought I'd read through most of your blog history but didn't recall that article :)

What you've written makes sense. I guess there's no way to guarantee an atomic wait since, even with a genuine CancellationToken, it could get cancelled just as you're making the call to begin the lock acquisition.

A TryLockAsync call might not be a bad addition to the library simply from the point of view of being able to avoid needing to catch an exception (sometimes the code flow can be more clear without having to catch the TaskCanceledException). The return value of that could still be something that's disposable but also has some property like LockWasEntered making its use like

using(var lockToken = await locker.TryLockAsync())
{
    if (lockToken.LockWasEntered) 
    {
        Stuff....


    }

}

In my app I have a bunch of repositories that I lock ahead of the operation I'm performing. For convenience I have a LockTokenFactory which handles this multi-lock acquisition (in consistent order to avoid deadlocks) and can be operated via using/IDisposable or via an overload that accepts a lambda. The lambda is executed upon successful lock and otherwise isn't executed. I could add additional optional parameters to accept a CancellationToken and even a lambda to execute if that token was cancelled during the wait. It may not be quite as efficient but it's convenient to use and ensures I can't get the pattern wrong.

The most common use is

var value = await lockTokenFactory.InReadToken( token=> myRepo.Search(param1,param2, token), myRepo );

(where myRepo implements ILockable and most repository methods take in the generated token because it's also how I implement unit-of-work semantics, particularly if I use the InWriteToken method since it will save the lazily generated EF context to the DB when there's no error).

I use a params[] at the end of the InReadToken method signature to accept the repositories that should be locked, but with a twist (again because I'm lazy and like the compiler to catch stupid mistakes). The signature is more like Task InReadToken<T>( Func<token, T> func, ILockable first, params ILockable[] others) - that way I (or new team members) can't accidentally leave the repository off the call, waiting for the blow-up at runtime or test time.

Like I said above (sorry this got so long but I thought it might be good to explain how I use your library at a higher level) there are other overloads that can accept CancellationToken, a Func to call when there's a cancellation, etc. The overloads without that Func will throw TaskCanceledException instead. Sometimes one approach is better than the other depending on the circumstances. This thread means I'll go double-check my code :)

from asyncex.

I like the TryLock option as well.

Also, this didn't really answer my question. Is there any way to tell if the lock was applied when calling the synchronous methods (like Lock instead of LockAsync)?

from asyncex.

Sorry about that.

In AsyncEx, the synchronous APIs all have the same semantics as the asynchronous APIs (except that they're synchronous, obviously). So, in the case of AsyncReaderWriterLock.ReaderLock, if you pass a cancelled CancellationToken, then:

• If the lock is in a state where a reader lock can be taken, the lock is taken and ReaderLock returns.
• Otherwise, the lock is not taken and ReaderLock throws an OperationCanceledException.

Note that if you pass a cancelled CancellationToken, then ReaderLock does not actually block at all.

from asyncex.

Moving this conversation here.

from asyncex.