Why locking on AsyncManualResetEvent WaitAsync?

[Patrick W (patawa91)]

Hello,
I recently used the package in a project. I started studying it just so I could understand better. What purpose does the locking do on WaitAsync for the AsyncManualResetEvent ? It is my understanding that all members of TaskCompletionSource are safe for concurrency and same with Task. In his basic example implementation Here he doesn't use it. Thank you.

[Andrew Arnott (AArnott)]

The lock doesn't protect the Task itself (as no one calls into it while within the lock anyway). Rather, the lock is to guard the state of our own AsyncManualResetEvent class itself. Consider where else the lock is taken in the class, around where that field is manipulated. Generally speaking, it could be that if another method is holding the lock, that the field's value might not be something we'd want to return from the WaitAsync method.
Another (very likely) reason is that we use a lock to avoid doing the more advanced memory barriers. Without a kind of memory barrier, we might return a stale value from the field long after it's been changed by a call to Set/Reset due to CPU core memory caching.

[Patrick W (patawa91)]

That makes sense. Thank you for the quick reply. This is a scaled down version (conceptually not exact) of how I'm using it in my project. Unfortunately, I don't have the freedom to enumerate twice to get the 1 record I need before processing the rest. I was going to ask you if using with a Parallel.ForEachAsync like this was appropriate, and I found out the answer is no! :) If you run it, it freezes because it uses all available threads on the box. My thinking was that the WaitAsync would allow threads to do other work. So either a flaw in my understanding of Parallel.ForEachAsync or AsyncManualResetEvent. I understand if you don't answer this if it's not directly related to this library. Thanks again.

var mre = new AsyncManualResetEvent();
            await Parallel.ForEachAsync(Enumerable.Range(1, 1000), async (i, _) =>
            {
                Console.WriteLine("iteration {0}", i);
                if (i == 500)
                {
                    Console.WriteLine("begin set {0}", i);
                    mre.Set();
                }

                await mre.WaitAsync().ConfigureAwait(false);
                Console.WriteLine("released {0}", i);

                // Do work now that we have 500
            }).ConfigureAwait(false);

[Andrew Arnott (AArnott)]

If you run it, it freezes because it uses all available threads on the box

Not exactly. This isn't about the number of threads, since you're writing async code. And a box isn't limited to a certain number of threads. The .NET threadpool limits itself, but it's tens of thousands range.
And the problem has nothing to do with locks taken within the AsyncManualResetEvent.

No, the problem here is evidently that the Parallel.ForEachAsync method is designed to not over-tax your system through too much parallelization. On my machine, it invokes your delegate 32 times before the app hangs. Evidently it's only willing to schedule 32 work items before waiting for one of them to finish before moving on. It's acting like a semaphore, guarding unbounded concurrent work. This is a Good Thing, since it may avoid swamping a user's machine so badly that it becomes unresponsive.
But it does mean you shouldn't make one item need to wait for another item, since that's prone to deadlocks.

[Patrick W (patawa91)]

Thank you.