Should Timer be waiting for callback to finish before firing a new one?

Question

I am using the System.Threading.Timer class in one of my projects and I've noticed that the callback methods are called before the previous ones get to finish which is different than what I was expecting.

For example the following code

var delta = new TimeSpan(0,0,1);
var timer = new Timer(TestClass.MethodAsync, null, TimeSpan.Zero, delta);

static class TestClass
{
    static int i = 0;
    public static async void MethodAsync(object _)
    {
        i++;
        Console.WriteLine("method " + i + "started");

        await Task.Delay(10000);
        
        Console.WriteLine("method " + i + "finished");
    }
}

has this output

method 1started
method 2started
method 3started
method 4started
method 5started
method 6started
method 7started
method 8started
method 9started
method 10started
method 11started
method 11finished
method 12started
method 12finished

Which of course is not thread safe. What I was expecting is that a new call would be made to the callback method after the previous call has succeeded and additionally after the delta period is elapsed.

What I am looking for is where in the docs from Microsoft is this behavior documented and maybe if there is a built in way to make it wait for the callback calls to finish before starting new ones

I would have timer call MethodAsync() only once and restart the timer at the end of the method. — emilsteen, Jun 21 '22 at 21:00
If thread safety is needed you can wrap the code in MethodAsync() with lock(). — emilsteen, Jun 21 '22 at 21:03
@emilsteen I know we can have workarounds, I'm looking more for the official documentation and a common robust approach if there is any — meJustAndrew, Jun 21 '22 at 22:51
I'm looking at Microsoft [System.Timer](https://learn.microsoft.com/en-us/dotnet/api/system.timers.timer?view=net-6.0) and it says "If processing of the `Elapsed` event lasts longer than Interval, the event might be raised again on another `ThreadPool` thread. In this situation, the event handler should be reentrant." My read is that the answer to your question **Should Timer be waiting** would be 'no' if using `System.Timer` but there are other `Timer` classes of course. Which one are you using? — IVSoftware, Jun 21 '22 at 23:21
I'm also looking at Microsoft documentation for [await](https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/operators/await) operator which says, "The await operator doesn't block the thread that evaluates the `async` method. When the `await` operator suspends the enclosing `async` method, **the control returns to the caller of the method**. My interpretation is that even if the `Timer` _did_ wait for the callback to finish, your use of the `await` operator gives an explicit directive to return from the callback _before_ executing the Task.Delay. — IVSoftware, Jun 21 '22 at 23:44

IVSoftware · Answer 1 · 2022-06-22T01:03:48.977

What I am looking for is where in the docs from Microsoft is this behavior documented...

System.Threading.Timer

If processing of the Elapsed event lasts longer than Interval, the event might be raised again on another ThreadPool thread. In this situation, the event handler should be reentrant.

System.Timers.Timer

The callback method executed by the timer should be reentrant, because it is called on ThreadPool threads.

For System.Windows.Forms.Timer this post asserts that the event does wait. The documentation doesn't seem very specific, but in the Microsoft Timer.Tick Event official example the code shows turning the timer off and on in the handler. So it seems that, regardless, steps are taken to prevent ticks and avoid reentrancy.

...and if there is a built in way to make it wait for the callback calls to finish before starting new ones.

According to the first Microsoft link (you might consider this a workaround, but it's straight from the horse's mouth):

One way to resolve this race condition is to set a flag that tells the event handler for the Elapsed event to ignore subsequent events.

The way I personally go about achieving this objective this is to call Wait(0) on the synchronization object of choice as a robust way to ignore reentrancy without having timer events piling up in a queue:

static SemaphoreSlim _sslim = new SemaphoreSlim(1, 1);
public static async void MethodAsync(object _)
{
    if (_sslim.Wait(0))
    {
        try
        {
            i++;
            Console.WriteLine($"method {i} started @ {DateTime.Now}");

            await Task.Delay(10000);

            Console.WriteLine($"method {i} finished @ {DateTime.Now}");
        }
        catch (Exception ex)
        {
            Debug.Assert(false, ex.Message);
        }
        finally
        {
            _sslim.Release();
        }
    }
}

In which case your MethodAsync generates this output:

score 3 · Accepted Answer · answered Jun 22 '22 at 02:10

The problem of overlapping event handlers is inherent with the classic multithreaded .NET timers (the System.Threading.Timer and the System.Timers.Timer). Attempting to solve this problem while remaining on the event-publisher-subscriber model is difficult, tricky, and error prone. The .NET 6 introduced a new timer, the PeriodicTimer, that attempts to solve this problem once and for all. Instead of handling an event, you start an asynchronous loop and await the PeriodicTimer.WaitForNextTickAsync method on each iteration. Example:

class TestClass : IDisposable
{
    private int i = 0;
    private PeriodicTimer _timer;

    public async Task StartAsynchronousLoop()
    {
        if (_timer != null) throw new InvalidOperationException();
        _timer = new(TimeSpan.FromSeconds(1));
        while (await _timer.WaitForNextTickAsync())
        {
            i++;
            Console.WriteLine($"Iteration {i} started");
            await Task.Delay(10000); // Simulate an I/O-bound operation
            Console.WriteLine($"Iteration {i} finished");
        }
    }

    public void Dispose() => _timer?.Dispose();
}

This way there is no possibility for overlapping executions, provided that you will start only one asynchronous loop.

The await _timer.WaitForNextTickAsync() returns false when the timer is disposed. You can also stop the loop be passing a CancellationToken as argument. When the token is canceled, the WaitForNextTickAsync will complete with an OperationCanceledException.

In case the periodic action is not asynchronous, you can offload it to the ThreadPool, by wrapping it in Task.Run:

await Task.Run(() => Thread.Sleep(10000)); // Simulate a blocking operation

If you are targeting a .NET platform older than .NET 6, you can find alternatives to the PeriodicTimer here.

Should Timer be waiting for callback to finish before firing a new one?

2 Answers2

Linked