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);
2 Answers 2
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
ConfigureAwait
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).
-
\$\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– mitomed2015年11月16日 16:10:20 +00:00Commented Nov 16, 2015 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– Peter Taylor2015年11月16日 17:05:34 +00:00Commented Nov 16, 2015 at 17:05
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.
-
\$\begingroup\$ Thanks @Steve, but it's not the point to avoid having blocking calls? \$\endgroup\$mitomed– mitomed2015年11月16日 15:40:38 +00:00Commented Nov 16, 2015 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– Steve2015年11月16日 16:36:48 +00:00Commented Nov 16, 2015 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– Peter Taylor2015年11月16日 17:04:48 +00:00Commented Nov 16, 2015 at 17:04