From 791128f238735a7fc7c9008112c7d87095b0acce Mon Sep 17 00:00:00 2001
From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com>
Date: Tue, 18 Oct 2022 18:14:26 -0700
Subject: [PATCH] fix: Fixes timer orphaning issue (#1196)

* Fixes an edge case where a check to see if a timer was being executed resulted in it not being properly detached on stop. Fixed this by changing how we determine what timer is currently being executed.
* Adds execution count to #DEBUG_TIMERS so shards can get notified (rudimentarily for now) about sequentially executing long chains of timers.

Unfortunately, for now, there is no good option when it comes to executing timers. If we execute let's say 500, and defer the rest, that makes all timers effectively 8ms off until it "catches up". Depending on the shard, that may never happen.
---
 Projects/Server/Timer/Timer.DelayCall.cs  |  2 +-
 Projects/Server/Timer/Timer.TimerWheel.cs | 44 +++++++++++++----------
 Projects/Server/Timer/Timer.cs            | 22 ++++++------
 3 files changed, 39 insertions(+), 29 deletions(-)

diff --git a/Projects/Server/Timer/Timer.DelayCall.cs b/Projects/Server/Timer/Timer.DelayCall.cs
index 0dacb38de..3aa4837ec 100644
--- a/Projects/Server/Timer/Timer.DelayCall.cs
+++ b/Projects/Server/Timer/Timer.DelayCall.cs
@@ -16,8 +16,8 @@
 using System;
 #if DEBUG_TIMERS
 using System.Collections.Generic;
-#endif
 using System.Diagnostics;
+#endif
 using System.Runtime.CompilerServices;
 
 namespace Server;
diff --git a/Projects/Server/Timer/Timer.TimerWheel.cs b/Projects/Server/Timer/Timer.TimerWheel.cs
index d02874036..d3fa2ef7b 100644
--- a/Projects/Server/Timer/Timer.TimerWheel.cs
+++ b/Projects/Server/Timer/Timer.TimerWheel.cs
@@ -22,6 +22,9 @@ namespace Server;
 
 public partial class Timer
 {
+#if DEBUG_TIMERS
+    private const int _chainExecutionThreshold = 512;
+#endif
     private const int _ringSizePowerOf2 = 12;
     private const int _ringSize = 1 << _ringSizePowerOf2; // 4096
     private const int _ringLayers = 3;
@@ -33,7 +36,6 @@ public partial class Timer
     private static Timer[] _executingRings = new Timer[_ringLayers];
 
     private static long _lastTickTurned = -1;
-    private static bool _timerWheelExecuting;
 
     public static void Init(long tickCount)
     {
@@ -59,7 +61,6 @@ public partial class Timer
 
     private static void Turn()
     {
-        _timerWheelExecuting = true;
         var turnNextWheel = false;
 
         // Detach the chain from the timer wheel. This allows adding timers to the same slot during execution.
@@ -86,15 +87,19 @@ public partial class Timer
 
         for (var i = 0; i < _ringLayers; i++)
         {
-            var timer = _executingRings[i];
-            if (timer == null)
+#if DEBUG_TIMERS
+            var executionCount = 0;
+#endif
+            while (_executingRings[i] != null)
             {
-                continue;
-            }
+#if DEBUG_TIMERS
+                executionCount++;
+#endif
 
-            do
-            {
-                var next = timer._nextTimer;
+                var timer = _executingRings[i];
+
+                // Set the executing timer to the next in the link list because we will be detaching.
+                _executingRings[i] = timer._nextTimer;
 
                 timer.Detach();
 
@@ -116,15 +121,18 @@ public partial class Timer
                 {
                     timer.OnDetach();
                 }
-
-                timer = next;
-            } while (timer != null);
-
-            // Clear out the rings
-            _executingRings[i] = null;
+            }
+#if DEBUG_TIMERS
+            if (executionCount > _chainExecutionThreshold)
+            {
+                logger.Warning(
+                    "Timer threshold of {Threshold} met. Executed {Count} timers sequentially.",
+                    _chainExecutionThreshold,
+                    executionCount
+                );
+            }
+#endif
         }
-
-        _timerWheelExecuting = false;
     }
 
     private static void Execute(Timer timer)
@@ -232,7 +240,7 @@ public partial class Timer
 
                     total++;
 
-                    t = t?._nextTimer;
+                    t = t._nextTimer;
                 }
             }
         }
diff --git a/Projects/Server/Timer/Timer.cs b/Projects/Server/Timer/Timer.cs
index e736d0bda..f6d9a2870 100644
--- a/Projects/Server/Timer/Timer.cs
+++ b/Projects/Server/Timer/Timer.cs
@@ -153,20 +153,22 @@ public partial class Timer
 
         Running = false;
 
-        // Do not detach if we are in the middle of executing the timer wheel for this ring/slot
-        if (!_timerWheelExecuting || _ringIndexes[_ring] != _slot)
+        // We are the head on the timer ring
+        if (_rings[_ring][_slot] == this)
         {
-            // We are at the head
-            if (_rings[_ring][_slot] == this)
-            {
-                _rings[_ring][_slot] = _nextTimer;
-            }
-
-            Detach();
-            OnDetach();
+            _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();
         if (prof != null)