Keep the timer queue ordered when a timer is changed or disposed

CHANGELOG.md

@@ -7,6 +7,10 @@ and this project aims to follow [Semantic Versioning](https://semver.org/spec/v2
 
 ## [Unreleased]
 
+### Fixed
+- `SimulatedTimeProvider`: rescheduling a queued timer with `ITimer.Change` no longer corrupts the timer queue. Moving the earliest timer later, or stopping it with `Infinite`, hid every other due timer from `Advance` until that timer was reached, so `CancellationTokenSource.CancelAfter`, `TryReset` and `PeriodicTimer.Period` on a provider-made timer could stall unrelated delays and timeouts. Queue entries now carry an immutable `(due, id)` priority and a schedule version, and stale entries are discarded.
+- `SimulatedTimeProvider`: a timer disposed or rescheduled by an earlier callback of the same `Advance` no longer fires for that occurrence. A one-shot timer is spent when its callback runs rather than when `Advance` finds it due.
+
 ## [1.4.0] - 2026-05-16
 
 ### Fixed

property-tests/SimulatedTimeProviderProperties.fs

@@ -192,3 +192,62 @@ let ``GetElapsedTime matches advanced time`` (advanceMs: uint16) =
 
     // Allow small rounding differences
     abs (elapsed.TotalMilliseconds - expectedElapsed.TotalMilliseconds) < 0.01
+
+/// Reference model for the timer queue: each advance fires every live timer that is due, once, in (due, id) order;
+/// a periodic timer is rescheduled one period after the advanced-to time; a disposed timer, or a one-shot timer
+/// that has fired, refuses Change. A due of None never comes.
+type private ModelTimer = { Id: int; mutable Due: int64 option; mutable Period: int64; mutable Done: bool }
+
+/// Property: under any sequence of create, change, dispose and advance, timers fire as the reference model says
+[<Property(MaxTest = 500)>]
+let ``Timers fire as a reference model says under change and dispose`` (ops: (byte * byte * byte) list) =
+    let timeProvider = new SimulatedTimeProvider()
+    let timers = ResizeArray<System.Threading.ITimer>()
+    let model = ResizeArray<ModelTimer>()
+    let fired = ResizeArray<int>()
+    let mutable now = 0L
+    let mutable agrees = true
+
+    let dueOf (b: byte) = if b % 8uy = 7uy then None else Some (int64 (b % 50uy))
+    let periodOf (b: byte) = if b % 3uy = 0uy then int64 (b % 20uy) + 1L else 0L
+    let dueSpan due =
+        match due with
+        | Some ms -> TimeSpan.FromMilliseconds(float ms)
+        | None -> System.Threading.Timeout.InfiniteTimeSpan
+    let periodSpan period =
+        if period = 0L then System.Threading.Timeout.InfiniteTimeSpan else TimeSpan.FromMilliseconds(float period)
+
+    for (kind, a, b) in List.truncate 60 ops do
+        match kind % 4uy with
+        | 0uy ->
+            let id = model.Count
+            let due, period = dueOf a, periodOf b
+            model.Add({ Id = id; Due = due |> Option.map ((+) now); Period = period; Done = false })
+            timers.Add(timeProvider.CreateTimer((fun state -> fired.Add(unbox<int> state)), box id, dueSpan due, periodSpan period))
+        | 1uy when model.Count > 0 ->
+            let timer = model[int a % model.Count]
+            let due, period = dueOf b, periodOf (a ^^^ b)
+            let expected = not timer.Done
+            if expected then
+                timer.Due <- due |> Option.map ((+) now)
+                timer.Period <- period
+            agrees <- agrees && timers[timer.Id].Change(dueSpan due, periodSpan period) = expected
+        | 2uy when model.Count > 0 ->
+            let timer = model[int a % model.Count]
+            timer.Done <- true
+            timers[timer.Id].Dispose()
+        | _ ->
+            let by = int64 (a % 30uy)
+            now <- now + by
+            let due =
+                model
+                |> Seq.filter (fun timer -> not timer.Done && timer.Due |> Option.exists (fun d -> d <= now))
+                |> Seq.sortBy (fun timer -> timer.Due.Value, timer.Id)
+                |> Seq.toList
+            for timer in due do
+                if timer.Period > 0L then timer.Due <- Some (now + timer.Period) else timer.Done <- true
+            fired.Clear()
+            timeProvider.Advance(TimeSpan.FromMilliseconds(float by))
+            agrees <- agrees && List.ofSeq fired = (due |> List.map (fun timer -> timer.Id))
+
+    agrees

src/SimulatedTimeProvider.cs

@@ -20,7 +20,11 @@ public sealed class SimulatedTimeProvider : TimeProvider
     private long _schedulerTicks;
 
     private long _nextId;
-    private readonly PriorityQueue<ScheduledTimer, ScheduledTimer> _queue;
+
+    // Each entry carries the timer's schedule version and an immutable (due, id) priority. Change and Dispose
+    // bump the version and leave the old entry in place; Advance discards entries whose version is stale. An
+    // entry's priority never changes while it is queued, so the heap order always holds.
+    private readonly PriorityQueue<(ScheduledTimer Timer, long Version), (long DueAtTicks, long Id)> _queue;
 
     /// <summary>
     /// Gets lightweight counters that can be used to observe scheduling behavior during simulation.
@@ -38,7 +42,7 @@ public SimulatedTimeProvider(DateTimeOffset? startTime = null, TimeZoneInfo? loc
         _localTimeZone = localTimeZone ?? TimeZoneInfo.Utc;
 
         _schedulerTicks = 0;
-        _queue = new PriorityQueue<ScheduledTimer, ScheduledTimer>(new ScheduledTimerComparer());
+        _queue = new PriorityQueue<(ScheduledTimer Timer, long Version), (long DueAtTicks, long Id)>();
     }
 
     /// <summary>
@@ -135,47 +139,38 @@ public void Advance(TimeSpan by)
 
         Statistics.RecordAdvance(by);
 
-        List<(TimerCallback Callback, object? State)>? due = null;
+        List<(ScheduledTimer Timer, long Version)>? due = null;
 
         lock (_gate)
         {
             _utcNow = _utcNow.Add(by);
             _schedulerTicks += by.Ticks;
 
-            while (_queue.TryPeek(out var timer, out _))
+            while (_queue.TryPeek(out var entry, out var priority))
             {
-                if (timer.IsDisposed)
+                if (entry.Timer.IsDisposed || entry.Version != entry.Timer.Version)
                 {
+                    // Disposed, or rescheduled since this entry was queued: a newer entry stands for it.
                     _queue.Dequeue();
                     continue;
                 }
 
-                if (timer.DueAtTicks > _schedulerTicks)
+                if (priority.DueAtTicks > _schedulerTicks)
                 {
                     break;
                 }
 
                 _queue.Dequeue();
-
-                if (timer.IsDisposed)
-                {
-                    continue;
-                }
-
                 due ??= [];
-                due.Add((timer.Callback, timer.State));
+                due.Add(entry);
 
                 // Periodic timers: coalesce on jump; schedule next occurrence from "now".
-                if (timer.PeriodTicks > 0)
+                if (entry.Timer.PeriodTicks > 0)
                 {
-                    timer.DueAtTicks = _schedulerTicks + timer.PeriodTicks;
-                    _queue.Enqueue(timer, timer);
+                    entry.Timer.DueAtTicks = _schedulerTicks + entry.Timer.PeriodTicks;
+                    Enqueue(entry.Timer);
                     Statistics.RecordPeriodicReschedule(_queue.Count);
                 }
-                else
-                {
-                    timer.MarkDisposed();
-                }
             }
         }
 
@@ -184,11 +179,25 @@ public void Advance(TimeSpan by)
             return;
         }
 
-        // Fire callbacks outside lock.
-        foreach (var (callback, state) in due)
+        // Fire callbacks outside lock. A callback may dispose or reschedule a timer that this advance has already
+        // found due; such a timer does not fire for this occurrence.
+        foreach (var (timer, version) in due)
         {
+            lock (_gate)
+            {
+                if (timer.IsDisposed || timer.Version != version)
+                {
+                    continue;
+                }
+
+                if (timer.PeriodTicks == 0)
+                {
+                    timer.MarkDisposed();
+                }
+            }
+
             Statistics.RecordCallbackFired();
-            callback(state);
+            timer.Callback(timer.State);
         }
     }
 
@@ -221,36 +230,15 @@ public override ITimer CreateTimer(TimerCallback callback, object? state, TimeSp
             var periodTicks = period == Timeout.InfiniteTimeSpan ? 0 : period.Ticks;
 
             var timer = new ScheduledTimer(this, id, callback, state, dueTicks, periodTicks);
-            _queue.Enqueue(timer, timer);
+            Enqueue(timer);
             Statistics.RecordTimerCreated(_queue.Count);
             return timer;
         }
     }
 
-    private void Reschedule(ScheduledTimer timer)
-    {
-        if (timer.IsDisposed)
-        {
-            return;
-        }
-
-        _queue.Enqueue(timer, timer);
-        Statistics.RecordQueueEnqueue(_queue.Count);
-    }
-
-    private sealed class ScheduledTimerComparer : IComparer<ScheduledTimer>
-    {
-        public int Compare(ScheduledTimer? x, ScheduledTimer? y)
-        {
-            if (ReferenceEquals(x, y)) return 0;
-            if (x is null) return -1;
-            if (y is null) return 1;
-
-            var cmp = x.DueAtTicks.CompareTo(y.DueAtTicks);
-            if (cmp != 0) return cmp;
-            return x.Id.CompareTo(y.Id);
-        }
-    }
+    // Queues the timer's current schedule. Callers hold _gate.
+    private void Enqueue(ScheduledTimer timer) =>
+        _queue.Enqueue((timer, timer.Version), (timer.DueAtTicks, timer.Id));
 
     private sealed class ScheduledTimer : ITimer
     {
@@ -295,6 +283,12 @@ public ScheduledTimer(
         /// </summary>
         public long PeriodTicks { get; set; }
 
+        /// <summary>
+        /// Gets the version of the timer's schedule, which <see cref="Change"/> increments. A queued entry with an
+        /// older version is stale. Read and written under the owner's lock.
+        /// </summary>
+        public long Version { get; private set; }
+
         /// <summary>
         /// Gets a value indicating whether this timer has been disposed.
         /// </summary>
@@ -332,8 +326,11 @@ public bool Change(TimeSpan dueTime, TimeSpan period)
                 DueAtTicks = dueTicks;
                 PeriodTicks = period == Timeout.InfiniteTimeSpan ? 0 : period.Ticks;
 
+                // Never reorder a queued entry: supersede it, and queue the new schedule.
+                Version++;
                 _owner.Statistics.RecordTimerChange();
-                _owner.Reschedule(this);
+                _owner.Enqueue(this);
+                _owner.Statistics.RecordQueueEnqueue(_owner._queue.Count);
                 return true;
             }
         }

tests/SimulatedTimeProviderTests.cs

@@ -323,4 +323,96 @@ public void Timer_callback_can_make_another_timer_due_and_it_fires_on_next_advan
         tp.Advance(TimeSpan.Zero);
         Assert.Equal([1, 2], list);
     }
+
+    [Fact]
+    public void Changing_the_earliest_timer_later_does_not_hide_a_due_timer()
+    {
+        var tp = SimulatedTimeProvider.FromEpoch();
+        var fired = new List<string>();
+
+        using var a = tp.CreateTimer(_ => fired.Add("a"), state: null, dueTime: TimeSpan.FromSeconds(10), period: Timeout.InfiniteTimeSpan);
+        using var b = tp.CreateTimer(_ => fired.Add("b"), state: null, dueTime: TimeSpan.FromSeconds(20), period: Timeout.InfiniteTimeSpan);
+
+        Assert.True(a.Change(TimeSpan.FromSeconds(25), Timeout.InfiniteTimeSpan));
+
+        tp.Advance(TimeSpan.FromSeconds(20));
+        Assert.Equal(["b"], fired);
+
+        tp.Advance(TimeSpan.FromSeconds(5));
+        Assert.Equal(["b", "a"], fired);
+    }
+
+    [Fact]
+    public void Stopping_the_earliest_timer_does_not_block_later_timers()
+    {
+        var tp = SimulatedTimeProvider.FromEpoch();
+        var fired = new List<string>();
+
+        using var a = tp.CreateTimer(_ => fired.Add("a"), state: null, dueTime: TimeSpan.FromSeconds(10), period: Timeout.InfiniteTimeSpan);
+        using var b = tp.CreateTimer(_ => fired.Add("b"), state: null, dueTime: TimeSpan.FromSeconds(20), period: Timeout.InfiniteTimeSpan);
+
+        Assert.True(a.Change(Timeout.InfiniteTimeSpan, Timeout.InfiniteTimeSpan));
+
+        tp.Advance(TimeSpan.FromDays(1));
+        Assert.Equal(["b"], fired);
+    }
+
+    [Fact]
+    public void CancelAfter_on_a_provider_source_does_not_hide_a_due_delay()
+    {
+        var tp = SimulatedTimeProvider.FromEpoch();
+
+        using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(1), tp);
+        var delay = Task.Delay(TimeSpan.FromSeconds(2), tp);
+
+        // CancelAfter reschedules the source's timer through ITimer.Change.
+        cts.CancelAfter(TimeSpan.FromSeconds(5));
+
+        tp.Advance(TimeSpan.FromSeconds(3));
+        Assert.True(delay.IsCompletedSuccessfully);
+        Assert.False(cts.IsCancellationRequested);
+
+        tp.Advance(TimeSpan.FromSeconds(2));
+        Assert.True(cts.IsCancellationRequested);
+    }
+
+    [Fact]
+    public void Timer_disposed_by_an_earlier_callback_in_the_same_advance_does_not_fire()
+    {
+        var tp = SimulatedTimeProvider.FromEpoch();
+        var fired = new List<string>();
+
+        ITimer? second = null;
+        using var first = tp.CreateTimer(_ =>
+        {
+            fired.Add("first");
+            second!.Dispose();
+        }, state: null, dueTime: TimeSpan.FromSeconds(1), period: Timeout.InfiniteTimeSpan);
+        second = tp.CreateTimer(_ => fired.Add("second"), state: null, dueTime: TimeSpan.FromSeconds(2), period: Timeout.InfiniteTimeSpan);
+
+        tp.Advance(TimeSpan.FromSeconds(2));
+
+        Assert.Equal(["first"], fired);
+    }
+
+    [Fact]
+    public void Timer_rescheduled_by_an_earlier_callback_in_the_same_advance_fires_at_its_new_due_time()
+    {
+        var tp = SimulatedTimeProvider.FromEpoch();
+        var fired = new List<string>();
+
+        ITimer? second = null;
+        using var first = tp.CreateTimer(_ =>
+        {
+            fired.Add("first");
+            Assert.True(second!.Change(TimeSpan.FromSeconds(10), Timeout.InfiniteTimeSpan));
+        }, state: null, dueTime: TimeSpan.FromSeconds(1), period: Timeout.InfiniteTimeSpan);
+        using var secondTimer = second = tp.CreateTimer(_ => fired.Add("second"), state: null, dueTime: TimeSpan.FromSeconds(2), period: Timeout.InfiniteTimeSpan);
+
+        tp.Advance(TimeSpan.FromSeconds(2));
+        Assert.Equal(["first"], fired);
+
+        tp.Advance(TimeSpan.FromSeconds(10));
+        Assert.Equal(["first", "second"], fired);
+    }
 }