Third party methods with ConfigureAwait

[Xriuk]

Hello,
In the readme it says:

it's fine if the current SynchronizationContext is changed or ConfigureAwait(false) is used by an awaited method within the guarded section

But later it says:

So if you're executing third party methods within the guarded section and if you're concerned that they might change the current SynchronizationContext then just wrap them in Task.Run or something similar.

So if I understood correctly by "awaited method" you mean an async one? Not one that just returns a Task, right?
Basically, is there any difference in the below methods? Or they both should run fine even without Task.Run? When exactly you should wrap a method in Task.Run? Thanks

Task TaskMethod(){
    SynchronizationContext.SetSynchronizationContext(null);
    return Task.Delay(1).ConfigureAwait(false);
}

async Task AsyncTaskMethod(){
    SynchronizationContext.SetSynchronizationContext(null);
    await Task.Delay(1).ConfigureAwait(false);
}


var asyncLock = new ReentrantAsyncLock();
await using (await asyncLock.LockAsync(CancellationToken.None))
{
    await TaskMethod();
}
await using (await asyncLock.LockAsync(CancellationToken.None))
{
    await AsyncTaskMethod();
}

[matthew-a-thomas]

Good question. By "awaited method" I mean any asynchronous method that you await. To use your example, await TaskMethod() causes problems but await AsyncTaskMethod() does not.

For example:

AsyncTaskMethod is okay
TaskMethod is not okay

So you'd want to wrap the call to TaskMethod e.g.

await Task.Run(() => TaskMethod());

The difference is the async keyword on the method. That makes the compiler build an asynchronous state machine behind the scenes for that method. And then it moves the guts of that method into that state machine, and then replaces the method with code that looks sort of like this:

private static Task AsyncTaskMethod()
{
  var stateMachine = new StateMachine();
  stateMachine._builder = AsyncTaskMethodBuilder.Create();
  stateMachine._state = -1;
  stateMachine._builder.Start<StateMachine>(ref stateMachine);
  return stateMachine._builder.Task;
}

That call to stateMachine._builder.Start is the reason why your zeroed out SynchronizationContext doesn't escape AsyncTaskMethod. Take a look at that Start method here:

https://source.dot.net/#System.Private.CoreLib/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncMethodBuilderCore.cs,73d2481e933d1ebf

Notice how that Start method grabs the current SynchronizationContext before jumping into the (original) guts of your AsyncTaskMethod, and then restores that SynchronizationContext after your AsyncTaskMethod is finished.

---

I'll update the documentation to be more clear about this.

[NickStrupat]

What if you made the API public Task LockAsync(Func<Task> whenLocked, CancellationToken token), and then you did await Task.Run(async () => await whenLocked())?

Would that allow the calling code to change the sync context or use ConfigureAwait(false) without having to worry?

[matthew-a-thomas]

@NickStrupat My apologies for being six months late to reply. But while attempting just now to explain why I don't think that would completely eliminate the problem, I've discovered that I have a serious issue: the code inside Task.Run is actually not considered part of the guarded section. This is very serious for me because I've been advertising otherwise, and also because it means several of my demonstrations of my lock's reentrance have actually not shown that at all! I've recorded this error as #7. I need to clear that up before I can address your suggestion.

[matthew-a-thomas]

@NickStrupat Okay I've convinced myself that #7 is a documentation issue. I was incorrect to say "several of my demonstrations of my lock's reentrance have actually not shown that at all" because the code examples do in fact show that. I can elaborate if you'd like.

Now to address your original suggestion, such a LockAsync method could be a helpful extension method. But it doesn't work around the underlying issue of losing the current SynchronizationContext which is necessary to prevent mutual exclusion violations when the lock is afterward reentered. That's kind of a mouthful so let me break it down.

First, an example of what I mean. The following code will eventually print "There was a mutual exclusion violation on try X". You can run it here.

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

static class ReentrantAsyncLockExtensions
{
    public static async Task LockAsync(
        this ReentrantAsyncLock asyncLock,
        Func<Task> whenLocked,
        CancellationToken token)
    {
        await using var _ = await asyncLock.LockAsync(token);
        await whenLocked();
    }
}

class Program
{
    static async Task Main()
    {
        var asyncLock = new ReentrantAsyncLock();
        var mutualExclusionTest = 0;
        for (long i = 1;; i++)
        {
            await Task.WhenAll(
                Go(),
                Go(),
                Go(),
                Go(),
                Go()
            );
            if (mutualExclusionTest != 0)
            {
                Console.WriteLine($"There was a mutual exclusion violation on try {i:N0}");
                return;
            }

            if (i % 100_000 == 0 && i > 0)
            {
                Console.WriteLine($"So far so good after try {i:N0}");
            }
        }

        Task Go() =>
            asyncLock.LockAsync(async () =>
            {
                // The following two lines are equivalent to the effects of .ConfigureAwait(false)
                SynchronizationContext.SetSynchronizationContext(null);
                await Task.Yield();

                var reenterTask = asyncLock.LockAsync(
                    () =>
                    {
                        // If mutual exclusion is preserved then the following two lines will have no net effect
                        mutualExclusionTest++;
                        mutualExclusionTest--;
                        return Task.CompletedTask;
                    },
                    CancellationToken.None
                );

                // If mutual exclusion is preserved then the following two lines will have no net effect
                mutualExclusionTest++;
                mutualExclusionTest--;

                await reenterTask;
            }, CancellationToken.None);
    }
}

You can see I've written your LockAsync method as an extension method. It is pretty convenient. I'll have to bake it into the next release.

At the heart of it is the Go method. That enters the lock, loses the synchronization context, then races against reentering the lock. You'll have to trust me that SynchronizationContext.SetSynchronizationContext(null) followed by await Task.Yield() is equivalent to awaiting a task with ConfigureAwait(false). I can dig into that with you if you'd like.

I'd like to point out two additional significant facts:

First, if you comment out the two lines that lose the synchronization context then the code chugs along forever without mutual exclusion violations. In other words, the async lock works.

Second, if you instead comment out the reenterTask parts while still losing the synchronization context then the code again chugs along forever without mutual exclusion violations. This could trick you into thinking that ConfigureAwait(false) isn't a great danger after all. That's why I mentioned reentrance when I said ConfigureAwait(false) can lead to mutual exclusion violations when the lock is afterward reentered.