Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

ConfigureAwait(False) doesn't change context after ContinueWith()

I don't know if I am doing something wrong or I found a bug in the Async library, but I have seen an issue when running some async code after I came back to the Synchronized context with continueWith().

UPDATE: The code now runs

using System;
using System.ComponentModel;
using System.Net.Http;
using System.Threading.Tasks;
using System.Windows.Forms;

namespace WindowsFormsApplication1
{
internal static class Program
{
    [STAThread]
    private static void Main()
    {
        Application.EnableVisualStyles();
        Application.SetCompatibleTextRenderingDefault(false);
        Application.Run(new Form1());
    }
}

public partial class Form1 : Form
{
    public Form1()
    {
        InitializeComponent();
        MainFrameController controller = new MainFrameController(this);
        //First async call without continueWith
        controller.DoWork();

        //Second async call with continueWith
        controller.DoAsyncWork();
    }

    public void Callback(Task<HttpResponseMessage> task)
    {
        Console.Write(task.Result); //IT WORKS

        MainFrameController controller =
            new MainFrameController(this);
        //third async call
        controller.DoWork(); //IT WILL DEADLOCK, since ConfigureAwait(false) in HttpClient DOESN'T  change context
    }
}


internal class MainFrameController
{
    private readonly Form1 form;

    public MainFrameController(Form1 form)
    {
        this.form = form;
    }

    public void DoAsyncWork()
    {
        Task<HttpResponseMessage> task = Task<HttpResponseMessage>.Factory.StartNew(() => DoWork());
        CallbackWithAsyncResult(task);
    }

    private void CallbackWithAsyncResult(Task<HttpResponseMessage> asyncPrerequisiteCheck)
    {
        asyncPrerequisiteCheck.ContinueWith(task =>
            form.Callback(task),
            TaskScheduler.FromCurrentSynchronizationContext());
    }

    public HttpResponseMessage DoWork()
    {
        MyHttpClient myClient = new MyHttpClient();
        return myClient.RunAsyncGet().Result;
    }
}

internal class MyHttpClient
{
    public async Task<HttpResponseMessage> RunAsyncGet()
    {
        HttpClient client = new HttpClient();
        return await client.GetAsync("https://www.google.no").ConfigureAwait(false);
    }
}

partial class Form1
{
    private IContainer components;

    protected override void Dispose(bool disposing)
    {
        if (disposing && (components != null))
        {
            components.Dispose();
        }
        base.Dispose(disposing);
    }

    #region Windows Form Designer generated code
    private void InitializeComponent()
    {
        this.components = new System.ComponentModel.Container();
        this.AutoScaleMode = System.Windows.Forms.AutoScaleMode.Font;
        this.Text = "Form1";
    }

    #endregion
}
}
  • The HttpClient code which is async runs well the first time.
  • Then, I run the second async code and return to the UI context with ContinueWith, and it works well.
  • I run the HttClient code again, but it deadlock because this time ConfigureAwait(false) does not change the context.
like image 395
Max.Moraga Avatar asked Dec 24 '22 06:12

Max.Moraga


1 Answers

The main problem in your code is due to StartNew and ContinueWith. ContinueWith is dangerous for the same reasons that StartNew is dangerous, as I describe on my blog.

In summary: StartNew and ContinueWith should only be used if you're doing dynamic task-based parallelism (which this code is not).

The actual problem is that HttpClient.GetAsync doesn't use (the equivalent of) ConfigureAwait(false); it's using ContinueWith with its the default scheduler argument (which is TaskScheduler.Current, not TaskScheduler.Default).

To explain in more detail...

The default scheduler for StartNew and ContinueWith is not TaskScheduler.Default (the thread pool); it's TaskScheduler.Current (the current task scheduler). So, in your code, DoAsyncWork as it currently is does not always execute DoWork on the thread pool.

The first time DoAsyncWork is called, it will be called on the UI thread but without a current TaskScheduler. In this case, TaskScheduler.Current is the same as TaskScheduler.Default, and DoWork is called on the thread pool.

Then, CallbackWithAsyncResult invokes Form1.Callback with a TaskScheduler that runs it on the UI thread. So, when Form1.Callback calls DoAsyncWork, it is called on the UI thread with a current TaskScheduler (the UI task scheduler). In this case, TaskScheduler.Current is the UI task scheduler, and DoAsyncWork ends up calling DoWork on the UI thread.

For this reason, you should always specify a TaskScheduler when calling StartNew or ContinueWith.

So, this is a problem. But it's not actually causing the deadlock you're seeing, because ConfigureAwait(false) should allow this code to just block the UI instead of deadlocking.

It's deadlocking because Microsoft made the same mistake. Check out line 198 here: GetContentAsync (which is called by GetAsync) uses ContinueWith without specifying a scheduler. So, it's picking up the TaskScheduler.Current from your code, and will not ever complete its task until it can run on that scheduler (i.e., the UI thread), causing the classic deadlock.

There's nothing you can do to fix the HttpClient.GetAsync bug (obviously). You'll just have to work around it, and the easiest way to do that is to avoid having a TaskScheduler.Current. Ever, if you can.

Here's some general guidelines for asynchronous code:

  • Don't ever use StartNew. Use Task.Run instead.
  • Don't ever use ContinueWith. Use await instead.
  • Don't ever use Result. Use await instead.

If we just do minimal changes (replacing StartNew with Run and ContinueWith with await), then DoAsyncWork always executes DoWork on the thread pool, and the deadlock is avoided (since await uses the SynchronizationContext directly and not a TaskScheduler):

public void DoAsyncWork()
{
    Task<HttpResponseMessage> task = Task.Run(() => DoWork());
    CallbackWithAsyncResult(task);
}

private async void CallbackWithAsyncResult(Task<HttpResponseMessage> asyncPrerequisiteCheck)
{
    try
    {
        await asyncPrerequisiteCheck;
    }
    finally
    {
        form.Callback(asyncPrerequisiteCheck);
    }
}

However, it's always questionable to have a callback scenario with Task-based asynchrony, because Tasks themselves have the power of callbacks within them. It looks like you're trying to do a sort of asynchronous initialization; I have a blog post on asynchronous construction that shows a few possible approaches.

Even something really basic like this would be a better design than callbacks (again, IMO), even though it uses async void for initialization:

public partial class Form1 : Form
{
    public Form1()
    {
        InitializeComponent();
        MainFrameController controller = new MainFrameController();
        controller.DoWork();

        Callback(controller.DoAsyncWork());
    }

    private async void Callback(Task<HttpResponseMessage> task)
    {
        await task;
        Console.Write(task.Result);

        MainFrameController controller = new MainFrameController();
        controller.DoWork();
    }
}

internal class MainFrameController
{
    public Task<HttpResponseMessage> DoAsyncWork()
    {
        return Task.Run(() => DoWork());
    }

    public HttpResponseMessage DoWork()
    {
        MyHttpClient myClient = new MyHttpClient();
        var task = myClient.RunAsyncGet();
        return task.Result;
    }
}

Of course, there's other design problems here: namely, DoWork is blocking on a naturally-asynchronous operation, and DoAsyncWork is blocking a thread pool thread on a naturally-asynchronous operation. So, when Form1 calls DoAsyncWork, it's awaiting a thread pool task that's blocked on an asynchronous operation. Async-over-sync-over-async, that is. You may also benefit from my blog series on Task.Run etiquette.

like image 144
Stephen Cleary Avatar answered Dec 29 '22 05:12

Stephen Cleary