11

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:

  1. Await an initial call that gets me some information I need to work on
  2. Work on that information synchronously
  3. 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?

migrated from codereview.stackexchange.com Jan 26 '14 at 21:27

This question came from our site for peer programmer code reviews.

  • The code seems to me as short and understandable as possible. Anything I can think of introduces unnecessary complexity. – Euphoric Jan 27 '14 at 16:11
  • @Euphoric: Should I bother awaiting my last call? What would happen if I didn't and it threw? Would it be any different as it is currently? – Will Jan 27 '14 at 17:00
3

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.

0

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

  • 2
    You should never fire and forget code with Task.Run. All tasks should be awaited. If you don't await the Task, and the Task is garbage-collected in a state where the exception is "unobserved", it will cause an UnobservedTaskException to be raised in the runtime and could crash your application depending on the framework version. – Triynko Jan 7 at 17:39

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.