Fixing Timer.Change issues with negative dueTimes

xavivars · sjdirect · commit f6352c3ac0aa · 2015-09-21T10:59:12.000-07:00
* First: The implementation of the ExitTimerCallback checks the queue during a while() loop, but then again at a subsequent if(). This is unnecessary, but also this if()’s TryPeek() output may be past-due, resulting in a negative number in timeUntilNextCheck. * Second: Environment.TickCount rolls over every 24.9 days, which can also mess with constant-up service deployments. More info: http://www.jackleitch.net/2010/10/better-rate-limiting-with-dot-net/#comment-3620 Signed-off-by: sjdirect <sjdirect@gmail.com>
diff --git a/Abot/Util/RateLimiter.cs b/Abot/Util/RateLimiter.cs
@@ -168,24 +168,22 @@ private void ExitTimerCallback(object state)
             // While there are exit times that are passed due still in the queue,
             // exit the semaphore and dequeue the exit time.
             int exitTime;
-            while (_exitTimes.TryPeek(out exitTime)
-                    && unchecked(exitTime - Environment.TickCount) <= 0)
+            bool exitTimeValid = _exitTimes.TryPeek(out exitTime);
+            while (exitTimeValid)
             {
+                if (unchecked(exitTime - Environment.TickCount) > 0)
+                {
+                    break;
+                }
                 _semaphore.Release();
                 _exitTimes.TryDequeue(out exitTime);
+                exitTimeValid = _exitTimes.TryPeek(out exitTime);
             }
 
-            // Try to get the next exit time from the queue and compute
-            // the time until the next check should take place. If the 
-            // queue is empty, then no exit times will occur until at least
-            // one time unit has passed.
-            int timeUntilNextCheck;
-            if (_exitTimes.TryPeek(out exitTime))
-                timeUntilNextCheck = unchecked(exitTime - Environment.TickCount);
-            else
-                timeUntilNextCheck = TimeUnitMilliseconds;
-
-            // Set the timer.
+            // we are already holding the next item from the queue, do not peek again
+            // although this exit time may have already pass by this stmt.
+            var timeUntilNextCheck = exitTimeValid ? Math.Min(TimeUnitMilliseconds, Math.Max(0, exitTime - Environment.TickCount)) : TimeUnitMilliseconds;
+
             _exitTimer.Change(timeUntilNextCheck, -1);
         }
 
