FakeTimeProvider.CreateTimer should return an implicitly rooted timer
- Dominant language
- C#
- Stars
- 3.2k
- Forks
- 894
- Avg merge
- 1d 12h
- Merged PRs (30d)
- 23
Description
`TimeProvider.System.CreateTimer` returns a `SystemTimeProviderTimer`.
Unlike `System.Threading.Timer`, it is implicitly rooted timer, meaning that when the variable to which it was assigned is eligible for GC, it will still work until `Disposed`.
This is deliberately not the case with `FakeTimeProvider`s implementation for `CreateTimer` since it returns a version which is more like `System.Threading.Timer` and less than `SystemTimeProviderTimer`. From the comments for [`Waiter`'s class](https://github.com/dotnet/extensions/blob/20c12ef61fc33865f36c1f4f6e8e2240e8c25f32/src/Libraries/Microsoft.Extensions.TimeProvider.Testing/Waiter.cs#L8):
```
// We keep all timer state here in order to prevent Timer instances from being self-referential,
// which would block them being collected when someone forgets to call Dispose on the timer. With
// this arrangement, the Timer object will always be collectible, which will end up calling Dispose
// on this object due to the timer's finalizer.
```
But what if I forget to dispose the the Timer and depend on this implicitly-rooted behavior of timers that is promised to me for [`TimeProvider.CreateTimer`](https://learn.microsoft.com/en-us/dotnet/api/system.timeprovider.createtimer?view=net-9.0)
>
> The return [ITimer](https://learn.microsoft.com/en-us/dotnet/api/system.threading.itimer?view=net-9.0) instance will be implicitly rooted while the timer is still scheduled.
>
I think the current implementation breaks the contract from the documentation for the abstract class and will cause certain tests to fail where they won't when using the default `TimeScheduler.System`.
Short reproducible code:
```
void Main() {
TimeProvider timeProvider = TimeProvider.System;
var timerStarter = new TimerStarter();
timerStarter.Start(timeProvider, _ => Console.WriteLine("System"),
null, TimeSpan.FromSeconds(1), TimeSpan.FromSeconds(1));
GC.Collect();
GC.WaitForPendingFinalizers();
GC.Collect();
Thread.Sleep(5000);
Console.WriteLine("--------------");
// System
var fakeTimeProvider = new FakeTimeProvider();
timerStarter.Start(fakeTimeProvider, _ => Console.WriteLine("Fake"),
null, TimeSpan.FromSeconds(1), TimeSpan.FromSeconds(1));
// Simulate GC during testing
GC.Collect();
GC.WaitForPendingFinalizers();
GC.Collect();
fakeTimeProvider.Advance(TimeSpan.FromSeconds(5));
Thread.Sleep(5000);
}
class TimerStarter {
public void Start(TimeProvider timeProvider,
TimerCallback callback,
object? state,
TimeSpan dueDate,
TimeSpan period
) {
timeProvider.CreateTimer(callback, state, dueDate, period);
}
}
```
Contributor guide
Assessment
This issue has not been assessed yet.