0

In my ASP.NET Core app I have few (dozen) methods created in the form of

public async Task DoStuff() 
{ 
   // ...
}

So, no return type.

Later, I realized that I accidentally forgot to include await on such methods here and there, in the caller methods (which were obviously left without async keyword as async "zombie virus" hasn't spread out that far yet).

During the execution, there were no unwanted consequences whatsoever.

The thing is, Visual Studio generates no warning message either when such thing happens and I'm asking myself, is there actually any danger present of lefting out the await in such cases? I know that await should be applied on every async method naturally, but I don't really understand the reasons behind this when caller practically has no return value to use. Perhaps something with catching exceptions?

I haven't found any clear answer to this for the general statement is 'simply include await'. And believe it or not, this async/await thing, to which I'm relatively new, keeps biting me repeatedly from time to time.

NamespaceNotFound
  • 11
  • 2
  • 2
    `So, no return type` - no, the return type is `Task`, as opposed to e.g. [`async void`](https://stackoverflow.com/a/41687322/11683). – GSerg Feb 07 '21 at 12:08
  • 1
    Does this answer your question? [C# async/await with/without awaiting (fire and forget)](https://stackoverflow.com/questions/46053175/c-sharp-async-await-with-without-awaiting-fire-and-forget) – GSerg Feb 07 '21 at 12:09
  • @GSerg sorry, I did meant in the context of async/await terminology, as no return type is produced by the method to by wrapped by the Task. Should work on my wording, I guess. – NamespaceNotFound Feb 07 '21 at 12:22
  • @GSerg thanks for the suggestion, I see there that Stephen partially answered my question, by looking at the explanation to #1 provided. Having this in mind, can you perhaps comment the following: if I handle the exceptions in the said methods themselves (the ones not properly awaited), could we then say that all is fine and well? Caller won't know about the exceptions, yes, but they won't be entirely lost. – NamespaceNotFound Feb 07 '21 at 12:28
  • Is it possible to include in the question a more realistic example than the `DoStuff` method? It would be easier to answer the question if this method had a name that indicates what it does. Also by showing how and where you call it would help us estimate whether omitting the `await` has consequences, and what kind of consequences. Btw according to the [guidelines](https://learn.microsoft.com/en-us/dotnet/standard/asynchronous-programming-patterns/task-based-asynchronous-pattern-tap#naming-parameters-and-return-types), the `DoStuff` method should have the name `DoStuffAsync`. – Theodor Zoulias Feb 07 '21 at 12:56
  • @TheodorZoulias I usually do include 'Async' keyword, when I am actually being a sane programmer. As for the purpose, they are actually either mailer or logger methods that perhaps could be described as ''fire-and-forget'' ones, as they are not so business-critical but, as Stephen pointed out, this is not a way to do things. – NamespaceNotFound Feb 08 '21 at 08:28

1 Answers1

5

During the execution, there were no unwanted consequences whatsoever.

I disagree. The resulting code is dangerous. ASP.NET pre-Core was able to detect similar situations and throw an exception ("An asynchronous module or handler completed while an asynchronous operation was still pending"). For technical reasons, ASP.NET Core cannot detect this situation so you don't get that "safety net" exception, but the situation itself is still just as bad.

The thing is, Visual Studio generates no warning message either when such thing happens

You don't get CS4014 ("Because this call is not awaited, execution of the current method continues before the call is completed. Consider applying the await operator to the result of the call.")?

is there actually any danger present of lefting out the await in such cases? I know that await should be applied on every async method naturally, but I don't really understand the reasons behind this when caller practically has no return value to use. Perhaps something with catching exceptions?

Yes, there are dangers. Task (even without a result type) is used for two things: for the caller to know when the operation completes, and for the caller to detect exceptions from that operation.

So, the one issue is that exceptions are silently swallowed. More specifically, exceptions from the async method are captured by the async state machine and placed on the returned Task, which is then ignored.

if I handle the exceptions in the said methods themselves (the ones not properly awaited), could we then say that all is fine and well?

No, because the other issue still exists: the caller doesn't know when the asynchronous operation completes. This is particularly important to know in ASP.NET, because the result should not be sent until the operation is complete. Any kind of "fire and forget" code on ASP.NET lives outside the request/response lifetime; i.e., it's request-extrinsic code.

I go into some detail on my blog about why request-extrinsic code is dangerous. In summary, your ASP.NET handler may complete too soon, and in that case, the request-extrinsic code may get "lost". At the very least, whatever it's doing won't be done by the time the response is sent; and in the case of a regular shutdown (e.g., rolling upgrades), it might not get done at all.

Stephen Cleary
  • 437,863
  • 77
  • 675
  • 810
  • Thank you for the answer, Stephen. I traced down all the cases where await is missing. As I said, there were no warning as those asyncs that return only plain Task were called from the sync methods. Still, I have one case where I'm unable to await the async called - this are the methods of an WCF IClientMessageInspector, and they contain a 'ref' as a parameter e.g. void AfterReceiveReply(ref Message reply, object correlationState). The async called is not something bussines-critical (it's a logger) but still, do you propose any workaround to this? – NamespaceNotFound Feb 08 '21 at 08:24
  • @NamespaceNotFound: For loggers, I recommend having a separate dedicated logging thread, and make all the logging methods synchronously add to an in-memory queue that is processed by that thread. So that way, logging doesn't require asynchrony. – Stephen Cleary Feb 08 '21 at 15:57