20
votes

I read this article https://blog.stephencleary.com/2012/07/dont-block-on-async-code.html - however I'm seeing a contradiction:

I'm aware of the problem of deadlocking the UI thread because the UI thread blocks waiting for an async operation to complete, but the same async operation is synchronized to the UI thread context - consequently the async operation cannot enter the UI thread, so the UI thread won't stop waiting.

The article tells us the workaround is to not block on the UI thread, otherwise you need to use ConfigureAwait(false) everywhere:

You would have to use for every await in the transitive closure of all methods called by the blocking code, including all third- and second-party code.

However later on in the article the author writes:

Preventing the Deadlock
There are two best practices (both covered in my intro post) that avoid this situation:

  1. In your “library” async methods, use ConfigureAwait(false) wherever possible.
  2. Don’t block on Tasks; use async all the way down.

I'm seeing a contradiction here - in the "don't do this" section he writes that having to use ConfigureAwait(false) everywhere would be the consequence of blocking the UI thread - but in his "best practices" list he then tells us to do just that: "use ConfigureAwait(false) wherever possible." - though I suppose "wherever possible" would exclude third-party code, but in the case where there is no third-party code the result is the same if I block the UI thread or not.

As for my specific problem, here is my current code in a WPF MVVM project:

MainWindowViewModel.cs

private async void ButtonClickEventHandler()
{
    WebServiceResponse response = await this.client.PushDinglebopThroughGrumbo();

    this.DisplayResponseInUI( response );
}

WebServiceClient.cs

public class PlumbusWebServiceClient {

    private static readonly HttpClient _client = new HttpClient();

    public async Task<WebServiceResponse> PushDinglebopThroughGrumbo()
    {
        try
        {
            using( HttpResponseMessage response = await _client.GetAsync( ... ) )
            {
                if( !response.IsSuccessStatusCode ) return WebServiceResponse.FromStatusCode( response.StatusCode );

                using( Stream versionsFileStream = await response.Content.ReadAsStreamAsync() )
                using( StreamReader rdr = new StreamReader( versionsFileStream ) )
                {
                    return await WebServiceResponse.FromResponse( rdr );
                }
            }
        }
        catch( HttpResponseException ex )
        {
            return WebServiceResponse.FromException( ex );
        }
    }
}

If I understand the document correctly, I should add ConfigureAwait(false) to every await that is not in a method that has code that needs to run on the UI thread - which is every method inside my PushDinglebopThroughGrumbo method, but also all code in WebServiceResponse.FromResponse (which calls await StreamReader.ReadLineAsync). But what about any third-party code I call which also performs await operations on the StreamReader? I won't have access to their source-code so that would be impossible.

I'm also a bit put-off by having to place ConfigureAwait(false) everywhere - I thought the point of the await keyword was to eliminate explicit task library calls - shouldn't there be a different keyword for resume-context-free awaiting then? (e.g. awaitfree).

So should my code then look like this?

MainWindowViewModel.cs

(unmodified, same as above)

WebServiceClient.cs

public class PlumbusWebServiceClient {

    private static readonly HttpClient _client = new HttpClient();

    public async Task<WebServiceResponse> PushDinglebopThroughGrumbo()
    {
        try
        {
            using( HttpResponseMessage response = await _client.GetAsync( ... ).ConfigureAwait(false) ) // <-- here
            {
                if( !response.IsSuccessStatusCode ) return WebServiceResponse.FromStatusCode( response.StatusCode );

                using( Stream versionsFileStream = await response.Content.ReadAsStreamAsync().ConfigureAwait(false) )  // <-- and here
                using( StreamReader rdr = new StreamReader( versionsFileStream ) )
                {
                    return await WebServiceResponse.FromResponse( rdr ).ConfigureAwait(false);  // <-- and here again, and inside `FromResponse` too
                }
            }
        }
        catch( HttpResponseException ex )
        {
            return WebServiceResponse.FromException( ex );
        }
    }
}

...I would have thought that calling ConfigureAwait(false) would only be necessary on the topmost await call inside the PlumbusWebServiceClient method - i.e. the GetAsync call.

If I do need to apply it everywhere, could I simplify it to an extension method?

public static ConfiguredTaskAwaitable<T> CF<T>(this Task<T> task) {
    return task.ConfigureAwait(false);
}

using( HttpResponseMessage response = await _client.GetAsync( ... ).CF() )
{
    ...
}

...though this doesn't alleviate all of the fiddliness.

Update: Second example

Here is some async code I wrote that exports my application's settings to a simple text file - I can't help but think it doesn't feel right, is this really the correct way to do this?

class Settings
{
    public async Task Export(String fileName)
    {
        using( StreamWriter wtr = new StreamWriter( fileName, append: false ) )
        {
            await ExportSetting( wtr, nameof(this.DefaultStatus     ), this.DefaultStatus                         ).ConfigureAwait(false);
            await ExportSetting( wtr, nameof(this.ConnectionString  ), this.ConnectionString                      ).ConfigureAwait(false);
            await ExportSetting( wtr, nameof(this.TargetSystem      ), this.TargetSystem.ToString("G")            ).ConfigureAwait(false);
            await ExportSetting( wtr, nameof(this.ThemeBase         ), this.ThemeBase                             ).ConfigureAwait(false);
            await ExportSetting( wtr, nameof(this.ThemeAccent       ), this.ThemeAccent                           ).ConfigureAwait(false);
            await ExportSetting( wtr, nameof(this.ShowSettingsButton), this.ShowSettingsButton ? "true" : "false" ).ConfigureAwait(false);
            await ExportSetting( wtr, nameof(this.ShowActionsColumn ), this.ShowActionsColumn  ? "true" : "false" ).ConfigureAwait(false);
            await ExportSetting( wtr, nameof(this.LastNameFirst     ), this.LastNameFirst      ? "true" : "false" ).ConfigureAwait(false);
            await ExportSetting( wtr, nameof(this.TitleCaseCustomers), this.TitleCaseCustomers ? "true" : "false" ).ConfigureAwait(false);
            await ExportSetting( wtr, nameof(this.TitleCaseVehicles ), this.TitleCaseVehicles  ? "true" : "false" ).ConfigureAwait(false);
            await ExportSetting( wtr, nameof(this.CheckForUpdates   ), this.CheckForUpdates    ? "true" : "false" ).ConfigureAwait(false);
        }
    }

    private static async Task ExportSetting(TextWriter wtr, String name, String value)
    {
        String valueEnc = Uri.EscapeDataString( value ); // to encode line-breaks, etc.

        await wtr.WriteAsync( name ).ConfigureAwait(false);
        await wtr.WriteAsync( '=' ).ConfigureAwait(false);
        await wtr.WriteLineAsync( valueEnc ).ConfigureAwait(false);
    }
}
1
@PeterDuniho My apologies for being unclear. I've updated my opening paragraph to be more precise. I don't believe I've misunderstood the article - I'm just seeking clarification.Dai
@PeterDuniho No, "If I understand the document correctly, I should add ConfigureAwait(false) to every await that is not in a method that has code that needs to run on the UI thread" is entirely correct. It's just: don't do that to avoid deadlocks, do that because it's the right thing to do, because there's no point for non-UI code in waiting for the UI thread to become available.user743382
I'd probably change the call from your VM to await Task.Run(() => this.client.PushDinglebopThroughGrumbo()). Then each await in your service code has no context to capture and you can stop worrying about it. If that code was in a library, I'd put in the ConfigureAwait(false) everywhere.Charles Mager

1 Answers

8
votes

If I understand the document correctly, I should add ConfigureAwait(false) to every await that is not in a method that has code that needs to run on the UI thread

Yes. The default behaviour in UI applications is for code after await to continue on the UI thread. When the UI thread is busy, but your code does not need to access the UI, there is no point in waiting for the UI thread to become available.

(Note: this intentionally leaves out some details not relevant here.)

But what about any third-party code I call which also performs await operations on the StreamReader?

So long as you avoid deadlocks through other means, this will only affect performance, not correctness. And the problem of potentially poorly performing third party code is not a new problem.

In other words: follow both best practices.

I'm also a bit put-off by having to place ConfigureAwait(false) everywhere - I thought the point of the await keyword was to eliminate explicit task library calls - shouldn't there be a different keyword for resume-context-free awaiting then? (e.g. awaitfree).

ConfigureAwait isn't a TPL method.

await is generalised so that it can be used on arbitrary types so long as they support the required methods. For a random example, you might add an extension method for a Task to return a type that allows the code after await to continue in a new dedicated thread. This would not require a new version of the compiler with a new keyword.

But yes, it's a long name.

If I do need to apply it everywhere, could I simplify it to an extension method?

Yes, that is perfectly fine.


Here is some async code I wrote that exports my application's settings to a simple text file - I can't help but think it doesn't feel right, is this really the correct way to do this?

As I wrote in the comments, I wouldn't use that approach at all myself... but if you do want to, you've got a lot of code duplication in there that you can get rid of. And with that gone, it doesn't look nearly as bad any more.

/* SettingsCollection omitted, but trivially implementable using
   Dictionary<string, string>, NameValueCollection,
   List<KeyValuePair<string, string>>, whatever. */

SettingsCollection GetAllSettings()
{
     return new SettingsCollection
     {
         { nameof(this.DefaultStatus     ), this.DefaultStatus                         },
         { nameof(this.ConnectionString  ), this.ConnectionString                      },
         { nameof(this.TargetSystem      ), this.TargetSystem.ToString("G")            },
         { nameof(this.ThemeBase         ), this.ThemeBase                             },
         { nameof(this.ThemeAccent       ), this.ThemeAccent                           },
         { nameof(this.ShowSettingsButton), this.ShowSettingsButton ? "true" : "false" },
         { nameof(this.ShowActionsColumn ), this.ShowActionsColumn  ? "true" : "false" },
         { nameof(this.LastNameFirst     ), this.LastNameFirst      ? "true" : "false" },
         { nameof(this.TitleCaseCustomers), this.TitleCaseCustomers ? "true" : "false" },
         { nameof(this.TitleCaseVehicles ), this.TitleCaseVehicles  ? "true" : "false" },
         { nameof(this.CheckForUpdates   ), this.CheckForUpdates    ? "true" : "false" }
     };
}

public async Task Export(String fileName)
{
    using( StreamWriter wtr = new StreamWriter( fileName, append: false ) )
        foreach (var setting in GetAllSettings())
            await ExportSetting( wtr, setting.Key, setting.Value ).ConfigureAwait(false);
}