Are non-thread-safe functions async safe?

Question

Consider the following async function that modifies a non-thread-safe list:

async Task AddNewToList(List<Item> list)
{
    // Suppose load takes a few seconds
    Item item = await LoadNextItem();
    list.Add(item);
}

Simply put: Is this safe?

My concern is that one may invoke the async method, and then while it's loading (either on another thread, or as an I/O operation), the caller may modify the list.

Suppose that the caller is partway through the execution of list.Clear(), for example, and suddenly the Load method finishes! What will happen?

Will the task immediately interrupt and run the list.Add(item); code? Or will it wait until the main thread is done with all scheduled CPU tasks (ie: wait for Clear() to finish), before running the code? Edit: Since I've basically answered this for myself below, here's a bonus question: Why? Why does it immediately interrupt instead of waiting for CPU bound operations to complete? It seems counter-intuitive to not queue itself up, which would be completely safe.

Edit: Here's a different example I tested myself. The comments indicate the order of execution. I am disappointed!

TaskCompletionSource<bool> source;
private async void buttonPrime_click(object sender, EventArgs e)
{
    source = new TaskCompletionSource<bool>();  // 1
    await source.Task;                          // 2
    source = null;                              // 4
}

private void buttonEnd_click(object sender, EventArgs e)
{
    source.SetResult(true);                     // 3
    MessageBox.Show(source.ToString());         // 5 and exception is thrown
}

Answer 1

No, its not safe. However also consider that the caller might also have spawned a thread and passed the List to its child thread before calling your code, even in a non async environment, which will have the same detrimental effect.

So; although not safe, there is nothing inherently thread-safe about receiving a List from a caller anyway - there is no way of knowing whether the list is actually being processed from other threads that your own.

Answer 2

Short answer

You always need to be careful using async.

Longer answer

It depends on your SynchronizationContext and TaskScheduler, and what you mean by "safe."

When your code awaits something, it creates a continuation and wraps it in a task, which is then posted to the current SynchronizationContext's TaskScheduler. The context will then determine when and where the continuation will run. The default scheduler simply uses the thread pool, but different types of applications can extend the scheduler and provide more sophisticated synchronization logic.

If you are writing an application that has no SynchronizationContext (for example, a console application, or anything in .NET core), the continuation is simply put on the thread pool, and could execute in parallel with your main thread. In this case you must use lock or synchronized objects such as ConcurrentDictionary<> instead of Dictionary<>, for anything other than local references or references that are closed with the task.

If you are writing a WinForms application, the continuations are put in the message queue, and will all execute on the main thread. This makes it safe to use non-synchronized objects. However, there are other worries, such as deadlocks. And of course if you spawn any threads, you must make sure they use lock or Concurrent objects, and any UI invocations must be marshaled back to the UI thread. Also, if you are nutty enough to write a WinForms application with more than one message pump (this is highly unusual) you'd need to worry about synchronizing any common variables.

If you are writing an ASP.NET application, the SynchronizationContext will ensure that, for a given request, no two threads are executing at the same time. Your continuation might run on a different thread (due to a performance feature known as thread agility), but they will always have the same SynchronizationContext and you are guaranteed that no two threads will access your variables at the same time (assuming, of course, they are not static, in which case they span across HTTP requests and must be synchronized). In addition, the pipeline will block parallel requests for the same session so that they execute in series, so your session state is also protected from threading concerns. However you still need to worry about deadlocks.

And of course you can write your own SynchronizationContext and assign it to your threads, meaning that you specify your own synchronization rules that will be used with async.

See also How do yield and await implement flow of control in .NET?

Answer 3

Assuming the "invalid acces" occures in LoadNextItem(): The Task will throw an exception. Since the context is captured it will pass on to the callers thread so list.Add will not be reached.

So, no it's not thread-safe.

Answer 4

Yes I think that could be a problem.

I would return item and add to the list on the main tread.

private async void GetIntButton(object sender, RoutedEventArgs e)
{
    List<int> Ints = new List<int>();
    Ints.Add(await GetInt());
}
private async Task<int> GetInt()
{
    await Task.Delay(100);
    return 1;
}

But you have to call from and async so I do not this this would work either.