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

Timeprovider advance multiple invocations #4022

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Globalization;
using System.Threading;
using Microsoft.Shared.Diagnostics;

namespace Microsoft.Extensions.Time.Testing;

Expand All @@ -14,13 +16,22 @@ namespace Microsoft.Extensions.Time.Testing;
/// </summary>
public class FakeTimeProvider : TimeProvider
{
internal static readonly DateTimeOffset Epoch = new(2000, 1, 1, 0, 0, 0, 0, TimeSpan.Zero);

internal readonly List<FakeTimeProviderTimer.Waiter> Waiters = new();

private DateTimeOffset _now = Epoch;
private DateTimeOffset _now;
private TimeZoneInfo _localTimeZone;

/// <summary>
/// Gets the time which was used as the starting point for the clock in this <see cref="FakeTimeProvider"/>.
/// </summary>
/// <remarks>
/// This can be set by passing in a <see cref="DateTimeOffset"/> to the constructor
/// which takes the <c>epoch</c> argument. If the default constructor is used,
/// the clocks start time defaults to midnight January 1st 2000.
/// </remarks>
[Experimental]
public DateTimeOffset Epoch { get; } = new DateTimeOffset(2000, 1, 1, 0, 0, 0, 0, TimeSpan.Zero);
egil marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// Initializes a new instance of the <see cref="FakeTimeProvider"/> class.
/// </summary>
Expand All @@ -31,16 +42,19 @@ public class FakeTimeProvider : TimeProvider
public FakeTimeProvider()
{
_localTimeZone = TimeZoneInfo.Utc;
_now = Epoch;
}

/// <summary>
/// Initializes a new instance of the <see cref="FakeTimeProvider"/> class.
/// </summary>
/// <param name="startTime">The initial time reported by the clock.</param>
public FakeTimeProvider(DateTimeOffset startTime)
/// <param name="epoch">The starting point for the clock used by this <see cref="FakeTimeProvider"/>.</param>
[Experimental]
public FakeTimeProvider(DateTimeOffset epoch)
: this()
{
_now = startTime;
Epoch = epoch;
_now = epoch;
}

/// <inheritdoc />
Expand All @@ -55,31 +69,26 @@ public override DateTimeOffset GetUtcNow()
/// <param name="value">The date and time in the UTC timezone.</param>
public void SetUtcNow(DateTimeOffset value)
{
List<FakeTimeProviderTimer.Waiter> waiters;
lock (Waiters)
if (value < _now)
{
Throw.ArgumentOutOfRangeException(nameof(value), $"Cannot go back in time. Current time is {GetUtcNow()}.");
}

while (value >= _now && TryGetWaiterToWake(value) is FakeTimeProviderTimer.Waiter waiter)
{
_now = value;
waiters = GetWaitersToWake();
_now = waiter.WakeupTime;
waiter.TriggerAndSchedule(false);
}

WakeWaiters(waiters);
_now = value;
}

/// <summary>
/// Advances the clock's time by a specific amount.
/// </summary>
/// <param name="delta">The amount of time to advance the clock by.</param>
public void Advance(TimeSpan delta)
{
List<FakeTimeProviderTimer.Waiter> waiters;
lock (Waiters)
{
_now += delta;
waiters = GetWaitersToWake();
}

WakeWaiters(waiters);
}
=> SetUtcNow(_now + delta);

/// <summary>
/// Advances the clock's time by one millisecond.
Expand Down Expand Up @@ -158,28 +167,39 @@ internal void RemoveWaiter(FakeTimeProviderTimer.Waiter waiter)
}
}

private List<FakeTimeProviderTimer.Waiter> GetWaitersToWake()
private FakeTimeProviderTimer.Waiter? TryGetWaiterToWake(DateTimeOffset targetNow)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could it use the standard Try... pattern by being bool TryGetWaiterToWake(DateTimeOffset targetNow, out FakeTimeProviderTimer.Waiter?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. But please indulge me, why?

I would use the common bool TryGetX(Foo foo, out Foo bar) in a public API surface since that is a very recognizable pattern in the .NET ecosystem. For private/internal stuff, I have changed to this pattern since we now have nullable reference types and pattern matching, I feel this version is more direct, clean, and doesn't involve out arguments. I am fairly certain that had these language features been around in the early days, the TryGetX pattern would not be the norm.

I do respect that you may have a coding standard in this repo you like to follow (I admit I didn't look at other projects in the repo, just this one) and I would change it regardless if you prefer the other style.

{
var l = new List<FakeTimeProviderTimer.Waiter>(Waiters.Count);
foreach (var w in Waiters)
var candidate = default(FakeTimeProviderTimer.Waiter);

lock (Waiters)
{
if (_now >= w.WakeupTime)
if (Waiters.Count == 0)
{
l.Add(w);
return null;
}
}

return l;
}

private void WakeWaiters(List<FakeTimeProviderTimer.Waiter> waiters)
{
foreach (var w in waiters)
{
if (_now >= w.WakeupTime)
foreach (var waiter in Waiters)
{
w.TriggerAndSchedule(false);
if (waiter.WakeupTime > targetNow)
{
continue;
}

if (candidate is null)
{
candidate = waiter;
continue;
}

// This finds the waiter with the minimum WakeupTime and also ensures that if multiple waiters have the same
// the one that is picked is also the one that was scheduled first.
candidate = candidate.WakeupTime > waiter.WakeupTime
|| (candidate.WakeupTime == waiter.WakeupTime && candidate.ScheduledOn > waiter.ScheduledOn)
? waiter
: candidate;
}
}

return candidate;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,16 @@ internal sealed class Waiter : IDisposable

private long _periodMs;
private long _dueTimeMs;
public DateTimeOffset WakeupTime { get; set; }

/// <summary>
/// Gets the timestamp for when the <see cref="Waiter.WakeupTime"/> was last set.
/// </summary>
/// <remarks>
/// This property ensures timer callbacks are invoked in the order they were scheduled.
/// </remarks>
public long ScheduledOn { get; private set; }

public DateTimeOffset WakeupTime { get; private set; }

public Waiter(FakeTimeProvider fakeTimeProvider, TimeSpan dueTime, TimeSpan period, TimerCallback callback, object? state)
{
Expand All @@ -91,33 +100,29 @@ public void ChangeAndValidateDurations(TimeSpan dueTime, TimeSpan period)
_ = Throw.IfOutOfRange(_dueTimeMs, -1, MaxSupportedTimeout, nameof(dueTime));
_ = Throw.IfOutOfRange(_periodMs, -1, MaxSupportedTimeout, nameof(period));
#pragma warning restore S3236 // Caller information arguments should not be provided explicitly

}

public void TriggerAndSchedule(bool restart)
{
if (restart)
{
WakeupTime = DateTimeOffset.MaxValue;
DisableTimer();

if (_dueTimeMs == 0)
{
// If dueTime is zero, callback is invoked immediately
// If dueTime is zero, callback is invoked immediately.

_callback(_state);
}
else if (_dueTimeMs == Timeout.Infinite)
{
// If dueTime is Timeout.Infinite, callback is not invoked; the timer is disabled
// If dueTime is Timeout.Infinite, callback is not invoked; the timer is disabled.

return;
}
else
{
// Schedule next event on dueTime

WakeupTime = _fakeTimeProvider.GetUtcNow() + TimeSpan.FromMilliseconds(_dueTimeMs);

ScheduleTimer(_dueTimeMs);
return;
}
}
Expand All @@ -130,17 +135,29 @@ public void TriggerAndSchedule(bool restart)

if (_periodMs == 0 || _periodMs == Timeout.Infinite)
{
WakeupTime = DateTimeOffset.MaxValue;
DisableTimer();
}
else
{
WakeupTime = _fakeTimeProvider.GetUtcNow() + TimeSpan.FromMilliseconds(_periodMs);
ScheduleTimer(_periodMs);
}
}

public void Dispose()
{
_fakeTimeProvider.RemoveWaiter(this);
}

private void DisableTimer()
{
ScheduledOn = long.MaxValue;
WakeupTime = DateTimeOffset.MaxValue;
}

private void ScheduleTimer(long delayMilliseconds)
{
ScheduledOn = _fakeTimeProvider.GetTimestamp();
WakeupTime = _fakeTimeProvider.GetUtcNow() + TimeSpan.FromMilliseconds(delayMilliseconds);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
<Workstream>Fundamentals</Workstream>
<Category>Testing</Category>
<PackageTags>$(PackageTags);Testing</PackageTags>
<InjectExperimentalAttributeOnLegacy>true</InjectExperimentalAttributeOnLegacy>
</PropertyGroup>

<PropertyGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ public void TryGetLogStream_WhenViewStreamDisposed_ReturnsFalse()
public async Task SelfDiagnosticsConfigRefresher_WhenConfigDisappearsAndAppearsBack_CaptureAsConfigured()
{
const string LogFileName = "withUnreliableConfig.log";
var timeProvider = new FakeTimeProvider(startTime: DateTime.UtcNow);
var timeProvider = new FakeTimeProvider(epoch: DateTime.UtcNow);
var parserMock = new Mock<SelfDiagnosticsConfigParser>();
var configFileContentInitial = @"{""LogDirectory"": ""."", ""FileSize"": 1024, ""LogLevel"": ""Verbose""}";
var configFileContentNew = @"{""LogDirectory"": ""."", ""FileSize"": 1025, ""LogLevel"": ""Verbose""}";
Expand Down Expand Up @@ -174,7 +174,7 @@ public async Task SelfDiagnosticsConfigRefresher_WhenConfigDisappearsAndAppearsB
public async Task SelfDiagnosticsConfigRefresher_WhenLogLevelUpdated_CaptureAsConfigured()
{
const string LogFileName = "withNewLogLevel.log";
var timeProvider = new FakeTimeProvider(startTime: DateTime.UtcNow);
var timeProvider = new FakeTimeProvider(epoch: DateTime.UtcNow);
var parserMock = new Mock<SelfDiagnosticsConfigParser>();
var configFileContentInitial = @"{""LogDirectory"": ""."", ""FileSize"": 1024, ""LogLevel"": ""Error""}";
var configFileContentNew = @"{""LogDirectory"": ""."", ""FileSize"": 1024, ""LogLevel"": ""Verbose""}";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ public void DefaultCtor()
var timestamp = timeProvider.GetTimestamp();
var frequency = timeProvider.TimestampFrequency;

Assert.Equal(timeProvider.Epoch, now);
Assert.Equal(2000, now.Year);
Assert.Equal(1, now.Month);
Assert.Equal(1, now.Day);
Expand All @@ -32,17 +33,12 @@ public void DefaultCtor()

var timestamp2 = timeProvider.GetTimestamp();
var frequency2 = timeProvider.TimestampFrequency;
now = timeProvider.GetUtcNow();
var now2 = timeProvider.GetUtcNow();

Assert.Equal(2000, now.Year);
Assert.Equal(1, now.Month);
Assert.Equal(1, now.Day);
Assert.Equal(0, now.Hour);
Assert.Equal(0, now.Minute);
Assert.Equal(0, now.Second);
Assert.Equal(0, now.Millisecond);
Assert.Equal(10_000_000, frequency2);
Assert.Equal(timestamp2, timestamp);
Assert.Equal(timeProvider.Epoch, now2);
Assert.Equal(now, now2);
Assert.Equal(frequency, frequency2);
Assert.Equal(timestamp, timestamp2);
}

[Fact]
Expand All @@ -55,6 +51,7 @@ public void RichCtor()
var frequency = timeProvider.TimestampFrequency;
var now = timeProvider.GetUtcNow();

Assert.Equal(timeProvider.Epoch + TimeSpan.FromMilliseconds(8), now);
Assert.Equal(2001, now.Year);
Assert.Equal(2, now.Month);
Assert.Equal(3, now.Day);
Expand All @@ -70,14 +67,15 @@ public void RichCtor()
var frequency2 = timeProvider.TimestampFrequency;
now = timeProvider.GetUtcNow();

Assert.Equal(timeProvider.Epoch + TimeSpan.FromMilliseconds(16), now);
Assert.Equal(2001, now.Year);
Assert.Equal(2, now.Month);
Assert.Equal(3, now.Day);
Assert.Equal(4, now.Hour);
Assert.Equal(5, now.Minute);
Assert.Equal(6, now.Second);
Assert.Equal(16, now.Millisecond);
Assert.Equal(10_000_000, frequency2);
Assert.Equal(frequency, frequency2);
Assert.True(pnow2 > pnow);
}

Expand Down Expand Up @@ -138,6 +136,15 @@ public void AdvanceGoesForward()
Assert.Equal(1234, elapsedTime.TotalMilliseconds);
}

[Fact]
public void TimeCannotGoBackwards()
{
var timeProvider = new FakeTimeProvider();

Assert.Throws<ArgumentOutOfRangeException>(() => timeProvider.Advance(TimeSpan.FromTicks(-1)));
Assert.Throws<ArgumentOutOfRangeException>(() => timeProvider.SetUtcNow(timeProvider.Epoch - TimeSpan.FromTicks(1)));
}

[Fact]
public void ToStr()
{
Expand Down Expand Up @@ -173,7 +180,7 @@ public async Task Delay_Timeout()
var timeProvider = new FakeTimeProvider();

var delay = timeProvider.Delay(TimeSpan.FromMilliseconds(1), CancellationToken.None);
timeProvider.Advance();
timeProvider.Advance(TimeSpan.FromMilliseconds(1));
await delay;

Assert.True(delay.IsCompleted);
Expand Down Expand Up @@ -203,7 +210,7 @@ public async Task CreateSource()
var timeProvider = new FakeTimeProvider();

using var cts = timeProvider.CreateCancellationTokenSource(TimeSpan.FromMilliseconds(1));
timeProvider.Advance();
timeProvider.Advance(TimeSpan.FromMilliseconds(1));

await Assert.ThrowsAsync<TaskCanceledException>(() => timeProvider.Delay(TimeSpan.FromTicks(1), cts.Token));
}
Expand All @@ -224,7 +231,7 @@ public async Task WaitAsync()
var t = source.Task.WaitAsync(TimeSpan.FromSeconds(100000), timeProvider, CancellationToken.None);
while (!t.IsCompleted)
{
timeProvider.Advance();
timeProvider.Advance(TimeSpan.FromMilliseconds(1));
await Task.Delay(1);
_ = source.TrySetResult(true);
}
Expand Down
Loading