fix(timers): enforce stored deadlines

A one-millisecond timer could become runnable immediately after registration.
Remove the early dispatch allowance from the shared scheduler so registered
and rescheduled timers become runnable only at or after their actual
monotonic deadline. All readiness queries use the same rule.

Regression coverage checks registration-time and pre-deadline queries,
exact-deadline dispatch through filtered and unfiltered paths, and interval
rescheduling without advancing the new deadline.

moli-renderer-v8/src/host/timers.rs

@@ -22,7 +22,7 @@ use crate::{
         get_private_value, script_nonce_from_host_defined_options,
     },
 };
-use moli_time::{TimerId, TimerReadyAllowance, TimerScheduler};
+use moli_time::{TimerId, TimerScheduler};
 use moli_webapi_declare::WebApiObject;
 
 #[derive(WebApiObject)]
@@ -211,7 +211,6 @@ impl ScheduledTimerTask {
     }
 }
 
-const MIN_DELAY_TIMER_READY_EARLY_ALLOWANCE: Duration = Duration::from_millis(1);
 const TIMER_CALLBACK_WATCHDOG_TIMEOUT: Duration = Duration::from_secs(8);
 
 #[derive(Default)]
@@ -469,18 +468,17 @@ impl HostTimeoutScheduler {
         ) else {
             return 0;
         };
-        self.scheduler
-            .schedule_after(
-                ScheduledTimerTask {
-                    callback: ScheduledTimerCallback::WindowWebIdl(callback),
-                    owner,
-                    is_interval,
-                    extra_args,
-                },
-                u64::from(delay_ms),
-                Instant::now(),
-            )
-            .get()
+        let task = ScheduledTimerTask {
+            callback: ScheduledTimerCallback::WindowWebIdl(callback),
+            owner,
+            is_interval,
+            extra_args,
+        };
+        let now = Instant::now();
+        let id = self
+            .scheduler
+            .schedule_after(task, u64::from(delay_ms), now);
+        id.get()
     }
 
     pub(crate) fn queue_source_once_with_receiver<'s>(
@@ -737,11 +735,12 @@ impl HostTimeoutScheduler {
         selection: RendererPageTimerSelection,
     ) -> HostTimeoutRunResult {
         let now_ms = crate::window_host::current_time_ms();
-        let Some(timer) = self.scheduler.take_next_ready_matching(
-            Instant::now(),
-            min_delay_ready_allowance(),
-            |task| task.matches_selection(selection, now_ms),
-        ) else {
+        let Some(timer) = self
+            .scheduler
+            .take_next_ready_matching(Instant::now(), |task| {
+                task.matches_selection(selection, now_ms)
+            })
+        else {
             return HostTimeoutRunResult::Idle;
         };
         self.run_timer(scope, timer)
@@ -749,8 +748,7 @@ impl HostTimeoutScheduler {
 
     #[cfg(test)]
     pub(crate) fn has_ready_timer(&self) -> bool {
-        self.scheduler
-            .has_ready_timer(Instant::now(), min_delay_ready_allowance())
+        self.scheduler.has_ready_timer(Instant::now())
     }
 
     pub(crate) fn next_ready_timer_deadline(
@@ -758,11 +756,10 @@ impl HostTimeoutScheduler {
         selection: RendererPageTimerSelection,
     ) -> Option<Instant> {
         let now_ms = crate::window_host::current_time_ms();
-        self.scheduler.next_ready_deadline_matching(
-            Instant::now(),
-            min_delay_ready_allowance(),
-            |task| task.matches_selection(selection, now_ms),
-        )
+        self.scheduler
+            .next_ready_deadline_matching(Instant::now(), |task| {
+                task.matches_selection(selection, now_ms)
+            })
     }
 
     fn run_timer(
@@ -1076,13 +1073,6 @@ fn timer_callback_relevant_context_is_current(
     })
 }
 
-fn min_delay_ready_allowance() -> TimerReadyAllowance {
-    TimerReadyAllowance {
-        max_delay_ms: 1,
-        allowance: MIN_DELAY_TIMER_READY_EARLY_ALLOWANCE,
-    }
-}
-
 fn run_window_timer_callback(
     scope: &mut v8::PinScope<'_, '_>,
     callback: &ScheduledTimerCallback,

moli-time/src/lib.rs

@@ -6,7 +6,7 @@ use std::{
 };
 
 mod timers;
-pub use timers::{ReadyTimer, TimerId, TimerReadyAllowance, TimerScheduler};
+pub use timers::{ReadyTimer, TimerId, TimerScheduler};
 
 pub fn unix_epoch_millis() -> f64 {
     SystemTime::now()

moli-time/src/timers.rs

@@ -17,19 +17,6 @@ impl TimerId {
     }
 }
 
-#[derive(Clone, Copy, Debug, Eq, PartialEq)]
-pub struct TimerReadyAllowance {
-    pub max_delay_ms: u32,
-    pub allowance: Duration,
-}
-
-impl TimerReadyAllowance {
-    pub const NONE: Self = Self {
-        max_delay_ms: 0,
-        allowance: Duration::ZERO,
-    };
-}
-
 #[derive(Debug)]
 pub struct ReadyTimer<T> {
     pub id: TimerId,
@@ -46,6 +33,12 @@ struct ScheduledTimer<T> {
     payload: T,
 }
 
+impl<T> ScheduledTimer<T> {
+    fn is_ready_at(&self, now: Instant) -> bool {
+        self.run_at <= now
+    }
+}
+
 impl<T> Ord for ScheduledTimer<T> {
     fn cmp(&self, other: &Self) -> Ordering {
         match self.run_at.cmp(&other.run_at).reverse() {
@@ -69,6 +62,7 @@ impl<T> PartialEq for ScheduledTimer<T> {
     }
 }
 
+/// Timers become runnable only at or after their stored monotonic deadline.
 #[derive(Debug)]
 pub struct TimerScheduler<T> {
     pending: BinaryHeap<ScheduledTimer<T>>,
@@ -137,18 +131,14 @@ impl<T> TimerScheduler<T> {
             .flatten()
     }
 
-    pub fn take_next_ready(
-        &mut self,
-        now: Instant,
-        allowance: TimerReadyAllowance,
-    ) -> Option<ReadyTimer<T>> {
+    pub fn take_next_ready(&mut self, now: Instant) -> Option<ReadyTimer<T>> {
         loop {
             let timer = self.pending.peek()?;
             if !self.active.contains(&timer.id) {
                 let _ = self.pending.pop();
                 continue;
             }
-            if !timer_ready(timer.run_at, timer.delay_ms, now, allowance) {
+            if !timer.is_ready_at(now) {
                 return None;
             }
 
@@ -168,13 +158,12 @@ impl<T> TimerScheduler<T> {
     pub fn take_next_ready_matching<F>(
         &mut self,
         now: Instant,
-        allowance: TimerReadyAllowance,
         predicate: F,
     ) -> Option<ReadyTimer<T>>
     where
         F: FnMut(&T) -> bool,
     {
-        let selected = self.next_ready_matching_timer(now, allowance, predicate)?;
+        let selected = self.next_ready_matching_timer(now, predicate)?;
         let selected_id = selected.id;
         let selected_sequence = selected.sequence;
 
@@ -203,19 +192,12 @@ impl<T> TimerScheduler<T> {
         })
     }
 
-    pub fn has_ready_matching<F>(
-        &self,
-        now: Instant,
-        allowance: TimerReadyAllowance,
-        mut predicate: F,
-    ) -> bool
+    pub fn has_ready_matching<F>(&self, now: Instant, mut predicate: F) -> bool
     where
         F: FnMut(&T) -> bool,
     {
         self.pending.iter().any(|timer| {
-            self.active.contains(&timer.id)
-                && timer_ready(timer.run_at, timer.delay_ms, now, allowance)
-                && predicate(&timer.payload)
+            self.active.contains(&timer.id) && timer.is_ready_at(now) && predicate(&timer.payload)
         })
     }
 
@@ -239,23 +221,17 @@ impl<T> TimerScheduler<T> {
         true
     }
 
-    pub fn has_ready_timer(&self, now: Instant, allowance: TimerReadyAllowance) -> bool {
-        self.pending.iter().any(|timer| {
-            self.active.contains(&timer.id)
-                && timer_ready(timer.run_at, timer.delay_ms, now, allowance)
-        })
+    pub fn has_ready_timer(&self, now: Instant) -> bool {
+        self.pending
+            .iter()
+            .any(|timer| self.active.contains(&timer.id) && timer.is_ready_at(now))
     }
 
-    pub fn next_ready_deadline_matching<F>(
-        &self,
-        now: Instant,
-        allowance: TimerReadyAllowance,
-        predicate: F,
-    ) -> Option<Instant>
+    pub fn next_ready_deadline_matching<F>(&self, now: Instant, predicate: F) -> Option<Instant>
     where
         F: FnMut(&T) -> bool,
     {
-        self.next_ready_matching_timer(now, allowance, predicate)
+        self.next_ready_matching_timer(now, predicate)
             .map(|timer| timer.run_at)
     }
 
@@ -309,22 +285,17 @@ impl<T> TimerScheduler<T> {
     fn next_ready_matching_timer<F>(
         &self,
         now: Instant,
-        allowance: TimerReadyAllowance,
         mut predicate: F,
     ) -> Option<&ScheduledTimer<T>>
     where
         F: FnMut(&T) -> bool,
     {
-        let mut first_non_ready = None;
         let mut selected = None;
         for timer in &self.pending {
             if !self.active.contains(&timer.id) {
                 continue;
             }
-            if !timer_ready(timer.run_at, timer.delay_ms, now, allowance) {
-                if first_non_ready.is_none_or(|current| timer_precedes(timer, current)) {
-                    first_non_ready = Some(timer);
-                }
+            if !timer.is_ready_at(now) {
                 continue;
             }
             if predicate(&timer.payload)
@@ -334,11 +305,7 @@ impl<T> TimerScheduler<T> {
             }
         }
 
-        let selected = selected?;
-        if first_non_ready.is_some_and(|barrier| timer_precedes(barrier, selected)) {
-            return None;
-        }
-        Some(selected)
+        selected
     }
 }
 
@@ -350,21 +317,58 @@ fn timer_precedes<T>(left: &ScheduledTimer<T>, right: &ScheduledTimer<T>) -> boo
     }
 }
 
-fn timer_ready(
-    run_at: Instant,
-    delay_ms: u64,
-    now: Instant,
-    allowance: TimerReadyAllowance,
-) -> bool {
-    run_at <= now
-        || (delay_ms <= u64::from(allowance.max_delay_ms)
-            && run_at.duration_since(now).le(&allowance.allowance))
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;
 
+    #[test]
+    fn timers_wait_until_their_deadline_in_every_ready_query() {
+        let now = Instant::now();
+        for delay_ms in [1, 2, 100] {
+            for consume_matching in [false, true] {
+                let mut scheduler = TimerScheduler::default();
+                let id = scheduler.schedule_after("timer", delay_ms, now);
+                let deadline = now + Duration::from_millis(delay_ms);
+
+                for before_deadline in [now, deadline - Duration::from_nanos(1)] {
+                    assert!(!scheduler.has_ready_timer(before_deadline));
+                    assert!(!scheduler.has_ready_matching(before_deadline, |_| true));
+                    assert_eq!(
+                        scheduler.next_ready_deadline_matching(before_deadline, |_| true),
+                        None
+                    );
+                    assert!(scheduler.take_next_ready(before_deadline).is_none());
+                    assert!(
+                        scheduler
+                            .take_next_ready_matching(before_deadline, |_| true)
+                            .is_none()
+                    );
+                    assert_eq!(scheduler.next_deadline(), Some(deadline));
+                    assert!(scheduler.ms_to_next(before_deadline).unwrap() > 0);
+                    assert_eq!(scheduler.pending_count(), 1);
+                }
+
+                assert!(scheduler.has_ready_timer(deadline));
+                assert!(scheduler.has_ready_matching(deadline, |_| true));
+                assert_eq!(
+                    scheduler.next_ready_deadline_matching(deadline, |_| true),
+                    Some(deadline)
+                );
+                assert_eq!(scheduler.ms_to_next(deadline), Some(0));
+                let ready = if consume_matching {
+                    scheduler.take_next_ready_matching(deadline, |_| true)
+                } else {
+                    scheduler.take_next_ready(deadline)
+                }
+                .unwrap();
+                assert_eq!(ready.id, id);
+                assert_eq!(ready.payload, "timer");
+                scheduler.finish_running(id);
+                assert_eq!(scheduler.pending_count(), 0);
+            }
+        }
+    }
+
     #[test]
     fn ready_timers_fire_by_deadline_then_sequence() {
         let now = Instant::now();
@@ -374,14 +378,14 @@ mod tests {
         let second = scheduler.schedule_after("second", 10, now);
 
         let ready = scheduler
-            .take_next_ready(now + Duration::from_millis(10), TimerReadyAllowance::NONE)
+            .take_next_ready(now + Duration::from_millis(10))
             .expect("first timer should be ready");
         assert_eq!(ready.id, first);
         assert_eq!(ready.payload, "first");
         scheduler.finish_running(ready.id);
 
         let ready = scheduler
-            .take_next_ready(now + Duration::from_millis(10), TimerReadyAllowance::NONE)
+            .take_next_ready(now + Duration::from_millis(10))
             .expect("second timer should be ready");
         assert_eq!(ready.id, second);
         assert_eq!(ready.payload, "second");
@@ -389,12 +393,12 @@ mod tests {
 
         assert!(
             scheduler
-                .take_next_ready(now + Duration::from_millis(10), TimerReadyAllowance::NONE)
+                .take_next_ready(now + Duration::from_millis(10))
                 .is_none()
         );
 
         let ready = scheduler
-            .take_next_ready(now + Duration::from_millis(20), TimerReadyAllowance::NONE)
+            .take_next_ready(now + Duration::from_millis(20))
             .expect("slow timer should be ready");
         assert_eq!(ready.id, slow);
         assert_eq!(ready.payload, "slow");
@@ -411,15 +415,10 @@ mod tests {
             assert_eq!(scheduler.next_deadline(), Some(deadline));
             assert!(
                 scheduler
-                    .take_next_ready(
-                        deadline - Duration::from_millis(1),
-                        TimerReadyAllowance::NONE
-                    )
+                    .take_next_ready(deadline - Duration::from_millis(1))
                     .is_none()
             );
-            let ready = scheduler
-                .take_next_ready(deadline, TimerReadyAllowance::NONE)
-                .unwrap();
+            let ready = scheduler.take_next_ready(deadline).unwrap();
             assert_eq!(ready.id, id);
             assert_eq!(ready.delay_ms, delay_ms);
             assert!(scheduler.reschedule_running_after(id, ready.payload, delay_ms, deadline));
@@ -442,16 +441,12 @@ mod tests {
         scheduler.cancel(cancelled);
 
         let ready = scheduler
-            .take_next_ready(now, TimerReadyAllowance::NONE)
+            .take_next_ready(now)
             .expect("kept timer should be ready");
         assert_eq!(ready.id, kept);
         assert_eq!(ready.payload, "kept");
         scheduler.finish_running(ready.id);
-        assert!(
-            scheduler
-                .take_next_ready(now, TimerReadyAllowance::NONE)
-                .is_none()
-        );
+        assert!(scheduler.take_next_ready(now).is_none());
     }
 
     #[test]
@@ -461,7 +456,7 @@ mod tests {
         let interval = scheduler.schedule_after("tick", 0, now);
 
         let ready = scheduler
-            .take_next_ready(now, TimerReadyAllowance::NONE)
+            .take_next_ready(now)
             .expect("interval should be ready");
         scheduler.cancel(interval);
         assert!(!scheduler.reschedule_running_after(
@@ -493,7 +488,7 @@ mod tests {
         assert_eq!(scheduler.pending_count(), 1);
 
         let ready = scheduler
-            .take_next_ready(now + Duration::from_millis(12), TimerReadyAllowance::NONE)
+            .take_next_ready(now + Duration::from_millis(12))
             .expect("kept timer should become ready");
         assert_eq!(ready.id, kept);
         scheduler.finish_running(ready.id);
@@ -509,40 +504,40 @@ mod tests {
         scheduler.cancel(cancelled);
 
         assert_eq!(
-            scheduler
-                .next_ready_deadline_matching(now, TimerReadyAllowance::NONE, |payload| *payload
-                    == "selected",),
+            scheduler.next_ready_deadline_matching(now, |payload| *payload == "selected",),
             None
         );
         assert_eq!(
-            scheduler.next_ready_deadline_matching(
-                now + Duration::from_millis(1),
-                TimerReadyAllowance::NONE,
-                |payload| *payload == "selected",
-            ),
+            scheduler
+                .next_ready_deadline_matching(now + Duration::from_millis(1), |payload| *payload
+                    == "selected",),
             Some(now + Duration::from_millis(1))
         );
     }
 
     #[test]
-    fn matching_ready_deadline_respects_an_earlier_non_ready_barrier() {
+    fn matching_ready_deadline_does_not_admit_a_future_timer() {
         let now = Instant::now();
         let mut scheduler = TimerScheduler::default();
-        scheduler.schedule_after("barrier", 2, now);
+        scheduler.schedule_after("earlier", 2, now);
         scheduler.schedule_after("selected", 1, now + Duration::from_micros(1_500));
-        let allowance = TimerReadyAllowance {
-            max_delay_ms: 1,
-            allowance: Duration::from_millis(1),
-        };
-
+        let before_selected = now + Duration::from_micros(2_499);
+        assert!(scheduler.has_ready_timer(before_selected));
         assert_eq!(
-            scheduler.next_ready_deadline_matching(
-                now + Duration::from_micros(1_500),
-                allowance,
-                |payload| *payload == "selected",
-            ),
-            None,
-            "a later timer admitted by early allowance must not overtake the heap head"
+            scheduler
+                .next_ready_deadline_matching(before_selected, |payload| *payload == "selected"),
+            None
+        );
+        assert!(
+            scheduler
+                .take_next_ready_matching(before_selected, |payload| *payload == "selected")
+                .is_none()
+        );
+        assert_eq!(
+            scheduler.next_ready_deadline_matching(now + Duration::from_micros(2_500), |payload| {
+                *payload == "selected"
+            },),
+            Some(now + Duration::from_micros(2_500))
         );
     }
 
@@ -564,25 +559,25 @@ mod tests {
     }
 
     #[test]
-    fn early_allowance_only_applies_to_short_delays() {
+    fn rescheduled_short_timer_waits_for_its_new_deadline() {
         let now = Instant::now();
         let mut scheduler = TimerScheduler::default();
-        let short = scheduler.schedule_after("short", 1, now);
-        scheduler.schedule_after("long", 2, now);
+        let id = scheduler.schedule_after("tick", 1, now);
+        let first_deadline = now + Duration::from_millis(1);
+        let ready = scheduler.take_next_ready(first_deadline).unwrap();
+        assert!(scheduler.reschedule_running_after(id, ready.payload, 1, first_deadline));
 
-        let allowance = TimerReadyAllowance {
-            max_delay_ms: 1,
-            allowance: Duration::from_millis(1),
-        };
-        let just_before = now + Duration::from_micros(500);
-        assert!(scheduler.has_ready_timer(just_before, allowance));
-        let ready = scheduler
-            .take_next_ready(just_before, allowance)
-            .expect("short timer should be ready within allowance");
-        assert_eq!(ready.id, short);
-        scheduler.finish_running(ready.id);
-
-        assert!(!scheduler.has_ready_timer(just_before, allowance));
+        let next_deadline = first_deadline + Duration::from_millis(1);
+        for before_deadline in [first_deadline, next_deadline - Duration::from_nanos(1)] {
+            assert!(!scheduler.has_ready_timer(before_deadline));
+            assert!(scheduler.take_next_ready(before_deadline).is_none());
+            assert_eq!(scheduler.next_deadline(), Some(next_deadline));
+        }
+        let ready = scheduler.take_next_ready(next_deadline).unwrap();
+        assert_eq!(ready.id, id);
+        assert_eq!(ready.payload, "tick");
+        scheduler.finish_running(id);
+        assert_eq!(scheduler.pending_count(), 0);
     }
 
     #[test]
@@ -593,29 +588,23 @@ mod tests {
         let selected = scheduler.schedule_after("selected", 0, now);
         let second = scheduler.schedule_after("second", 0, now);
 
-        assert!(
-            scheduler.has_ready_matching(now, TimerReadyAllowance::NONE, |payload| {
-                *payload == "selected"
-            })
-        );
+        assert!(scheduler.has_ready_matching(now, |payload| { *payload == "selected" }));
         let ready = scheduler
-            .take_next_ready_matching(now, TimerReadyAllowance::NONE, |payload| {
-                *payload == "selected"
-            })
+            .take_next_ready_matching(now, |payload| *payload == "selected")
             .expect("selected timer should be ready");
         assert_eq!(ready.id, selected);
         assert_eq!(ready.payload, "selected");
         scheduler.finish_running(ready.id);
 
         let ready = scheduler
-            .take_next_ready(now, TimerReadyAllowance::NONE)
+            .take_next_ready(now)
             .expect("first timer should remain pending");
         assert_eq!(ready.id, first);
         assert_eq!(ready.payload, "first");
         scheduler.finish_running(ready.id);
 
         let ready = scheduler
-            .take_next_ready(now, TimerReadyAllowance::NONE)
+            .take_next_ready(now)
             .expect("second timer should remain pending");
         assert_eq!(ready.id, second);
         assert_eq!(ready.payload, "second");
@@ -633,16 +622,14 @@ mod tests {
         let selected = scheduler.schedule_after("selected", 0, now);
 
         let ready = scheduler
-            .take_next_ready_matching(now, TimerReadyAllowance::NONE, |payload| {
-                *payload == "selected"
-            })
+            .take_next_ready_matching(now, |payload| *payload == "selected")
             .expect("selected timer should be ready after skipped timers");
         assert_eq!(ready.id, selected);
         assert_eq!(ready.payload, "selected");
         scheduler.finish_running(ready.id);
 
         let ready = scheduler
-            .take_next_ready(now, TimerReadyAllowance::NONE)
+            .take_next_ready(now)
             .expect("first timer should remain first after matching drain");
         assert_eq!(ready.id, first);
         assert_eq!(ready.payload, "first");
@@ -650,7 +637,7 @@ mod tests {
 
         for skipped_id in skipped {
             let ready = scheduler
-                .take_next_ready(now, TimerReadyAllowance::NONE)
+                .take_next_ready(now)
                 .expect("skipped timer should remain pending");
             assert_eq!(ready.id, skipped_id);
             assert_eq!(ready.payload, "skipped");