Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FakeTimeProvider.CreateTimer should return an implicitly rooted timer #5765

Open
tinmanjk opened this issue Jan 2, 2025 · 0 comments
Open

Comments

@tinmanjk
Copy link

tinmanjk commented Jan 2, 2025

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 FakeTimeProviders 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:

// 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

The return ITimer 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);
    }
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants