fix: allow ManualTimer finalizer to run

Author: egil

src/TimeProviderExtensions/ManualTimeProvider.ManualTimer.cs

@@ -0,0 +1,79 @@
+using System.Runtime.CompilerServices;
+
+namespace TimeProviderExtensions;
+
+/// <summary>
+/// Represents a synthetic time provider that can be used to enable deterministic behavior in tests.
+/// </summary>
+/// <remarks>
+/// Learn more at <see href="https://github.com/egil/TimeProviderExtensions"/>.
+/// </remarks>
+public partial class ManualTimeProvider : TimeProvider
+{    
+    private sealed class ManualTimer : ITimer
+    {
+        private ManualTimerScheduler? scheduledCallback;
+
+        public ManualTimer(TimerCallback callback, object? state, ManualTimeProvider timeProvider)
+        {
+            scheduledCallback = new ManualTimerScheduler(timeProvider, callback, state);
+        }
+
+        /// <inheritdoc/>
+        public bool Change(TimeSpan dueTime, TimeSpan period)
+        {
+            ValidateTimeSpanRange(dueTime);
+            ValidateTimeSpanRange(period);
+
+            if (scheduledCallback is null)
+            {
+                return false;
+            }
+
+            scheduledCallback.Change(dueTime, period);
+
+            return true;
+        }
+
+        // In case the timer is not disposed explicitly by the user.
+        ~ManualTimer() => Dispose(false);
+
+        public void Dispose()
+        {
+            Dispose(true);
+            GC.SuppressFinalize(this);
+        }
+
+        public ValueTask DisposeAsync()
+        {
+            Dispose(true);
+            GC.SuppressFinalize(this);
+            return ValueTask.CompletedTask;
+        }
+
+        private void Dispose(bool _)
+        {
+            if (scheduledCallback is null)
+            {
+                return;
+            }
+
+            scheduledCallback.Cancel();
+            scheduledCallback = null;
+        }
+
+        private static void ValidateTimeSpanRange(TimeSpan time, [CallerArgumentExpression("time")] string? parameter = null)
+        {
+            long tm = (long)time.TotalMilliseconds;
+            if (tm < -1)
+            {
+                throw new ArgumentOutOfRangeException(parameter, $"{parameter}.TotalMilliseconds must be greater than -1.");
+            }
+
+            if (tm > MaxSupportedTimeout)
+            {
+                throw new ArgumentOutOfRangeException(parameter, $"{parameter}.TotalMilliseconds must be less than than {MaxSupportedTimeout}.");
+            }
+        }
+    }
+}

src/TimeProviderExtensions/ManualTimeProvider.ManualTimerScheduledCallback.cs

@@ -0,0 +1,83 @@
+namespace TimeProviderExtensions;
+
+/// <summary>
+/// Represents a synthetic time provider that can be used to enable deterministic behavior in tests.
+/// </summary>
+/// <remarks>
+/// Learn more at <see href="https://github.com/egil/TimeProviderExtensions"/>.
+/// </remarks>
+public partial class ManualTimeProvider : TimeProvider
+{
+    // The reason this class exists and it is separate from ManualTimer is that
+    // the GC should be collect the ManualTimer in case users forget to dispose it.
+    // If all the references captured by this type was part of the ManualTimer
+    // type, the finalizer would not be invoked on ManualTimer if a callback was scheduled.
+    private sealed class ManualTimerScheduler : IComparable<ManualTimerScheduler>
+    {
+        private readonly TimerCallback callback;
+        private readonly object? state;
+        private readonly ManualTimeProvider timeProvider;
+        private TimeSpan period;
+        private bool running;
+
+        public DateTimeOffset CallbackTime { get; set; }
+
+        public ManualTimerScheduler(ManualTimeProvider timeProvider, TimerCallback callback, object? state)
+        {
+            this.timeProvider = timeProvider;
+            this.callback = callback;
+            this.state = state;
+        }
+
+        public int CompareTo(ManualTimerScheduler? other)
+            => other is not null
+            ? Comparer<DateTimeOffset>.Default.Compare(CallbackTime, other.CallbackTime)
+            : -1;
+
+        internal void Cancel()
+        {
+            if (running)
+            {
+                timeProvider.RemoveCallback(this);
+            }
+        }
+
+        internal void Change(TimeSpan dueTime, TimeSpan period)
+        {
+            Cancel();
+
+            this.period = period;
+
+            if (dueTime != Timeout.InfiniteTimeSpan)
+            {
+                ScheduleCallback(dueTime);
+            }
+        }
+
+        internal void TimerElapsed()
+        {
+            running = false;
+
+            callback.Invoke(state);
+
+            if (period != Timeout.InfiniteTimeSpan && period != TimeSpan.Zero)
+            {
+                ScheduleCallback(period);
+            }
+        }
+
+        private void ScheduleCallback(TimeSpan waitTime)
+        {
+            running = true;
+
+            if (waitTime == TimeSpan.Zero)
+            {
+                TimerElapsed();
+            }
+            else
+            {
+                timeProvider.ScheduleCallback(this, waitTime);
+            }
+        }
+    }
+}

src/TimeProviderExtensions/ManualTimeProvider.cs

@@ -1,6 +1,5 @@
 using System.Diagnostics;
 using System.Globalization;
-using System.Runtime.CompilerServices;
 
 namespace TimeProviderExtensions;
 
@@ -11,13 +10,13 @@ namespace TimeProviderExtensions;
 /// Learn more at <see href="https://github.com/egil/TimeProviderExtensions"/>.
 /// </remarks>
 [DebuggerDisplay("{ToString()}. Scheduled callback count: {ScheduledCallbackCount}")]
-public class ManualTimeProvider : TimeProvider
+public partial class ManualTimeProvider : TimeProvider
 {
     internal const uint MaxSupportedTimeout = 0xfffffffe;
     internal const uint UnsignedInfinite = unchecked((uint)-1);
     internal static readonly DateTimeOffset DefaultStartDateTime = new(2000, 1, 1, 0, 0, 0, 0, TimeSpan.Zero);
 
-    private readonly List<ManualTimerScheduledCallback> callbacks = new();
+    private readonly List<ManualTimerScheduler> callbacks = new();
     private DateTimeOffset utcNow;
     private TimeZoneInfo localTimeZone;
     private TimeSpan autoAdvanceAmount = TimeSpan.Zero;
@@ -227,7 +226,7 @@ public void SetUtcNow(DateTimeOffset value)
     {
         if (value < utcNow)
         {
-            throw new ArgumentOutOfRangeException(nameof(value), $"The new UtcNow must be greater than or equal to the curren time {utcNow}. Going back in time is not supported.");
+            throw new ArgumentOutOfRangeException(nameof(value), $"The new UtcNow must be greater than or equal to the curren time {ToString()}. Going back in time is not supported.");
         }
 
         lock (callbacks)
@@ -238,16 +237,16 @@ public void SetUtcNow(DateTimeOffset value)
                 return;
             }
 
-            while (utcNow <= value && TryGetNext(value) is ManualTimerScheduledCallback mtsc)
+            while (utcNow <= value && TryGetNext(value) is ManualTimerScheduler mtsc)
             {
                 utcNow = mtsc.CallbackTime;
-                mtsc.Timer.TimerElapsed();
+                mtsc.TimerElapsed();
             }
 
             utcNow = value;
         }
 
-        ManualTimerScheduledCallback? TryGetNext(DateTimeOffset targetUtcNow)
+        ManualTimerScheduler? TryGetNext(DateTimeOffset targetUtcNow)
         {
             if (callbacks.Count > 0 && callbacks[0].CallbackTime <= targetUtcNow)
             {
@@ -266,174 +265,33 @@ public void SetUtcNow(DateTimeOffset value)
     /// <returns>A string representing the clock's current time.</returns>
     public override string ToString() => utcNow.ToString("yyyy-MM-ddTHH:mm:ss.fff", CultureInfo.InvariantCulture);
 
-    private void ScheduleCallback(ManualTimer timer, TimeSpan waitTime)
+    private void ScheduleCallback(ManualTimerScheduler scheduler, TimeSpan waitTime)
     {
         lock (callbacks)
         {
-            var timerCallback = new ManualTimerScheduledCallback(timer, utcNow + waitTime);
-            var insertPosition = callbacks.FindIndex(x => x.CallbackTime > timerCallback.CallbackTime);
+            scheduler.CallbackTime = utcNow + waitTime;
+
+            var insertPosition = callbacks.FindIndex(x => x.CallbackTime > scheduler.CallbackTime);
 
             if (insertPosition == -1)
             {
-                callbacks.Add(timerCallback);
+                callbacks.Add(scheduler);
             }
             else
             {
-                callbacks.Insert(insertPosition, timerCallback);
+                callbacks.Insert(insertPosition, scheduler);
             }
         }
     }
 
-    private void RemoveCallback(ManualTimer timer)
+    private void RemoveCallback(ManualTimerScheduler timerCallback)
     {
         lock (callbacks)
         {
-            var existingIndexOf = callbacks.FindIndex(0, x => ReferenceEquals(x.Timer, timer));
+            var existingIndexOf = callbacks.FindIndex(0, x => ReferenceEquals(x, timerCallback));
             if (existingIndexOf >= 0)
-                callbacks.RemoveAt(existingIndexOf);
-        }
-    }
-
-    private readonly struct ManualTimerScheduledCallback :
-        IEqualityComparer<ManualTimerScheduledCallback>,
-        IComparable<ManualTimerScheduledCallback>
-    {
-        public readonly ManualTimer Timer { get; }
-
-        public readonly DateTimeOffset CallbackTime { get; }
-
-        public ManualTimerScheduledCallback(ManualTimer timer, DateTimeOffset callbackTime)
-        {
-            Timer = timer;
-            CallbackTime = callbackTime;
-        }
-
-        public readonly bool Equals(ManualTimerScheduledCallback x, ManualTimerScheduledCallback y)
-            => ReferenceEquals(x.Timer, y.Timer);
-
-        public readonly int GetHashCode(ManualTimerScheduledCallback obj)
-            => Timer.GetHashCode();
-
-        public readonly int CompareTo(ManualTimerScheduledCallback other)
-            => Comparer<DateTimeOffset>.Default.Compare(CallbackTime, other.CallbackTime);
-    }
-
-    private sealed class ManualTimer : ITimer
-    {
-        private ManualTimeProvider? timeProvider;
-        private bool isDisposed;
-        private bool running;
-
-        private TimeSpan currentDueTime;
-        private TimeSpan currentPeriod;
-        private object? state;
-        private TimerCallback? callback;
-
-        public ManualTimer(TimerCallback callback, object? state, ManualTimeProvider timeProvider)
-        {
-            this.timeProvider = timeProvider;
-            this.callback = callback;
-            this.state = state;
-        }
-
-        public bool Change(TimeSpan dueTime, TimeSpan period)
-        {
-            ValidateTimeSpanRange(dueTime);
-            ValidateTimeSpanRange(period);
-
-            if (isDisposed || timeProvider is null)
-            {
-                return false;
-            }
-
-            if (running)
-            {
-                timeProvider.RemoveCallback(this);
-            }
-
-            currentDueTime = dueTime;
-            currentPeriod = period;
-
-            if (currentDueTime != Timeout.InfiniteTimeSpan)
-            {
-                ScheduleCallback(dueTime);
-            }
-
-            return true;
-        }
-
-        public void Dispose()
-        {
-            if (isDisposed || timeProvider is null)
-            {
-                return;
-            }
-
-            isDisposed = true;
-
-            if (running)
-            {
-                timeProvider.RemoveCallback(this);
-            }
-
-            callback = null;
-            state = null;
-            timeProvider = null;
-        }
-
-        public ValueTask DisposeAsync()
-        {
-            Dispose();
-            return ValueTask.CompletedTask;
-        }
-
-        internal void TimerElapsed()
-        {
-            if (isDisposed || timeProvider is null)
-            {
-                return;
-            }
-
-            running = false;
-
-            callback?.Invoke(state);
-
-            if (currentPeriod != Timeout.InfiniteTimeSpan && currentPeriod != TimeSpan.Zero)
             {
-                ScheduleCallback(currentPeriod);
-            }
-        }
-
-        private void ScheduleCallback(TimeSpan waitTime)
-        {
-            if (isDisposed || timeProvider is null)
-            {
-                return;
-            }
-
-            running = true;
-
-            if (waitTime == TimeSpan.Zero)
-            {
-                TimerElapsed();
-            }
-            else
-            {
-                timeProvider.ScheduleCallback(this, waitTime);
-            }
-        }
-
-        private static void ValidateTimeSpanRange(TimeSpan time, [CallerArgumentExpression("time")] string? parameter = null)
-        {
-            long tm = (long)time.TotalMilliseconds;
-            if (tm < -1)
-            {
-                throw new ArgumentOutOfRangeException(parameter, $"{parameter}.TotalMilliseconds must be greater than -1.");
-            }
-
-            if (tm > MaxSupportedTimeout)
-            {
-                throw new ArgumentOutOfRangeException(parameter, $"{parameter}.TotalMilliseconds must be less than than {MaxSupportedTimeout}.");
+                callbacks.RemoveAt(existingIndexOf);
             }
         }
     }