A case when ConfigureAwait(false) causes an error

2019-06-22 13:47发布

Suppose I have written a library which relies on async methods:

namespace MyLibrary1
{
    public class ClassFromMyLibrary1
    {
        public async Task<string> MethodFromMyLibrary1(string key, Func<string, Task<string>> actionToProcessNewValue)
        {
            var remoteValue = await GetValueByKey(key).ConfigureAwait(false);

            //do some transformations of the value
            var newValue = string.Format("Remote-{0}", remoteValue);

            var processedValue = await actionToProcessNewValue(newValue).ConfigureAwait(false);

            return string.Format("Processed-{0}", processedValue);
        }

        private async Task<string> GetValueByKey(string key)
        {
            //simulate time-consuming operation
            await Task.Delay(500).ConfigureAwait(false);

            return string.Format("ValueFromRemoteLocationBy{0}", key);
        }
    }
}

I followed the recommendations of using ConfigureAwait(false) (like in this post) everywhere in my library. Then I use it in synchronous way from my test app and get a failure:

namespace WpfApplication1
{
    /// <summary>
    /// Interaction logic for MainWindow.xaml
    /// </summary>
    public partial class MainWindow : Window
    {
        public MainWindow()
        {
            InitializeComponent();
        }

        private void Button1_OnClick(object sender, RoutedEventArgs e)
        {
            try
            {
                var c = new ClassFromMyLibrary1();

                var v1 = c.MethodFromMyLibrary1("test1", ActionToProcessNewValue).Result;

                Label2.Content = v1;
            }
            catch (Exception ex)
            {
                System.Diagnostics.Trace.TraceError("{0}", ex);
                throw;
            }
        }

        private Task<string> ActionToProcessNewValue(string s)
        {
            Label1.Content = s;
            return Task.FromResult(string.Format("test2{0}", s));
        }
    }
}

The failure is:

WpfApplication1.vshost.exe Error: 0 : System.InvalidOperationException: The calling thread cannot access this object because a different thread owns it. at System.Windows.Threading.Dispatcher.VerifyAccess() at System.Windows.DependencyObject.SetValue(DependencyProperty dp, Object value) at System.Windows.Controls.ContentControl.set_Content(Object value) at WpfApplication1.MainWindow.ActionToProcessNewValue(String s) in C:\dev\tests\4\WpfApplication1\WpfApplication1\MainWindow.xaml.cs:line 56 at MyLibrary1.ClassFromMyLibrary1.d__0.MoveNext() in C:\dev\tests\4\WpfApplication1\WpfApplication1\MainWindow.xaml.cs:line 77 --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult() at WpfApplication1.MainWindow.d__1.MoveNext() in C:\dev\tests\4\WpfApplication1\WpfApplication1\MainWindow.xaml.cs:line 39 Exception thrown: 'System.InvalidOperationException' in WpfApplication1.exe

Obviously the error happens because the awaiters in my library discard current WPF context.

From the other hand, after removing the ConfigureAwait(false) everywhere in the library I obviously get a deadlock instead.

There is more detailed example of code which explains some constraints that I have to deal with.

So how can I address this issue? What is the best approach here? Do I still need to follow the best practice regarding ConfigureAwait?

PS, In real scenario I have many classes and methods therefore tons of such async calls in my library. It's nearly impossible to find out if some particular async call requires context or not (see comments to @Alisson response) to fix it. I don't care about performance though, at least at this point. I'm looking for some general approach to address this issue.

3条回答
再贱就再见
2楼-- · 2019-06-22 14:10

In my opinion, you should redesign your library API to not mix a callback-based API with a Task-based API. At least in your example code there's no compelling case to be made to do that and you've nailed one reason not do do that - it is hard to control the context in which your callback runs.

I'd change your library API to be like so:

namespace MyLibrary1
{
    public class ClassFromMyLibrary1
    {
        public async Task<string> MethodFromMyLibrary1(string key)
        {
            var remoteValue = await GetValueByKey(key).ConfigureAwait(false);
            return remoteValue;
        }

        public string TransformProcessedValue(string processedValue)
        {
            return string.Format("Processed-{0}", processedValue);
        }

        private async Task<string> GetValueByKey(string key)
        {
            //simulate time-consuming operation
            await Task.Delay(500).ConfigureAwait(false);

            return string.Format("ValueFromRemoteLocationBy{0}", key);
        }
    }
}

And call it like so:

   private async void Button1_OnClick(object sender, RoutedEventArgs e)
    {
        try
        {
            var c = new ClassFromMyLibrary1();

            var v1 = await c.MethodFromMyLibrary1("test1");
            var v2 = await ActionToProcessNewValue(v1);
            var v3 = c.TransformProcessedValue(v2);

            Label2.Content = v3;
        }
        catch (Exception ex)
        {
            System.Diagnostics.Trace.TraceError("{0}", ex);
            throw;
        }
    }

    private Task<string> ActionToProcessNewValue(string s)
    {
        Label1.Content = s;
        return Task.FromResult(string.Format("test2{0}", s));
    }
查看更多
够拽才男人
3楼-- · 2019-06-22 14:21

Normally a library will document if a callback will be guaranteed to be on the same thread that called it, if it is not documented the safest option will be to assume it does not. Your code example (and the 3rd party you are working with from what I can tell from your comments) fall under the category of "Not guaranteed". In that situation you just need to check if you need to do a Invoke from inside the callback method and do it, you can call Dispatcher.CheckAccess() and it will return false if you need to invoke before using the control.

private async Task<string> ActionToProcessNewValue(string s)
{
    //I like to put the work in a delegate so you don't need to type 
    // the same code for both if checks
    Action work = () => Label1.Content = s;
    if(Label1.Dispatcher.CheckAccess())
    {
        work();
    }
    else
    {
        var operation = Label1.Dispatcher.InvokeAsync(work, DispatcherPriority.Send);

        //We likely don't need .ConfigureAwait(false) because we just proved
        // we are not on the UI thread in the if check.
        await operation.Task.ConfigureAwait(false);
    }

    return string.Format("test2{0}", s);
}

Here is a alternate version with a syncronous callback instead of a async one.

private string ActionToProcessNewValue(string s)
{
    Action work = () => Label1.Content = s;
    if(Label1.Dispatcher.CheckAccess())
    {
        work();
    }
    else
    {
        Label1.Dispatcher.Invoke(work, DispatcherPriority.Send);
    }

    return string.Format("test2{0}", s);
}

Here is another version if you wanted to get the value from Label1.Content instead of assigning it, this also does not need to use async/await inside the callback.

private Task<string> ActionToProcessNewValue(string s)
{
    Func<string> work = () => Label1.Content.ToString();
    if(Label1.Dispatcher.CheckAccess())
    {
        return Task.FromResult(work());
    }
    else
    {
        return Label1.Dispatcher.InvokeAsync(work, DispatcherPriority.Send).Task;
    }
}

IMPORTANT NOTE: all of these methods will cause your program to deadlock if you don't get rid of the .Result in the button click handler, the Dispatcher.Invoke or the Dispatcher.InvokeAsync in the callback will never start while it is waiting for .Result to return and .Result will never return while it is waiting for the callback to return. You must change the click handler to be async void and do a await instead of the .Result.

查看更多
在下西门庆
4楼-- · 2019-06-22 14:21

Actually, you're receiving a callback in your ClassFromMyLibrary1 and you can't assume what it'll do (like updating a Label). You don't need ConfigureAwait(false) in your class library, as the same link you provided gives us an explanation like this:

As asynchronous GUI applications grow larger, you might find many small parts of async methods all using the GUI thread as their context. This can cause sluggishness as responsiveness suffers from "thousands of paper cuts".

To mitigate this, await the result of ConfigureAwait whenever you can.

By using ConfigureAwait, you enable a small amount of parallelism: Some asynchronous code can run in parallel with the GUI thread instead of constantly badgering it with bits of work to do.

Now take a read here:

You should not use ConfigureAwait when you have code after the await in the method that needs the context. For GUI apps, this includes any code that manipulates GUI elements, writes data-bound properties or depends on a GUI-specific type such as Dispatcher/CoreDispatcher.

You're doing exactly the opposite. You're trying to update GUI in two points, one in your callback method, and another here:

var c = new ClassFromMyLibrary1();

var v1 = c.MethodFromMyLibrary1("test1", ActionToProcessNewValue).Result;

Label2.Content = v1; // updating GUI...

That's why removing ConfigureAwait(false) solves your problem. Also, you can make your button click handler async and await your ClassFromMyLibrary1 method call.

查看更多
登录 后发表回答