1

I'm using System.Threading.Timer to run some things that are async

It means my callbacks are async void.

private async void CallbackAsync(object state) {
    _timer.Change(Timeout.Infinite, Timeout.Infinite);
    await SomethingAsync();
    _timer.Change(TimeToNextCallback, Timeout.Inifinite);
}

With non async callbacks I have used code like this to shut down the timer safely.

using (var waitHandle = new AutoResetEvent(false)) {
    if (_timer.Dispose(waitHandle))        {
        waitHandle.WaitOne();
    }
}

My understanding is that this will not work with async callbacks. The callback will return on the first await and the Timer is unaware of scheduled continations. I.e the code above might return before the continuations has completed (and the callback might try to change a disposed timer).

First, is my understanding correct?

Second, what is the correct way to wait for async callbacks to finish?

adrianm
  • 14,468
  • 5
  • 55
  • 102
  • 1
    Standard [cancel task](https://stackoverflow.com/questions/10134310/how-to-cancel-a-task-in-await) is seems like a possible route... Also I'd drop timer altogether and just `await Task.Delay(canelationToken)`... – Alexei Levenkov Nov 27 '19 at 20:31
  • Can you describe what you're trying to achieve by using the timers? – Gabriel Luci Nov 27 '19 at 20:46
  • Sorry, for late reply. The timer is old code that I am trying to update to support async callbacks. – adrianm Nov 28 '19 at 10:00

1 Answers1

2

Your understanding is correct that your Timer will be unaware that your CallbackAsync function is still running and thus your call to Dispose(WaitHandle) will return and signal your WaitHandle immediately, at which point a call to CallbackAsync may make a call to Timer.Change after the Timer has been disposed.

There are a few red flags here:

  1. You are using async void. This is almost always a bad idea because async functions should return a Task, otherwise there is nothing for a caller to await.
  2. You are mixing Thread and Task paradigms.

Because TimerCallback does not support Tasks, the only way to make this work correctly while still using Timer would be to make your callback synchronous by dropping the async/await keywords and using Task.Wait to block until the Task returned by SomethingAsync is complete. As others have mentioned it would also be wise to add support for cancellation by creating a CancellationToken and passing it to SomethingAsync, that way when you call Dispose you will have the ability to cancel a running callback (this CancellationToken will need to be passed by SomethingAsync to any other async functions it calls and/or periodically checked for cancellation as SomethingAsync is executing).

EDIT: If you want to shift to a more natural Task based way of doing this see Run async method regularly with specified interval

Paul Wheeler
  • 18,988
  • 3
  • 28
  • 41
  • Was hoping for a simple change in existing code to support async callbacks. Seems like a rewrite to use Task.Delay instead is what is needed. Thanks for your input. – adrianm Nov 28 '19 at 10:04
  • If you must use Timer than the simplest solution is to drop the async/await and call `.Wait()` on the task returned by the async code. – Paul Wheeler Nov 29 '19 at 01:35