Implement clamped catchup strategy, allow timers to detect how many intervals behind they are

Author: HaroldCindy

VM/src/llltimers.cpp

@@ -16,8 +16,9 @@
 enum TimerDataIndex {
     TIMER_HANDLER = 1,
     TIMER_INTERVAL = 2,
-    TIMER_NEXT_RUN = 3,
-    TIMER_LEN = TIMER_NEXT_RUN,
+    TIMER_NEXT_RUN = 3,              // Actual time to fire (may be clamped for catch-up)
+    TIMER_LOGICAL_SCHEDULE = 4,      // Logical schedule time (never clamped, always += interval)
+    TIMER_LEN = TIMER_LOGICAL_SCHEDULE,
 };
 
 // Timer event wrapper for LLEvents integration
@@ -132,13 +133,16 @@ static int lltimers_on(lua_State *L)
     int old_len = lua_objlen(L, -1);
 
     // Create timer data table
-    lua_createtable(L, 3, 0);
+    lua_createtable(L, 4, 0);
     lua_pushvalue(L, 3);
     lua_rawseti(L, -2, TIMER_HANDLER);
-    lua_pushnumber(L, current_time + seconds);
-    lua_rawseti(L, -2, TIMER_NEXT_RUN);
     lua_pushnumber(L, seconds);
     lua_rawseti(L, -2, TIMER_INTERVAL);
+    double next_run_time = current_time + seconds;
+    lua_pushnumber(L, next_run_time);
+    lua_rawseti(L, -2, TIMER_NEXT_RUN);
+    lua_pushnumber(L, next_run_time);
+    lua_rawseti(L, -2, TIMER_LOGICAL_SCHEDULE);
 
     LUAU_ASSERT(lua_objlen(L, -1) == TIMER_LEN);
 
@@ -183,13 +187,16 @@ static int lltimers_once(lua_State *L)
     int old_len = lua_objlen(L, -1);
 
     // Create timer data table
-    lua_createtable(L, 3, 0);
+    lua_createtable(L, 4, 0);
     lua_pushvalue(L, 3);
     lua_rawseti(L, -2, TIMER_HANDLER);
     lua_pushnil(L);
     lua_rawseti(L, -2, TIMER_INTERVAL);
-    lua_pushnumber(L, current_time + seconds);
+    double next_run_time = current_time + seconds;
+    lua_pushnumber(L, next_run_time);
     lua_rawseti(L, -2, TIMER_NEXT_RUN);
+    lua_pushnumber(L, next_run_time);
+    lua_rawseti(L, -2, TIMER_LOGICAL_SCHEDULE);
     LUAU_ASSERT(lua_objlen(L, -1) == TIMER_LEN);
 
     // Add to the timers array
@@ -503,6 +510,15 @@ static int lltimers_tick_cont(lua_State *L, [[maybe_unused]]int status)
         double next_run = lua_tonumber(L, -1);
         lua_pop(L, 1);
 
+        // Get the logical schedule time (what we'll pass to the handler)
+        // We read this BEFORE updating it so the handler gets the current value
+        lua_rawgeti(L, CURRENT_TIMER, TIMER_LOGICAL_SCHEDULE);
+        double logical_schedule = lua_tonumber(L, -1);
+        lua_pop(L, 1);
+
+        // Save the original value - this is what the handler should receive
+        double handler_scheduled_time = logical_schedule;
+
         // Verify nextRun is a reasonable number
         LUAU_ASSERT(next_run >= 0.0);
 
@@ -555,15 +571,61 @@ static int lltimers_tick_cont(lua_State *L, [[maybe_unused]]int status)
         }
         else
         {
-            // Schedule its next run using absolute scheduling
+            // Schedule its next run using absolute scheduling with clamped catch-up
             // (next = previous_scheduled_time + interval)
-            // This prevents drift and ensures the timer maintains its rhythm
-            // regardless of execution delays.
+            // This prevents drift and ensures the timer maintains its rhythm.
+            // However, if the timer is very late (> 2 seconds), skip ahead to prevent
+            // excessive catch-up iterations that could bog down the system.
+            //
+            // We maintain two schedules:
+            // - TIMER_NEXT_RUN: May be clamped to prevent catch-up storms
+            // - TIMER_LOGICAL_SCHEDULE: Never clamped, always += interval
+            //   This lets handlers know their true delay from the logical schedule.
+            //
             // Note that we do this BEFORE the timer is ever run.
             // This ensures that handler runtime has no effect on
             // when the handler will be invoked next.
-            lua_pushnumber(L, next_run + interval);
+            const double MAX_CATCHUP_TIME = 2.0;
+            double next_scheduled = next_run + interval;
+            double new_next_run;
+            bool did_clamp = false;
+
+            // Check if the next scheduled time would still be >2s behind current time
+            if (start_time - next_scheduled > MAX_CATCHUP_TIME)
+            {
+                // Skip ahead to next interval after current time
+                // This prevents spiral of death from excessive catch-up
+                double time_behind = start_time - next_run;
+                double intervals_to_skip = ceil(time_behind / interval);
+                new_next_run = next_run + (intervals_to_skip * interval);
+                did_clamp = true;
+            }
+            else
+            {
+                // Normal absolute scheduling - maintain rhythm
+                new_next_run = next_scheduled;
+            }
+
+            // Update actual next run time (may be clamped)
+            lua_pushnumber(L, new_next_run);
             lua_rawseti(L, CURRENT_TIMER, TIMER_NEXT_RUN);
+
+            // Update logical schedule
+            // When clamping, sync to new_next_run (reset to new reality)
+            // When not clamping, increment normally (logical_schedule + interval)
+            if (did_clamp)
+            {
+                // Sync logical schedule when clamping - we're giving up on catch-up
+                // so reset the logical schedule to match the new reality.
+                // This ensures handlers see the initial delay (current fire), then return to normal.
+                lua_pushnumber(L, new_next_run);
+            }
+            else
+            {
+                // Normal increment - maintain rhythm
+                lua_pushnumber(L, logical_schedule + interval);
+            }
+            lua_rawseti(L, CURRENT_TIMER, TIMER_LOGICAL_SCHEDULE);
         }
 
         // Get the handler from the timer and call it
@@ -582,10 +644,20 @@ static int lltimers_tick_cont(lua_State *L, [[maybe_unused]]int status)
 
         // No pcall(), errors bubble up to the global error handler!
         lua_pushvalue(L, HANDLER_FUNC);
-        // Include when it was scheduled to run as an arg, allowing callees to do a diff between
+        // Include when it was scheduled to run as first arg, allowing callees to do a diff between
         // scheduled and actual time.
-        lua_pushnumber(L, next_run);
-        lua_call(L, 1, 0);
+        // We pass the saved handler_scheduled_time (the original logical schedule) so handlers
+        // can detect delays. When clamping occurs, the handler still receives the ORIGINAL
+        // scheduled time (when it was supposed to run), not the synced time.
+        lua_pushnumber(L, handler_scheduled_time);
+        // Include the interval as second arg, enabling handlers to calculate missed intervals.
+        // For once() timers, this will be nil. For on() timers, it's the interval.
+        if (is_one_shot) {
+            lua_pushnil(L);
+        } else {
+            lua_pushnumber(L, interval);
+        }
+        lua_call(L, 2, 0);
 
         if (L->status == LUA_YIELD || L->status == LUA_BREAK)
         {

tests/conformance/lltimers.lua

@@ -20,8 +20,9 @@ end
 -- Test basic on() functionality
 setclock(0.0)
 local on_count = 0
-local on_handler = LLTimers:on(0.1, function()
+local on_handler = LLTimers:on(0.1, function(scheduled_time, interval)
     on_count += 1
+    assert(interval == 0.1, "on() timer should receive interval")
 end)
 
 assert(typeof(on_handler) == "function")
@@ -50,8 +51,9 @@ LLTimers:off(on_handler)
 -- Test once() functionality
 setclock(1.0)
 local once_count = 0
-local once_handler = LLTimers:once(0.1, function()
+local once_handler = LLTimers:once(0.1, function(scheduled_time, interval)
     once_count += 1
+    assert(interval == nil, "once() timer should receive nil interval")
 end)
 
 incrementclock(0.1) -- Should fire the once handler
@@ -520,4 +522,36 @@ assert(math.abs(repeat_scheduled_times[1] - 35.5) < 0.001)
 assert(math.abs(repeat_scheduled_times[2] - 36.0) < 0.001)
 assert(math.abs(repeat_scheduled_times[3] - 36.5) < 0.001)
 
+-- Test clamped catch-up: timers >2s late skip ahead instead of rapid-firing
+setclock(40.0)
+local catchup_fires = 0
+local catchup_scheduled_times = {}
+local catchup_handler = LLTimers:on(0.1, function(scheduled_time)
+    catchup_fires += 1
+    table.insert(catchup_scheduled_times, scheduled_time)
+end)
+
+-- Make timer VERY late (4.9 seconds, exceeds 2-second threshold)
+setclock(45.0)
+LLEvents:_handleEvent('timer')
+
+-- Should fire ONCE per _handleEvent call, not 49 times
+assert(catchup_fires == 1, "Timer should fire once per handleEvent call")
+
+-- scheduled_time parameter shows when it WAS scheduled (40.1), not when it got rescheduled to
+assert(math.abs(catchup_scheduled_times[1] - 40.1) < 0.001, "First fire shows original schedule time")
+
+-- Fire again to verify logical schedule syncs when clamping
+setclock(45.1)
+LLEvents:_handleEvent('timer')
+assert(catchup_fires == 2, "Should fire again on next handleEvent")
+-- When we clamp, we sync the logical schedule to the new reality
+-- This means handlers see the initial delay (first fire), then return to normal
+assert(catchup_scheduled_times[2] > 44.9, "Second fire shows synced schedule (~45.0)")
+assert(catchup_scheduled_times[2] < 45.2, "Second fire shows synced schedule (~45.0)")
+-- Handler sees normal delay now: getclock() - scheduled_time = 45.1 - 45.0 = ~0.1s
+
+-- Clean up
+LLTimers:off(catchup_handler)
+
 return "OK"