Runtime: API Proposal: Setting to disable overlapping Timer callbacks

[davidfowl]

By default timer callbacks are queued to the thread pool and not waited on. This results in potentially overlapping timer callbacks firing (which can be problematic when code doesn't expect this). It seems like it would be extremely useful to have an option to disable this behavior (block overlapping calls to the callback).

It should be a constructor parameter but Timers already have 5 constructors and I don't want to explode things. Open to ideas on how to enable this type of functionality.

cc @stephentoub @kouvel

[stephentoub]

This is already possible. Developers do this just by creating non-repeating timers and then setting the timer to fire again in the callback:
C# new Timer(s => { // ... do work that must be serialized ((Timer)s).Change(1_000, -1); // ... do work that can overlap }).Change(1000, -1);
This has several advantages. First, it doesn't need any additional constructor (this is one of the reasons this constructor exists, passing the Timer instance into the callback so it can be manipulated). Second, it allows for the non-overlapping piece to only be a subset of the timer callback; it could be the whole thing if the developer wanted to by putting the Change call at the end, or at the beginning to enable full overlap, or somewhere in the middle if a portion of the callback should be a serialized and a portion can run concurrently. And third, it enables the timer period to vary each time.

I don't think it's worth adding yet another variant for this. If, however, we decided it was really important, I'd want to see https://github.com/dotnet/corefx/issues/32866 modified to accomodate it, e.g. with a flags enum such that this option could be set, rather than requiring yet another overload.

[GSPP]

A related issue is that the timer callback can be invoked after the timer has been stopped. Also, when you stop a timer you sometimes like to be able to wait until the last callback invocation has completed. This helps with resource cleanup and deterministic execution.

Starting a new one-shot timer repeatedly leads to drift. Especially when the callback takes some time to run, the effective frequency is lower.

There's a fairly simple and newbie-friendly pattern:

while (true) {
 await Task.Delay(...);
 Work();
}

Plus a CancellationTokenSource for shutting down. This is a lot of machinery and there's the drift issue. It's OK, but it could be better.

I'm personally using a utility class that handles both overlapping and "dangling" invocations. Pretty much each time I need a timer I need this protection. It would be nice if the framework handled this nicely. I'd estimate that 90% of usages of Timer in the wild are subtly broken due to these issues. The resulting bugs are timing specific and can easily slip through testing.

[stephentoub]

when you stop a timer you sometimes like to be able to wait until the last callback invocation has completed. This helps with resource cleanup and deterministic execution.

Timer already supports that. Either use Dispose that takes a WaitHandle, which it will set when all work has quiesced, or in .NET Core 3.0 use and await DisposeAsync.

[davidfowl]

I'm personally using a utility class that handles both overlapping and "dangling" invocations. Pretty much each time I need a timer I need this protection. It would be nice if the framework handled this nicely. I'd estimate that 90% of usages of Timer in the wild are subtly broken due to these issues. The resulting bugs are timing specific and can easily slip through testing.

https://github.com/aspnet/AspNetCore/blob/0a79d34f62a2e153d7104a88b487b75ef8860bb7/src/SignalR/common/Shared/TimerAwaitable.cs

Usage:

https://github.com/aspnet/AspNetCore/blob/9557630c0a470618b30679e8effcf019b1c74164/src/SignalR/clients/csharp/Client.Core/src/HubConnection.cs#L1774-L1781

This type was trying to handle:

• Overlapping timer events
• Async execution of timer callbacks
• Race between execution and disposal
• Disabling ExecutionContext capture (my favorite topic!)
• Simplified programming model

PS: This could an IAsyncEnumerable<TimerEvent> where each "item" is just a timer event firing.

[GSPP]

The using (timer) while (await timer) syntax is an interesting idea.

[omariom]

Though it doesn't fire events in parallel but
it is not very convenient that it fires the next event immediately if processing of the previous event took longer than period:
```C#
var timer = new TimerAwaitable(TimeSpan.FromSeconds(1), period: TimeSpan.FromSeconds(1));
timer.Start();

using (timer)
{
    var sw = Stopwatch.StartNew();

    while (await timer)
    {
        sw.ElapsedMilliseconds.Dump();

        Thread.Sleep(1100);

        sw.Restart();
    }
}

Output:

1003
0
0
0
0
0

The naive and 'allocaty' timer  behaves as expected:
```C#
while (true)
{
    await Task.Delay(1000);
    ..
}

[davidfowl]

Yes that seems les than ideal. Something we’d want to fix if we added a BCL API

[GSPP]

it fires the next event immediately if processing of the previous event took longer than period

That seems desirable. You normally want a fixed frequency (such as one action per second). If a new period begins only when the tick code has completed there will be timing shift. A period will take longer than expected. The amount it takes longer might vary and is unpredictable without measurements.

As an example, we want 1.0s, 1.0s, 1.0s... We don't want 1.1s, 1.5s, 1.2s, ...

[omariom]

If processing takes 2.5s-4s, it will be [2.5-4]s, [2.5-4]s, [2.5-4]s :)

But anyway, the behavior could be configurable. May be Task FromNow(TimeSpan) for the timer created with Infinity period.

[stephentoub]

Closing as dup of the proposed solution in https://github.com/dotnet/runtime/issues/31525.