FakeTimeProvider invokes timer callbacks multiple times in single adv…

…ance

fixes dotnet#3995

Reorder variable's in comparaison

Co-authored-by: Igor Velikorossov <RussKie@users.noreply.github.com>

remove unneeded comments

Squashed commit of the following:

commit 2f51f4063e2c0ec9042d425b50380f8c81ab6838
Author: Egil Hansen <egil@assimilated.dk>
Date:   Tue May 30 16:37:13 2023 +0000

    All changes

src/Libraries/Microsoft.Extensions.TimeProvider.Testing/FakeTimeProvider.cs

@@ -68,31 +68,21 @@ 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)
+        while (_now <= value && 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.
@@ -171,28 +161,39 @@ internal void RemoveWaiter(FakeTimeProviderTimer.Waiter waiter)
         }
     }
 
-    private List<FakeTimeProviderTimer.Waiter> GetWaitersToWake()
+    private FakeTimeProviderTimer.Waiter? TryGetWaiterToWake(DateTimeOffset targetNow)
     {
-        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;
     }
 }

src/Libraries/Microsoft.Extensions.TimeProvider.Testing/FakeTimeProviderTimer.cs

@@ -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)
         {
@@ -91,14 +100,13 @@ 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)
                 {
@@ -114,10 +122,7 @@ public void TriggerAndSchedule(bool restart)
                 }
                 else
                 {
-                    // Schedule next event on dueTime
-
-                    WakeupTime = _fakeTimeProvider.GetUtcNow() + TimeSpan.FromMilliseconds(_dueTimeMs);
-
+                    ScheduleTimer(_dueTimeMs);
                     return;
                 }
             }
@@ -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);
+        }
     }
 }

test/Libraries/Microsoft.Extensions.TimeProvider.Testing.Tests/FakeTimeProviderTests.cs

@@ -76,7 +76,7 @@ public void RichCtor()
         Assert.Equal(6, now.Second);
         Assert.Equal(16, now.Millisecond);
         Assert.Equal(frequency, frequency2);
-        Assert.True(pnow2 > pnow);        
+        Assert.True(pnow2 > pnow);
     }
 
     [Fact]

test/Libraries/Microsoft.Extensions.TimeProvider.Testing.Tests/FakeTimeProviderTimerTests.cs

@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System;
+using System.Collections.Generic;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.Extensions.Time.Testing;
@@ -131,23 +132,6 @@ public void TimerTriggersPeriodically()
         Assert.Equal(3, value4);
     }
 
-    [Fact]
-    public void LongPausesTriggerSingleCallback()
-    {
-        var counter = 0;
-        var timeProvider = new FakeTimeProvider();
-        var timer = timeProvider.CreateTimer(_ => { counter++; }, null, TimeSpan.Zero, TimeSpan.FromMilliseconds(10));
-
-        var value1 = counter;
-
-        timeProvider.Advance(TimeSpan.FromMilliseconds(100));
-
-        var value2 = counter;
-
-        Assert.Equal(1, value1);
-        Assert.Equal(2, value2);
-    }
-
     [Fact]
     public async Task TaskDelayWithFakeTimeProviderAdvanced()
     {
@@ -273,4 +257,77 @@ public void WaiterRemovedWhenCollectedWithoutDispose()
         Assert.Equal(1, waitersCountAfter);
     }
 #endif
+
+    [Fact]
+    public void UtcNowUpdatedBeforeTimerCallback()
+    {
+        var timeProvider = new FakeTimeProvider();
+        var callbackTime = DateTimeOffset.MinValue;
+        using var timer = timeProvider.CreateTimer(_ => { callbackTime = timeProvider.GetUtcNow(); }, null, TimeSpan.Zero, TimeSpan.FromMilliseconds(10));
+
+        var value1 = callbackTime;
+
+        timeProvider.SetUtcNow(timeProvider.Epoch + TimeSpan.FromMilliseconds(20));
+
+        var value2 = callbackTime;
+
+        timeProvider.SetUtcNow(timeProvider.Epoch + TimeSpan.FromMilliseconds(1000));
+
+        var value3 = callbackTime;
+
+        Assert.Equal(timeProvider.Epoch, value1);
+        Assert.Equal(timeProvider.Epoch + TimeSpan.FromMilliseconds(20), value2);
+        Assert.Equal(timeProvider.Epoch + TimeSpan.FromMilliseconds(1000), value3);
+    }
+
+    [Fact]
+    public void LongPausesTriggerMultipleCallbacks()
+    {
+        var callbackTimes = new List<DateTimeOffset>();
+        var timeProvider = new FakeTimeProvider();
+        var period = TimeSpan.FromMilliseconds(10);
+        var timer = timeProvider.CreateTimer(_ => { callbackTimes.Add(timeProvider.GetUtcNow()); }, null, TimeSpan.Zero, period);
+
+        var value1 = callbackTimes.ToArray();
+
+        timeProvider.Advance(period + period + period);
+
+        var value2 = callbackTimes.ToArray();
+
+        Assert.Equal(new[] { timeProvider.Epoch }, value1);
+        Assert.Equal(new[]
+        {
+            timeProvider.Epoch,
+            timeProvider.Epoch + period,
+            timeProvider.Epoch + period + period,
+            timeProvider.Epoch + period + period + period,
+        },
+        value2);
+    }
+
+    [Fact]
+    public void MultipleTimersCallbackInvokedInScheduledOrder()
+    {
+        var callbacks = new List<(int timerId, TimeSpan callbackTime)>();
+        var timeProvider = new FakeTimeProvider();
+        var startTime = timeProvider.GetTimestamp();
+        using var timer1 = timeProvider.CreateTimer(_ => callbacks.Add((1, timeProvider.GetElapsedTime(startTime))), null, TimeSpan.FromMilliseconds(3), TimeSpan.FromMilliseconds(3));
+        using var timer2 = timeProvider.CreateTimer(_ => callbacks.Add((2, timeProvider.GetElapsedTime(startTime))), null, TimeSpan.FromMilliseconds(3), TimeSpan.FromMilliseconds(3));
+        using var timer3 = timeProvider.CreateTimer(_ => callbacks.Add((3, timeProvider.GetElapsedTime(startTime))), null, TimeSpan.FromMilliseconds(6), TimeSpan.FromMilliseconds(5));
+
+        timeProvider.Advance(TimeSpan.FromMilliseconds(11));
+
+        Assert.Equal(new[]
+        {
+            (1, TimeSpan.FromMilliseconds(3)),
+            (2, TimeSpan.FromMilliseconds(3)),
+            (3, TimeSpan.FromMilliseconds(6)),
+            (1, TimeSpan.FromMilliseconds(6)),
+            (2, TimeSpan.FromMilliseconds(6)),
+            (1, TimeSpan.FromMilliseconds(9)),
+            (2, TimeSpan.FromMilliseconds(9)),
+            (3, TimeSpan.FromMilliseconds(11)),
+        },
+        callbacks);
+    }
 }