fix: Fixes timer Delay/Next not handling MinValue inputs (#1985)

### Summary

Fixes a few minor issues with timers:
- Timer.Delay and Timer.Interval was not reflecting the actual tick time (aligned to the next 8ms)
- Negative delay values were causing a crash when DateTime.Now - delay was below DateTime.MinValue
- Timer.Next now reflects the correct wall clock tick time based on the adjusted Delay.
- Timer.Next is not assigned when the timer is started. This was important for timers that were created, but started later.

Author: kamronbatman

Projects/Server.Tests/Tests/Timer/TimerTests.cs

@@ -1,108 +1,108 @@
 using System;
 using Xunit;
 
-namespace Server.Tests
+namespace Server.Tests;
+
+[Collection("Sequential Tests")]
+public class TimerTests : IClassFixture<ServerFixture>
 {
-    [Collection("Sequential Tests")]
-    public class TimerTests : IClassFixture<ServerFixture>
+    [Theory]
+    [InlineData(0L, 8L)]
+    [InlineData(80L, 80L)]
+    [InlineData(65L, 72L)]
+    [InlineData(32767L, 32768L)]
+    [InlineData(32768L, 32768L)]
+    [InlineData(32833L, 32840L)]
+    [InlineData(134217729L, 134217736L)]
+    public void TestVariousTimes(long ticks, long expectedTicks)
     {
-        [Theory]
-        [InlineData(0L, 8L)]
-        [InlineData(80L, 80L)]
-        [InlineData(65L, 72L)]
-        [InlineData(32767L, 32768L)]
-        [InlineData(32768L, 32768L)]
-        [InlineData(32833L, 32840L)]
-        [InlineData(134217729L, 134217736L)]
-        public void TestVariousTimes(long ticks, long expectedTicks)
-        {
-            var timerTicks = new TimerTicks();
-            void action()
-            {
-                Assert.Equal(expectedTicks, timerTicks.Ticks);
-                timerTicks.ExecutedCount++;
-            }
+        var timerTicks = new TimerTicks();
+        Timer.Init(timerTicks.Ticks);
 
-            Timer.Init(timerTicks.Ticks);
+        Timer.StartTimer(TimeSpan.FromMilliseconds(ticks), action);
 
-            Timer.StartTimer(TimeSpan.FromMilliseconds(ticks), action);
+        var tickCount = expectedTicks / 8;
 
-            var tickCount = expectedTicks / 8;
+        for (int i = 1; i <= tickCount; i++)
+        {
+            timerTicks.Ticks = i * 8;
 
-            for (int i = 1; i <= tickCount; i++)
-            {
-                timerTicks.Ticks = i * 8;
+            Timer.Slice(timerTicks.Ticks);
+        }
 
-                Timer.Slice(timerTicks.Ticks);
-            }
+        Assert.Equal(1, timerTicks.ExecutedCount);
+        return;
 
-            Assert.Equal(1, timerTicks.ExecutedCount);
+        void action()
+        {
+            Assert.Equal(expectedTicks, timerTicks.Ticks);
+            timerTicks.ExecutedCount++;
         }
+    }
 
-        [Theory]
-        [InlineData(1000L, 1000L, 30000L, 30000L, 2)]
-        public void TestIntervals(long delay, long expectedDelayTicks, long interval, long expectedIntervalTicks, int count)
-        {
-            var timerTicks = new TimerTicks();
+    [Theory]
+    [InlineData(1000L, 1000L, 30000L, 30000L, 2)]
+    public void TestIntervals(long delay, long expectedDelayTicks, long interval, long expectedIntervalTicks, int count)
+    {
+        var timerTicks = new TimerTicks();
 
-            Timer.Init(timerTicks.Ticks);
+        Timer.Init(timerTicks.Ticks);
 
-            Timer.StartTimer(TimeSpan.FromMilliseconds(delay), TimeSpan.FromMilliseconds(interval), count, action);
+        Timer.StartTimer(TimeSpan.FromMilliseconds(delay), TimeSpan.FromMilliseconds(interval), count, action);
 
-            var tickCount = (expectedDelayTicks + (expectedIntervalTicks * count - 1)) / 8;
+        var tickCount = (expectedDelayTicks + (expectedIntervalTicks * count - 1)) / 8;
 
-            for (int i = 1; i <= tickCount; i++)
-            {
-                timerTicks.Ticks = i * 8;
+        for (int i = 1; i <= tickCount; i++)
+        {
+            timerTicks.Ticks = i * 8;
 
-                Timer.Slice(timerTicks.Ticks);
-            }
+            Timer.Slice(timerTicks.Ticks);
+        }
 
-            Assert.Equal(count, timerTicks.ExecutedCount);
-            return;
+        Assert.Equal(count, timerTicks.ExecutedCount);
+        return;
 
-            void action()
-            {
-                timerTicks.ExpectedTicks += timerTicks.ExecutedCount++ == 0 ? expectedDelayTicks : expectedIntervalTicks;
-                Assert.Equal(timerTicks.ExpectedTicks, timerTicks.Ticks);
-            }
+        void action()
+        {
+            timerTicks.ExpectedTicks += timerTicks.ExecutedCount++ == 0 ? expectedDelayTicks : expectedIntervalTicks;
+            Assert.Equal(timerTicks.ExpectedTicks, timerTicks.Ticks);
         }
+    }
 
-        [Fact]
-        public void TestTimerStartedOnTick()
-        {
-            var timerTicks = new TimerTicks();
+    [Fact]
+    public void TestTimerStartedOnTick()
+    {
+        var timerTicks = new TimerTicks();
 
-            Timer.Init(timerTicks.Ticks);
+        Timer.Init(timerTicks.Ticks);
 
-            var timer = new SelfRunningTimer(timerTicks);
-            timer.Start();
+        var timer = new SelfRunningTimer(timerTicks);
+        timer.Start();
 
-            Timer.Slice(128);
-            Assert.Equal(1, timerTicks.ExecutedCount);
-            Timer.Slice(256);
-            Assert.Equal(2, timerTicks.ExecutedCount);
-        }
+        Timer.Slice(128);
+        Assert.Equal(1, timerTicks.ExecutedCount);
+        Timer.Slice(256);
+        Assert.Equal(2, timerTicks.ExecutedCount);
+    }
 
-        private class TimerTicks
-        {
-            public long ExpectedTicks;
-            public long Ticks;
-            public int ExecutedCount;
-        }
+    private class TimerTicks
+    {
+        public long ExpectedTicks;
+        public long Ticks;
+        public int ExecutedCount;
+    }
 
-        private class SelfRunningTimer : Timer
-        {
-            private readonly TimerTicks _timerTicks;
-            public SelfRunningTimer(TimerTicks ticks) : base(TimeSpan.FromMilliseconds(100)) => _timerTicks = ticks;
+    private class SelfRunningTimer : Timer
+    {
+        private readonly TimerTicks _timerTicks;
+        public SelfRunningTimer(TimerTicks ticks) : base(TimeSpan.FromMilliseconds(100)) => _timerTicks = ticks;
 
-            protected override void OnTick()
+        protected override void OnTick()
+        {
+            if (_timerTicks.ExecutedCount++ == 0)
             {
-                if (_timerTicks.ExecutedCount++ == 0)
-                {
-                    Delay = TimeSpan.FromMilliseconds(100);
-                    Start();
-                }
+                Delay = TimeSpan.FromMilliseconds(100);
+                Start();
             }
         }
     }

Projects/Server/Timer/Timer.TimerWheel.cs

@@ -1,6 +1,6 @@
 /*************************************************************************
  * ModernUO                                                              *
- * Copyright 2019-2023 - ModernUO Development Team                       *
+ * Copyright 2019-2024 - ModernUO Development Team                       *
  * Email: [email protected]                                                *
  * File: Timer.TimerWheel.cs                                             *
  *                                                                       *
@@ -18,6 +18,7 @@
 using System.Diagnostics;
 using System.IO;
 using System.Linq;
+using System.Runtime.CompilerServices;
 
 namespace Server;
 
@@ -33,9 +34,9 @@ public partial class Timer
     private const int _tickRate = 1 << _tickRatePowerOf2; // 8ms
     private const long _maxDuration = (long)_tickRate << (_ringSizePowerOf2 * _ringLayers - 1);
 
-    private static Timer[][] _rings = new Timer[_ringLayers][];
-    private static int[] _ringIndexes = new int[_ringLayers];
-    private static Timer[] _executingRings = new Timer[_ringLayers];
+    private static readonly Timer[][] _rings = new Timer[_ringLayers][];
+    private static readonly int[] _ringIndexes = new int[_ringLayers];
+    private static readonly Timer[] _executingRings = new Timer[_ringLayers];
 
     private static long _lastTickTurned = -1;
 
@@ -155,9 +156,7 @@ private static void Execute(Timer timer)
 
         if (!finished)
         {
-            timer.Delay = timer.Interval;
-            timer.Next = DateTime.UtcNow + timer.Interval;
-            AddTimer(timer, (long)timer.Delay.TotalMilliseconds);
+            AddTimer(timer, (long)timer.Interval.TotalMilliseconds);
         }
         else
         {
@@ -168,10 +167,21 @@ private static void Execute(Timer timer)
         timer.Index++;
     }
 
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
+    private static long RoundTicksToNextPowerOfTwo(long value)
+    {
+        if (value <= 0)
+        {
+            return _tickRate;
+        }
+
+        const long mask = _tickRate - 1;
+        return (value + mask) & ~mask;
+    }
+
     private static void AddTimer(Timer timer, long delay)
     {
-        var originalDelay = delay;
-        delay = Math.Max(0, delay);
+        var actualDelay = delay;
 
         var resolutionPowerOf2 = _tickRatePowerOf2;
         for (var i = 0; i < _ringLayers; i++)
@@ -205,7 +215,7 @@ private static void AddTimer(Timer timer, long delay)
                         logger.Error(
                             $"Timer {{Timer}} has a duration of {{Duration}}ms, more than max capacity of {{MaxDuration}}ms.{Environment.NewLine}{{StackTrace}}",
                             timer.GetType(),
-                            originalDelay,
+                            actualDelay,
                             _maxDuration,
                             new StackTrace()
                         );
@@ -214,18 +224,19 @@ private static void AddTimer(Timer timer, long delay)
                     }
                 }
 
+                timer.Next = Core.Now + timer.Delay;
                 timer.Attach(_rings[i][slot]);
                 timer._remaining = remaining;
                 timer._ring = i;
                 timer._slot = (int)slot;
 
                 _rings[i][slot] = timer;
-
                 return;
             }
 
             // The remaining amount until we turn this ring
-            delay -= resolution * (_ringSize - _ringIndexes[i]);
+            var offsetDelay = resolution * (_ringSize - _ringIndexes[i]);
+            delay -= offsetDelay;
             resolutionPowerOf2 = nextResolutionPowerOf2;
         }
     }

Projects/Server/Timer/Timer.cs

@@ -1,6 +1,6 @@
 /*************************************************************************
  * ModernUO                                                              *
- * Copyright 2019-2023 - ModernUO Development Team                       *
+ * Copyright 2019-2024 - ModernUO Development Team                       *
  * Email: [email protected]                                                *
  * File: Timer.cs                                                        *
  *                                                                       *
@@ -34,6 +34,8 @@ public static void Configure()
     private long _remaining;
     private Timer _nextTimer;
     private Timer _prevTimer;
+    private TimeSpan _delay;
+    private TimeSpan _interval;
 
     public Timer(TimeSpan delay) => Init(delay, TimeSpan.Zero, 1);
 
@@ -43,23 +45,34 @@ public static void Configure()
 
     protected void Init(TimeSpan delay, TimeSpan interval, int count)
     {
-        Running = false;
         Delay = delay;
-        Index = 0;
+        Next = DateTime.MinValue;
         Interval = interval;
         Count = count;
+        Running = false;
+        Index = 0;
         _nextTimer = null;
         _prevTimer = null;
-        Next = Core.Now + Delay;
         _ring = -1;
         _slot = -1;
     }
 
     protected int Version { get; set; } // Used to determine if a timer was altered and we should abandon it.
 
     public DateTime Next { get; private set; }
-    public TimeSpan Delay { get; set; }
-    public TimeSpan Interval { get; set; }
+
+    public TimeSpan Delay
+    {
+        get => _delay;
+        set => _delay = TimeSpan.FromMilliseconds(RoundTicksToNextPowerOfTwo((long)value.TotalMilliseconds));
+    }
+
+    public TimeSpan Interval
+    {
+        get => _interval;
+        set => _interval = TimeSpan.FromMilliseconds(RoundTicksToNextPowerOfTwo((long)value.TotalMilliseconds));
+    }
+
     public int Index { get; private set; }
     public int Count { get; private set; }
     public int RemainingCount => Count == 0 ? int.MaxValue : Count - Index;