0
\$\begingroup\$

Related to this question (and even this Stack Overflow's one) I have been trying to avoid blocking the code. So what I understood it's that is preferable to make all the methods that rely on an async one async themselve (thus not defeating the purpose of asynchronous). Therefore I have changed my code to do the following (I include here more "layers" than in the questions I linked as I had to change accordingly those).

My API, exposing a method:

[Route("api/postconfiguration")]
public async Task<IHttpActionResult> PostConfiguration(configurationData configurationData)
{
    try
    {
        await _configurationRequestHandler.HandlePostRequest(configurationData);
        return Ok();
    }
    catch (Exception e)
    {
        // Log error
        return InternalServerError(e);
    }
}

That uses this method:

public async Task HandlePostRequest(Data Data)
{
    var configuration = await _ConfigurationService.GetConfigurationAsync(Data.AgentId);

    // Use that configuration
}

And this would be the method calling the external api

public class ConfigurationApiClient : IConfigurationService
{
    private readonly HttpClient _client;
    private const string resourcePath = "configuration/{0}";

    public ConfigurationApiClient(IConfigurationManager configurationManager)
    {
        _client = new HttpClient();
        _client.BaseAddress = new Uri(configurationManager.GetAppSetting("ConfigurationApiUrl"));
        _client.DefaultRequestHeaders.Accept.Clear();
        _client.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json"));
    }

    public async Task<Configuration> GetConfigurationAsync(int agentId)
    {
        try
        {
            var response = await _client.GetAsync(String.Format(resourcePath, agentId));
            response.EnsureSuccessStatusCode();
            return await response.Content.ReadAsAsync<Configuration>();
        }
        catch (HttpRequestException ex)
        {
            throw new Exception(string.Format("Exception when getting  configuration for agent: {0}", agentId), ex);
        }
    }

}

This is my understanding of what I should do for making it asynchronous, but would appreciate if someone points me any error.

I am using the API on one client myself, which does this:

public async Task DeployConfiguration(Package package, EFEnvironment environment)
{
    using (var client = new HttpClient())
    {
        // Prepare the configurationData

        var agentTasks = agents.Select(a =>
            {
                var configurationPostRequest = new ConfigurationData
                    {
                        // Create it
                    };

                return client.PostAsJsonAsync("api/postconfiguration", configurationPostRequest);
            });

        await Task.WhenAll(agentTasks);
    }
}

Which is just called inside a sync method:

_deployApiService.DeployConfiguration(package, environment);
\$\endgroup\$
1
\$\begingroup\$

Your code falls into a classic trap, which IMO is a design error by Microsoft. 99% of the time, await Foo(); should be await Foo().ConfigureAwait(false);. The semi-official guidance is

To summarize this third guideline, you should use Configure­Await when possible. Context-free code has better performance for GUI applications and is a useful technique for avoiding deadlocks when working with a partially async codebase. The exceptions to this guideline are methods that require the context.

I think that it would have been far better for ConfigureAwait(false) to be the default and to require ConfigureAwait(true) in those cases which require context, but it's too late now for Microsoft to change this, so we should all get into the habit of using ConfigureAwait(false) by default and in those rare cases documenting the reason for leaving it off (or even explicitly using ConfigureAwait(true) and supplementing it with a comment which gives the reason).

\$\endgroup\$
  • \$\begingroup\$ Thanks do you mean use that in all the awaits as the only change? or do as per @Steve suggestion and remove all of them apart from the one calling the external api? \$\endgroup\$ – mitomed Nov 16 '15 at 16:10
  • 1
    \$\begingroup\$ Yes, that's the only essential change. (I did consider also suggesting removing a stray semicolon from the ConfigurationApiClient constructor, but I thought that would distract). \$\endgroup\$ – Peter Taylor Nov 16 '15 at 17:05
0
\$\begingroup\$

From my understanding of ASP.NET async/await, awaiting doesn't return control to the calling method, it will return the request thread to the thread pool, allowing it to be used for other requests. You should only need to have an await on the blocking calls at the very bottom.

\$\endgroup\$
  • \$\begingroup\$ Thanks @Steve, but it's not the point to avoid having blocking calls? \$\endgroup\$ – mitomed Nov 16 '15 at 15:40
  • \$\begingroup\$ @mitomed The problem of blocking calls on a web service is that the thread that is handling the request can't do anything while the blocking call is being made. The async/await frees up that thread to handle other requests during a blocking I/O call. The wait time for the user remains the same. \$\endgroup\$ – Steve Nov 16 '15 at 16:36
  • \$\begingroup\$ Ideally async methods will be called only from async methods and with await: this is the "async/await all the way" pattern which OP referenced in the title of the question. If you only await at the bottom of the call stack then the next method up gets a (probably running) Task and can't do anything useful with it except block, tying up the thread. \$\endgroup\$ – Peter Taylor Nov 16 '15 at 17:04

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.