6

I see some colleague code where he chooses to not await the database call and just return the Task. E.g.

public Task<UpdateResult> AddActivityAsync(ClaimsPrincipal principal, Activity activity)
{
    return _userManager.SaveToDatabaseAsync(principal, activity);
}

as _userManager.SaveToDatabaseAsync is async, I would have implemented this way

public async Task<UpdateResult> AddActivityAsync(ClaimsPrincipal principal, 
                                                                      Activity activity)
{
    return await _userManager.SaveToDatabaseAsync(principal, activity);
}

The calling method of this method always await it:

await _profile.AddActivityAsync(..., ...)

Is there any benefit to not make the inner method async and just return the Task, letting the caller await it ? I thought we had to write Async all the way down...

Maksim Simkin
  • 9,561
  • 4
  • 36
  • 49
Raphaël
  • 427
  • 5
  • 14

2 Answers2

4

It depends on the situation.

You must await, for example, if the code to be awaited is bound to an object that is in an using context (or gets manually disposed):

using (SomeDisposableType obj = ...)
{
    await obj.SomeOperationAsync();
}

If you returned the task without awaiting it, then the method might complain that the object had been disposed before it could have finished its job. (not all disposable objects throw an ObjectDisposedException if you attempt to perform something on them after they get disposed but it is generally a good idea to assume so). Consider the opposite:

using (SomeDisposableType obj = ...)
{
    // This returns the Task that represents the async operation,
    // and because it returns, the control goes out of the using
    // scope, and therefore obj gets disposed.
    return obj.SomeOperationAsync();
}

So there are cases when awaiting is necessary. If that is not the case, I can't really think of a reason why you couldn't return the Task itself.

Balázs
  • 2,929
  • 2
  • 19
  • 34
  • Yes, I agree that you MUST await in some cases. But about the cases you don't need to as you just return the inner result, which one is a better practice? – Raphaël Mar 11 '17 at 02:57
  • Very important caveat in must await if inside using. Fell foul of this code uncaught for months... Ideally compiler should block tasks being returned inside using block, hopefully one day! I found this question and is related to mine https://stackoverflow.com/questions/63584024/simple-injector-instance-is-requested-outside-the-context-of-an-active-async-s/63584565#63584565. I think of this answer is updated to Balazs would as it covers this important point – morleyc Aug 25 '20 at 20:28
1

You do not need to await a method which returns a Task<T>, the code will just run asynchronously if you have the async keyword on the method. You colleague has removed that and so is running synchronously deliberately and lets the calling code implement it if they so choose.

It depends at which layer this code is being run. If it is deep it may be better to let the higher code layers choose ansynchronous execution and maintain control there.

You definitely don't need to 'write async all the way down'. The purpose of the async keyword is simply to mark a method as being able to return a Task<T> and be able to use the await keyword. The await keyword is what invokes the framework to execute the Task and provide asynchronous processing.

TL;DR: It's a choice.

I welcome anyone improving or correcting me on this as I am no guru.

Matt W
  • 11,753
  • 25
  • 118
  • 215
  • 3
    `async/await` is *not* multi-threading - http://blog.stephencleary.com/2013/11/there-is-no-thread.html – Lloyd Mar 10 '17 at 11:06
  • So if it is just a wrapper around an async method, no meet to mark the wrapper async just return the task is fine? If I put it async, will it add overhead each time I await? In that cas for simple wrapper do not await seems even better to me – Raphaël Mar 10 '17 at 11:55
  • 1
    @lloyd To my eyes, that article does not make it clear what it is that is happening instead of a thread. What it appears to be saying is that a thread was used, but not necessarily a new one. So, await is causing a new context? – Matt W Mar 10 '17 at 12:26