Okay, it sounds odd, but the code is very simple and explains the situation well.
public virtual async Task RemoveFromRoleAsync(AzureTableUser user, string role)
{
AssertNotDisposed();
var roles = await GetRolesForUser(user);
roles.Roles = RemoveRoles(roles.Roles, role);
await Run(TableOperation.Replace(roles));
}
(I know I’m talking kind of in the abstract below, but the above is an actual method from what will be actual production code that is actually doing what I’m asking about here, and I’m actually interested in your actually reviewing it for correctness vis a vis the async/await pattern.)
I’m encountering this pattern more and more often now that I’m using async/await more. The pattern consists of the following chain of events:
- Await an initial call that gets me some information I need to work on
- Work on that information synchronously
- Await a final call that saves the updated work
The above code block is typically how I go about handling these methods. I await the first call, which I have to because it is asynchronous. Next, I do the work that I need to do which isn’t IO or resource bound, and so isn’t async. Finally, I save my work which is also an async call, and out of cargo-cult I await it.
But is this the most efficient/correct way to handle this pattern? It seems to me I could skip awaiting the last call, but what if it fails? And should I use a Task method such as ContinueWith to chain my synchronous work with the original call? I’m just at a point right now where I’m not sure if I’m handling this correctly.
Given the code in the example, is there a better way to handle this async/sync/async method call chain?
2
Yes, I think this is the right way to do it.
You can’t skip the second await. If you did, the method would seem to complete too early (before the removal was actually done), and you would never find out if the removal failed.
I don’t see how would ContinueWith() or anything like that help here. You could use it to avoid using await, but it would make your code more complicated and less readable. And that’s the whole point of await: making writing asynchronous code simpler, when compared with using continuations.
The way to handle this pattern is to ensure that all I/O is asynchronous. Synchronous I/O methods cause the current thread to block while it waits for a response from the I/O destination (network, file-system, etc).
Another thing to consider is that await should be used when you need a return value or when you need the awaited code to finish before doing something else. If you don’t need either of those things, you can “fire and forget” your async method with Task.Run.
So, for the most efficient use of computing resources, if RemoveRoles does any I/O, it should become await RemoveRolesAsync and the I/O methods called by RemoveRolesAsync should also be async (and possibly awaited).
If performance is not your utmost concern, then it’s fine to do some synchronous I/O on an async thread. It is a technical debt though. (In this case, you might want to call the first async method with ConfigureAwait, depending on where the code is running.)
Here’s a more in-depth look at the best practices – https://msdn.microsoft.com/en-us/magazine/jj991977.aspx
Here are some notes about the behavior of ConfigureAwait in different environments like ASP.NET, WebAPI, etc – https://stackoverflow.com/questions/13489065/best-practice-to-call-configureawait-for-all-server-side-code
1