Ambiguity in the FakeTimeProvider timer invocations

[Yorie]

Description

Hi, recently I've found that FakeTimeProvider behavior may differ depending on when and where FakeTimeProvider.Advance() method is called.

I have a question about that case.

Reproduction Steps

Two simple xUnit tests that are expected to be equal semantically: I expect the timer to be executed 10 times in both tests:

Test1:

[Fact]
public void Ambiguous1()
{
	var tp = new FakeTimeProvider();
	int counter = 0;

	const int samples = 10;
	tp.CreateTimer(_ =>
	{
		Interlocked.Increment(ref counter);
	}, null, TimeSpan.FromSeconds(1), TimeSpan.FromSeconds(1));
	
	tp.Advance(TimeSpan.FromSeconds(samples));
	
	// Immediate invocation + sampled.
	Assert.Equal(samples, counter);
}

// OK

Test2:

[Fact]
public void Ambiguous2()
{
	var tp = new FakeTimeProvider();
	int counter = 0;

	const int samples = 10;
	tp.CreateTimer(_ =>
	{
		int c = Interlocked.Increment(ref counter);

		if (c >= samples)
			return;
		
		tp.Advance(TimeSpan.FromSeconds(1));
	}, null, TimeSpan.FromSeconds(1), TimeSpan.FromSeconds(1));

	tp.Advance(TimeSpan.FromSeconds(1));
	
	// Immediate invocation + sampled.
	Assert.Equal(samples, counter);
}

// Assert.Equal() Failure: Values differ
// Expected: 10
// Actual:   1

The main difference here is that, in the first test I instantly advance time by 10 seconds. While in the second test, time is advanced 10 times iteratively by one second from the inside of the timer callback.
As a result, first test timer is executed 10 times, while second test timer is executed only once.

While I can understand customized behavior of the FakeTimeProvider, this example looks still weird - we passed same amount of time, but got different number of invocations.

I've dived in FakeTimeProvider sources a bit, and have found the next condition in WakeWaiters() method:

if (oldTicks != newTicks)
{
	// time changed while in the callback, readjust the wake time accordingly
	candidate.WakeupTime = newTicks + candidate.Period;
}
else
{
	// move on to the next period
	candidate.WakeupTime += candidate.Period;
}

If current time was changed during the current execution of the particular timer, then next timer wakeup time is immediately set to new current time + period; otherwise, timer's period iteratively added to the wakeup time.
This behavior exactly given the ambiguity I've described above. If I eliminate this condition (just keep candidate.WakeupTime += candidate.Period; in both cases), then both tests pass correctly.

I understand that eliminating this condition may change things dramatically. But the ambiguity looks weird also. So, I wanted to discuss that behavior or may be somebody could explain such conditional design decision and how to properly handle such cases.

Possibly related to #5722 also.

My very first issue here, so, I'm sorry for any inconvenience :)

Expected behavior

Expected the number of timer callback execution times is equal in both tests.

Actual behavior

The number of timer callback execution times differs.

Regression?

No response

Known Workarounds

No response

Configuration

Microsoft.Extensions.TimeProvider.Testing v9.4.0

Other information

No response

[MihuBot]

I'm a bot. Here is a possible related and/or duplicate issue (I may be wrong):

• FakeTimeProvider.Advance/SetUtcNow does not behave as expected #3995

[olstakh]

I'd also like to point out that this behavior prevents from testing PeriodicTimer, which takes TimeProvider as a constructor argument. So when passing FakeTimeProvider in tests - advancing the time during a callback does not actually do anything and a second tick of PeriodicTimer never happens. See dotnet/runtime#125077

[lilinus]

This behaviour was introduced by #4277, so it seems to be intentional. The documentation for FakeTimeProvider could be clearer on this.