14

I'm working on a Web API project which uses Azure's managed cache service to cache database results in memory to improve response times and alleviate duplicate traffic to the database. When attempting to put a new item in the cache, occasionally a cache-specific exception will be thrown with a code of DataCacheErrorCode.RetryLater. Naturally, in order to retry later without needing to block on this method I made it async and await Task.Delay to try again a short time later. Previously a developer had hardcoded a Thread.Sleep in there that was really hurting application performance.

The method signature looks something similar to this now:

public static async Task Put(string cacheKey, object obj)

Following this change I get ~75 compiler warnings from all the other places in the application that called the formerly synchronous version of Put indicating:

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.

In this case, since Put doesn't return anything, it makes sense to me to let this operation fire-and-forget as I don't see any reason to block execution of the method that called it. I'm just wondering if there are any dangers or pitfalls for allowing a lot of these fire-and-forget Tasks running in the background as Put can be called quite often. Or should I await anyway since 99% of the time I won't get the retry error and the Task will finish almost immediately. I just want to make sure that I'm not incurring any penalties for having too many threads (or something like that).

DavidRR
  • 18,291
  • 25
  • 109
  • 191
Jesse Carter
  • 20,062
  • 7
  • 64
  • 101

4 Answers4

10

If there is a chance Put will throw any other exception for any kind of reason, and you don't use await Put each time you're inserting an object to the cache, the exceptions will be swallowed inside the returned Task which isn't being awaited. If you're on .NET 4.0, this exception will be re-thrown inside the Finalizer of that Task.. If you're using .NET 4.5, it will simply be ignored (and that might not be desirable).

Want to make sure that I'm not incurring any penalties for having too many threads or something like that.

Im just saying this to make things clear. When you use Task.Delay, you aren't spinning any new threads. A Task isn't always equal to a new thread being spun. Specifically here, Task.Delay internally uses a Timer, so there aren't any thread overheads (except for the thread which is currently being delayed if you do use await).

Yuval Itzchakov
  • 146,575
  • 32
  • 257
  • 321
  • Thanks for the good feedback. Currently all exceptions other than RetryLater are caught and properly logged and we are on .NET 4.5 so I'm confident in letting these tasks fire and forget. – Jesse Carter Nov 17 '14 at 17:42
  • Technically Timer uses a [separate] thread from a thread pool, so does Task.Delay() – alexm Nov 17 '14 at 17:47
  • 2
    @alexm It uses a thread pool thread for a very short period of time after the time elapses to run the handler. It doesn't consume a thread pool thread while waiting. This means that a thread is only ever being used when it has productive work to do, which is exactly what you want (and cannot avoid). – Servy Nov 17 '14 at 17:51
  • 1
    @Servy: again, I agree with your statement, this is why i put a word "Technically" :) – alexm Nov 17 '14 at 17:53
  • @JesseCarter: To avoid the warning, use code like `var _ = Put(...);`. Also consider using `Task.Run` to ensure the code is not attempting to resume on a request context that is no longer there. – Stephen Cleary Nov 17 '14 at 18:46
  • @StephenCleary Use `Task.Run` inside Web-Api? I've read many of your posts discouraging that pattern. – Yuval Itzchakov Nov 17 '14 at 18:48
  • @StephenCleary Thanks for the tips can you elaborate at all or point me in the direction of some documentation regarding the potential for attempting to resume on a request context that is no longer there? – Jesse Carter Nov 17 '14 at 18:48
  • @StephenCleary I also thought I had read stuff you've posted online discouraging the use of Task.Run – Jesse Carter Nov 17 '14 at 18:50
  • 2
    I discourage `await Task.Run(..)` to fake `async` handlers on ASP.NET, which is the vast majority of attempted `Task.Run` usage. The only real `Run` use case is fire-and-forget. And easily 99% of fire-and-forget ASP.NET scenarios should *not* be using fire-and-forget. Updating cache is one of the *very few* use cases where fire-and-forget (and `Task.Run`) is acceptable. Regarding request context, I mean the `AspNetSynchronizationContext` for that request, which is [captured by default when you use `async`](http://blog.stephencleary.com/2012/02/async-and-await.html) (as I describe on my blog). – Stephen Cleary Nov 17 '14 at 18:58
  • 1
    @StephenCleary Isn't using `ConfiguareAwait(false)` preferable to using `Task.Run` in case you simply want to ignore the context? – Yuval Itzchakov Nov 17 '14 at 19:01
  • 2
    @YuvalItzchakov: Roughly equivalent. Something like `await Task.Yield().ConfigureAwait(false)` will yield the current thread and resume the method on a thread pool thread; whereas `Task.Run` will queue its delegate to the thread pool. I think `Task.Run` has a clearer intent (code without context is separated), but `ConfigureAwait` may be more efficient. – Stephen Cleary Nov 17 '14 at 19:35
  • I don't think you can actually call `ConfigureAwait` that way. `'System.Runtime.CompilerServices.YieldAwaitable' does not contain a definition for 'ConfigureAwait'` – Ed Ball Nov 17 '14 at 21:08
3

The warning is telling you that you're getting fire and forget behavior in a location where you may not actually want to fire and forget. If you really do want to fire and forget and don't have a problem continuing on without ever knowing when the operation finishes, or if it even finished successfully, then you can safely ignore the warning.

Servy
  • 202,030
  • 26
  • 332
  • 449
  • + 1; To mark method as explicit "fire and forget" one can change return type from async Task to async void. – alexm Nov 17 '14 at 17:42
  • @alexm async void doesn't place nice with Web API I've received exceptions in the past indicating that it is not allowed when I tried to use it – Jesse Carter Nov 17 '14 at 17:47
  • 2
    @alexm That would make it so that the only way the method could ever possibly be called is through fire and forget, which is different from *allowing* the method to be awaited but simply performing fire and forget semantics in certain situations for certain callers. – Servy Nov 17 '14 at 17:50
  • @Jesse Carter: you are correct, I am just suggesting how to get rid of the warnings.. Calling async Task method without awaiting it has the same effect though. – alexm Nov 17 '14 at 17:51
  • @alexm well trading off compiler warnings for exceptions that will break my application doesn't seem desirable at this point :P – Jesse Carter Nov 17 '14 at 17:53
  • @Jesse Carter: I am afraid you have to deal with the exceptions in both cases. – alexm Nov 17 '14 at 17:54
  • @alexm I'm not really sure what you're getting at. Not awaiting an async Task is quite alright and I've already stated that exceptions are properly handled there. You are correct about the fire and forget nature of async void however in my specific scenario it is not allowed so at this point I would consider it bad advice for people who might come here in the future. – Jesse Carter Nov 17 '14 at 17:57
  • @JesseCarter: I am just saying that not awaiting an async Task _means_ "fire and forget", just like "async void". They have exactly same effect, less the warnings. – alexm Nov 17 '14 at 18:01
  • @alexm Yah definitely agree with you there and worth nothing :) – Jesse Carter Nov 17 '14 at 18:02
  • `async void` doesn't mean fire and forget — if you get an exception on an async void method, your whole process can be killed. `async void` exists only for async event handlers, because event handling methods must be `void` and shouldn't be used in any other circumstances – Miguel Ventura Mar 30 '20 at 21:34
  • @MiguelVentura If you want to be more precise about it, an async void method is a method where the caller is not expected to observe the results of the asynchronous actions. Event handlers are the most common use case for that, but certainly not the *only* use case for that. While the other situations may be rare, and certainly should only be done by someone who understand why it's appropriate for no caller to be observing the results of the method, doesn't mean that you shouldn't ever do it. – Servy Mar 30 '20 at 23:13
2

One negative result of releasing a task to run unawaited is the compiler warnings themselves - 75 compiler warnings are a problem in and of themselves and they hide real warnings.

If you want to signal the compiler that you're intentionally not doing anything with the result of the task, you can use a simple extension method that does nothing, but satisfies the compiler's desire for explicitness.

// Do nothing.
public static void Release(this Task task)
{
}

Now you can call 

UpdateCacheAsync(data).Release();

without any compiler warnings.

https://gist.github.com/lisardggY/396aaca7b70da1bbc4d1640e262e990a

Avner Shahar-Kashtan
  • 14,492
  • 3
  • 37
  • 63
  • 1
    Starting in C# 7 you can explicitly discard the task by assigning it to the discard variable: `_ = UpdateCacheAsync(data);` – DBN May 22 '20 at 04:51
0

Recommended ASP.NET way is 

HostingEnvironment.QueueBackgroundWorkItem(WorkItem);

...

async Task WorkItem(CancellationToken cancellationToken)
{
    try { await ...} catch (Exception e) { ... }
}

BTW Not catching/re-throw on thread other then ASP.NET thread can cause server process to be crashed/restarted

Yuri
  • 301
  • 2
  • 6