Do race conditions exist when using just async/await?

Viewed 2003

.NET 5.0

using Microsoft.VisualStudio.TestTools.UnitTesting;
using System.Threading.Tasks;
using System;
using System.Collections.Generic;

namespace AsyncTest
{
    [TestClass]
    public class AsyncTest
    {
        public async Task AppendNewIntVal(List<int> intVals)
        {
            await Task.Delay(new Random().Next(15, 45));

            intVals.Add(new Random().Next());
        }

        public async Task AppendNewIntVal(int count, List<int> intVals)
        {
            var appendNewIntValTasks = new List<Task>();

            for (var a = 0; a < count; a++)
            {
                appendNewIntValTasks.Add(AppendNewIntVal(intVals));
            }

            await Task.WhenAll(appendNewIntValTasks);
        }

        [TestMethod]
        public async Task TestAsyncIntList()
        {
            var appendCount = 30;
            var intVals = new List<int>();

            await AppendNewIntVal(appendCount, intVals);

            Assert.AreEqual(appendCount, intVals.Count);
        }
    }
}

The above code compiles and runs, but the test fails with output similar to:

Assert.AreEqual failed. Expected:<30>. Actual:<17>.

In the above example the "Actual" value is 17, but it varies between executions.

I know I am missing some understanding around how asynchronous programming works in .NET as I'm not getting the expected output.

From my understanding, the AppendNewIntVal method kicks off N number of tasks, then waits for them all to complete. If they have all completed, I'd expect they would have each appended a single value to the list but that's not the case. It looks like there's a race condition but I didn't think that was possible because the code is not multithreaded. What am I missing?

3 Answers

Yes, if you don't await each awaitable immediately, i.e. here:

appendNewIntValTasks.Add(AppendNewIntVal(intVals));

This line is in async terms equivalent to (in thread-based code) Thread.Start, and we now have no safety around the inner async code:

intVals.Add(new Random().Next());

which can now fail in the same concurrency ways when two flows call Add at the same time. You should also probably avoid new Random(), as that isn't necessarily random (it is time based on many framework versions, and can end up with two flows getting the same seed).

So: the code as shown is indeed dangerous.

The obviously safe version is:

        public async Task AppendNewIntVal(int count, List<int> intVals)
        {
            for (var a = 0; a < count; a++)
            {
                await AppendNewIntVal(intVals);
            }
        }

It is possible to defer the await, but you're explicitly opting into concurrency when you do that, and your code needs to handle it suitably defensively.

Yes, race conditions do exist.

Async methods are basically tasks that can potentially run in parallel, depending on a task scheduler they are submitted to. The default one is ThreadPoolTaskScheduler, which is a wrapper around ThreadPool. Thus, if you submit your tasks to a scheduler (thread pool) that can execute multiple tasks in parallel, you are likely going to run into race conditions.

You could make your code a bit safer:

lock (intVals) intVals.Add(new Random().Next());

But then this opens up another can of worms :)

If you are interested in more details about async programming, see this link. Also this article is quite useful and explains best practices in asynchronous programming.

Happy (asynchronous) coding!

Yes, race conditions are indeed possible when using async/await in a way that introduces concurrency. To introduce concurrency you must:

  1. Launch multiple asynchronous operations concurrently, i.e. launch the next operation without awaiting the completion of the previous operation, and,
  2. Have no ambient synchronization¹ mechanism in place, namely a SynchronizationContext, that would synchronize the execution of the continuations of the asynchronous operations.

In your case both conditions are met, so the continuations are running on multiple threads concurrently. And since the List<T> class is not thread-safe, you get undefined behavior.

To see what effect a SynchronizationContext has in a situation like this, you can install the Nito.AsyncEx.Context package and do this:

[TestMethod]
public void TestAsyncIntList()
{
    AsyncContext.Run(async () =>
    {
        var appendCount = 30;
        var intVals = new List<int>();

        await AppendNewIntVal(appendCount, intVals);

        Assert.AreEqual(appendCount, intVals.Count);
    });
}

FYI many types of applications install automatically a SynchronizationContext when launched (WPF and Windows Forms to name a few). Console applications do not though, hence the need to be extra cautious when writing an async-enabled console application.

¹ It's worth noting that the similar-looking terms synchronized/unsynchronized and synchronous/asynchronous are mostly unrelated. This can be a source of confusion for someone who is not familiar with these terms. The first term is about preventing multiple threads from accessing a shared resource concurrently. The second term is about doing something without blocking a thread.

Related