11

I have a timer in a Windows Service, and there is a call made to an async method inside the timer_Elapsed event handler:

protected override void OnStart(string[] args)
{
     timer.Start();    
}  

private async void timer_Elapsed(object sender, ElapsedEventArgs e)
{
    _timer.Stop();
    await DoSomething();          
    _timer.Start();
}

Is the above code ok? Firstly, I have read that async void is not a good idea. Secondly, why would I need the timer_Elapsed method to be async in the first place? It's not like the elapsed event handler us going to get called in parallel by multiple callers. However, if I don't make the method async, then my logic will break because timer will start before DoSomething completes.

So, is making timer_Elapsed async the correct approach?

Prabhu
  • 12,995
  • 33
  • 127
  • 210
  • What you have is fine from the async composition point of view. `async void` is something that should be avoided, but in your scenario you can't really get away from it. Having said that, why would you even use a timer in the first place when you can just change your `OnStart` to `async void` and just `await DoSomething(); await Task.Delay(...);` inside a loop? – Kirill Shlenskiy Jul 29 '14 at 03:20
  • @Kirill thanks for the suggestion. Is it better to use Task.Delay in a loop, rather than using a timer? – Prabhu Jul 29 '14 at 03:23
  • 2
    Just like `await`, timers provide a way of composing your asynchronous work. My suggestion stems from the fact that you're *already* using one way of doing it (which is `await`), so why not stick with it for everything instead of mixing different composition methods? This is highly subjective though, and at the end of the day you should use the approach that you are comfortable with. There is nothing inherently "wrong" with timers as such. – Kirill Shlenskiy Jul 29 '14 at 03:31

1 Answers1

15

Async void should be avoided; it should only be used for event handlers. Timer.Elapsed is an event handler. So, it's not necessarily wrong here.

The timer_Elapsed method has to be async because it contains an await. This is the way the keywords work: async rewrites the method as an asynchronous state machine, which enables the await keyword.

Stephen Cleary
  • 437,863
  • 77
  • 675
  • 810
  • 1
    Thanks @Stephen. Interestingly, it's from your blog that I learnt about async/await just this morning, great write up! http://blog.stephencleary.com/2012/02/async-and-await.html – Prabhu Jul 29 '14 at 03:17
  • I was also wondering about how to use async/await correctly in a recursive method: http://stackoverflow.com/questions/25007709/what-is-the-correct-way-to-use-async-await-in-a-recursive-method – Prabhu Jul 29 '14 at 03:21
  • @Maxim: I'm afraid I don't understand your comment. Perhaps you should ask your own question? – Stephen Cleary Jul 21 '17 at 16:59
  • There are 2 timers: System.Timers.Timer and System.Threading.Timer. As I understand first one has Elapsed event and it is OK to have async event handler. 2nd one accepts callback as parameter in constructor. So what is about that timer? Can that callback also be async void? – Maxim Jul 21 '17 at 17:05
  • @Maxim: Sure, it's *logically* an event handler. – Stephen Cleary Jul 21 '17 at 17:11
  • What if the `Elapsed` event handler contains a single line of code calling an asynchronous method (returning a Task), but without awaiting it? Does it make a difference in practice? – mcont Aug 01 '19 at 17:22
  • @mcont: Yes. Exceptions are handled differently. `async void` methods will re-raise exceptions directly on the context that was active at the beginning of the method; this is essentially the same behavior as synchronous event handlers. Ignoring a `Task` is different - if that `Task` faults, then its exception is ignored. – Stephen Cleary Aug 01 '19 at 22:11
  • AFAIK, one of the reasons `async void` is bad is that it can bring down the whole app down if there is an exception. If it applies here, how do we ensure we catch exceptions? Is `try/catch` around `await DoSomething()` (or the whole body) enough? – bybor Dec 16 '21 at 13:57
  • @bybor: Yes; if you cannot avoid `async void`, then I'd recommend wrapping the entire body in a `try` and being careful about what code goes into the `catch`. – Stephen Cleary Dec 16 '21 at 15:11