Handle Scheduling Edge Cases (#446)

This PR fixes two edge cases in scheduling.

The first was observed in mnemos, where adding a timer BEFORE the newest deadline would not be represented in the "time to next ticks" turn report, which made mnemos sleep longer than it should have.

The second was found while looking for the first (but not observed to cause any problems), where pending ticks (e.g. not advanced through force) that caused a sleep timer to expire would not be counted in the turn report of expired tasks on the next call to force.

maitake/src/time/timer.rs

@@ -568,13 +568,15 @@ impl Timer {
         let pending_ticks = self.pending_ticks.swap(0, AcqRel) as Ticks;
         // we do two separate `advance` calls here instead of advancing once
         // with the sum, because `ticks` + `pending_ticks` could overflow.
+        let mut pend_exp = 0;
         if pending_ticks > 0 {
-            core.advance(pending_ticks);
+            let (expired, _next_deadline) = core.advance(pending_ticks);
+            pend_exp = expired;
         }
         let (expired, next_deadline) = core.advance(ticks);
 
         Turn {
-            expired,
+            expired: expired.saturating_add(pend_exp),
             next_deadline_ticks: next_deadline.map(|d| d.ticks),
             now: core.now(),
             tick_duration: self.tick_duration,

maitake/src/time/timer/tests/wheel_tests.rs

@@ -183,6 +183,49 @@ impl SleepGroupTest {
     }
 }
 
+#[test]
+fn pend_advance_wakes() {
+    static TIMER: Timer = Timer::new(Duration::from_millis(1));
+    let mut test = SleepGroupTest::new(&TIMER);
+
+    test.spawn_group(100, 2);
+
+    // first tick --- timer is still at zero
+    let tick = test.scheduler.tick();
+    assert_eq!(tick.completed, 0);
+    test.assert();
+
+    // advance the timer by 50 ticks.
+    test.advance(50);
+
+    // advance the timer by 50 more ticks
+    // but ONLY by pending
+    test.now += 50;
+    test.timer.pend_ticks(50);
+
+    // Tick the scheduler, nothing should have happened
+    let tick = test.scheduler.tick();
+    assert_eq!(tick.completed, 0);
+
+    // How many do we expect to complete "now"? (it's two)
+    let expected_complete: usize = test
+        .groups
+        .iter_mut()
+        .take_while(|(&t, _)| t <= test.now)
+        .map(|(_, g)| std::mem::replace(&mut g.tasks, 0))
+        .sum();
+
+    // Call force, which will "notice" the pending ticks
+    let turn = test.timer.force_advance_ticks(0);
+    assert_eq!(turn.expired, expected_complete);
+
+    // NOW the tasks will show up as scheduled, and complete
+    let tick = test.scheduler.tick();
+    assert_eq!(tick.completed, expected_complete);
+
+    test.assert_all_complete();
+}
+
 #[test]
 fn timer_basically_works() {
     static TIMER: Timer = Timer::new(Duration::from_millis(1));
@@ -260,6 +303,55 @@ fn schedule_after_start() {
     test.assert_all_complete();
 }
 
+#[test]
+fn expired_shows_up() {
+    static TIMER: Timer = Timer::new(Duration::from_millis(1));
+    let mut test = SleepGroupTest::new(&TIMER);
+
+    test.spawn_group(150, 2);
+
+    // first tick --- timer is still at zero
+    let tick = test.scheduler.tick();
+    assert_eq!(tick.completed, 0);
+    test.assert();
+
+    // advance the timer by 50 ticks.
+    test.advance(50);
+
+    // Second tick - still nothing
+    let tick = test.scheduler.tick();
+    assert_eq!(tick.completed, 0);
+    test.assert();
+
+    // Add MORE items, sooner than the previous ones, to force a re-org
+    test.spawn_group(60, 3);
+
+    // Tick - nothing happens, but the new sleeps should now be registered.
+    let tick = test.scheduler.tick();
+    assert_eq!(tick.completed, 0);
+    test.assert();
+
+    // advance the timer by 50 more ticks, NOT past our new sleeps,
+    // but forward
+    test.now += 50;
+    let turn = test.timer.force_advance_ticks(50);
+    assert_eq!(turn.ticks_to_next_deadline(), Some(10));
+
+    let tick = test.scheduler.tick();
+    assert_eq!(tick.completed, 0);
+    test.assert();
+
+    // advance the timer by 10 ticks.
+    test.advance(10);
+    test.assert();
+
+    // advance the timer by 40 ticks.
+    test.advance(40);
+    test.assert();
+
+    test.assert_all_complete();
+}
+
 #[test]
 fn max_sleep() {
     static TIMER: Timer = Timer::new(Duration::from_millis(1));

maitake/src/time/timer/wheel.rs

@@ -165,12 +165,23 @@ impl Core {
             ?next_deadline,
             "wheel turned to"
         );
+
+        // If we need to reschedule something, we may need to recalculate the next deadline
+        let any = !pending_reschedule.is_empty();
+
         for entry in pending_reschedule {
             let deadline = unsafe { entry.as_ref().deadline };
+            debug_assert!(deadline > self.now);
             debug_assert_ne!(deadline, 0);
             self.insert_sleep_at(deadline, entry)
         }
 
+        // Yup, we rescheduled something. Recalculate the next deadline in case one of those
+        // was sooner than the last calculated deadline
+        if any {
+            next_deadline = self.next_deadline();
+        }
+
         (fired, next_deadline)
     }