fix: Fixes timers not stopping before OnTick (#1890)

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

@@ -44,11 +44,6 @@ namespace Server.Tests
         public void TestIntervals(long delay, long expectedDelayTicks, long interval, long expectedIntervalTicks, int count)
         {
             var timerTicks = new TimerTicks();
-            void action()
-            {
-                timerTicks.ExpectedTicks += timerTicks.ExecutedCount++ == 0 ? expectedDelayTicks : expectedIntervalTicks;
-                Assert.Equal(timerTicks.ExpectedTicks, timerTicks.Ticks);
-            }
 
             Timer.Init(timerTicks.Ticks);
 
@@ -64,6 +59,29 @@ namespace Server.Tests
             }
 
             Assert.Equal(count, timerTicks.ExecutedCount);
+            return;
+
+            void action()
+            {
+                timerTicks.ExpectedTicks += timerTicks.ExecutedCount++ == 0 ? expectedDelayTicks : expectedIntervalTicks;
+                Assert.Equal(timerTicks.ExpectedTicks, timerTicks.Ticks);
+            }
+        }
+
+        [Fact]
+        public void TestTimerStartedOnTick()
+        {
+            var timerTicks = new TimerTicks();
+
+            Timer.Init(timerTicks.Ticks);
+
+            var timer = new SelfRunningTimer(timerTicks);
+            timer.Start();
+
+            Timer.Slice(128);
+            Assert.Equal(1, timerTicks.ExecutedCount);
+            Timer.Slice(256);
+            Assert.Equal(2, timerTicks.ExecutedCount);
         }
 
         private class TimerTicks
@@ -72,5 +90,20 @@ namespace Server.Tests
             public long Ticks;
             public int ExecutedCount;
         }
+
+        private class SelfRunningTimer : Timer
+        {
+            private readonly TimerTicks _timerTicks;
+            public SelfRunningTimer(TimerTicks ticks) : base(TimeSpan.FromMilliseconds(100)) => _timerTicks = ticks;
+
+            protected override void OnTick()
+            {
+                if (_timerTicks.ExecutedCount++ == 0)
+                {
+                    Delay = TimeSpan.FromMilliseconds(100);
+                    Start();
+                }
+            }
+        }
     }
 }

Projects/Server/Timer/Timer.TimerWheel.cs

@@ -118,11 +118,6 @@ public partial class Timer
                         Execute(timer);
                     }
                 }
-
-                if (!timer.Running)
-                {
-                    timer.OnDetach();
-                }
             }
 #if DEBUG_TIMERS
             if (executionCount > _chainExecutionThreshold)
@@ -141,27 +136,29 @@ public partial class Timer
     {
         var finished = timer.Count != 0 && ++timer.Index >= timer.Count;
 
-        var version = timer.Version;
-
         var prof = timer.GetProfile();
         prof?.Start();
+
+        // Stop the timer from running so that way if Start() is called in OnTick, the timer will be started.
+        if (finished)
+        {
+            timer.Stop();
+        }
+
+        var version = timer.Version;
+
         timer.OnTick();
         prof?.Finish();
 
-        // If the timer has not been stopped, and it has not been altered (restarted, returned etc)
-        if (timer.Running && timer.Version == version)
+        // If the timer has been altered (restarted, returned etc) then bail
+        if (finished || timer.Version != version)
         {
-            if (finished)
-            {
-                timer.Stop();
-            }
-            else
-            {
-                timer.Delay = timer.Interval;
-                timer.Next = Core.Now + timer.Interval;
-                AddTimer(timer, (long)timer.Delay.TotalMilliseconds);
-            }
+            return;
         }
+
+        timer.Delay = timer.Interval;
+        timer.Next = DateTime.UtcNow + timer.Interval;
+        AddTimer(timer, (long)timer.Delay.TotalMilliseconds);
     }
 
     private static void AddTimer(Timer timer, long delay)

Projects/Server/Timer/Timer.cs

@@ -153,21 +153,7 @@ public partial class Timer
 
         Running = false;
 
-        // We are the head on the timer ring
-        if (_rings[_ring][_slot] == this)
-        {
-            _rings[_ring][_slot] = _nextTimer;
-        }
-
-        // We are the head on the executing ring
-        if (_executingRings[_ring] == this)
-        {
-            _executingRings[_ring] = _nextTimer;
-        }
-
         Detach();
-
-        Version++;
         OnDetach();
 
         var prof = GetProfile();
@@ -175,6 +161,7 @@ public partial class Timer
         {
             prof.Stopped++;
         }
+        Version++;
     }
 
     protected virtual void OnTick()
@@ -213,6 +200,18 @@ public partial class Timer
 
     private void Detach()
     {
+        // We are the head on the timer ring
+        if (_rings[_ring][_slot] == this)
+        {
+            _rings[_ring][_slot] = _nextTimer;
+        }
+
+        // We are the head on the executing ring
+        if (_executingRings[_ring] == this)
+        {
+            _executingRings[_ring] = _nextTimer;
+        }
+
         if (_prevTimer != null)
         {
             _prevTimer._nextTimer = _nextTimer;

Projects/Server/Utilities/Utility.cs

@@ -746,7 +746,7 @@ public static class Utility
     public static T RandomList<T>(params T[] list) => list.RandomElement();
 
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
-    public static T RandomElement<T>(this ReadOnlySpan<T> list) => list.Length == 0 ? (T)default : list[Random(list.Length)];
+    public static T RandomElement<T>(this ReadOnlySpan<T> list) => list.Length == 0 ? default : list[Random(list.Length)];
 
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
     public static T RandomElement<T>(this T[] list) => list.RandomElement(default);