CR: adding comments; reset --> new_execution; other small cleanups

Author: Dylan Wolff

shuttle/src/runtime/execution.rs

@@ -5,7 +5,7 @@ use crate::runtime::task::labels::Labels;
 use crate::runtime::task::{ChildLabelFn, Task, TaskId, TaskName, TaskSignature, DEFAULT_INLINE_TASKS};
 use crate::runtime::thread::continuation::PooledContinuation;
 use crate::scheduler::{Schedule, Scheduler};
-use crate::sync::time::TimeModel;
+use crate::sync::time::{get_time_model, TimeModel};
 use crate::sync::{ResourceSignature, ResourceType};
 use crate::thread::thread_fn;
 use crate::{Config, MaxSteps};
@@ -73,6 +73,15 @@ fn backtrace_enabled() -> bool {
     std::env::var("RUST_BACKTRACE").is_ok() || std::env::var("RUST_LIB_BACKTRACE").is_ok()
 }
 
+/// While there are no runnable tasks and tasks are able to be woken by the time model, continually wakes tasks.
+/// The TimeModel's `wake_next` method is called in a loop in case there are stale wakers which do not actually
+/// result in a newly runnable task when woken. Requires access to the ExecutionState to obtain a reference to the
+/// time model.
+fn wake_sleepers_until_runnable() {
+    let tm = get_time_model();
+    while ExecutionState::num_runnable() == 0 && tm.borrow_mut().wake_next() {}
+}
+
 impl Execution {
     /// Run a function to be tested, taking control of scheduling it and any tasks it might spawn.
     /// This function runs until `f` and all tasks spawned by `f` have terminated, or until the
@@ -103,7 +112,6 @@ impl Execution {
 
             // Cleanup the state before it goes out of `EXECUTION_STATE` scope
             ExecutionState::cleanup();
-            self.time_model.borrow_mut().reset();
         });
     }
 
@@ -116,12 +124,7 @@ impl Execution {
             Finished,
         }
 
-        // While there are no runnable tasks and tasks are able to be woken by the time model, continue waking tasks
-        while ExecutionState::num_runnable() == 0
-            && ExecutionState::with(|s| Rc::clone(&s.time_model))
-                .borrow_mut()
-                .wake_next()
-        {}
+        wake_sleepers_until_runnable();
 
         let next_step = ExecutionState::with(|state| {
             if let Err(msg) = state.schedule() {
@@ -596,12 +599,7 @@ impl ExecutionState {
     /// is different from the currently running task, indicating that the current task should yield
     /// its execution.
     pub(crate) fn maybe_yield() -> bool {
-        // While there are no runnable tasks and tasks are able to be woken by the time model, continue waking tasks
-        while ExecutionState::num_runnable() == 0
-            && ExecutionState::with(|s| Rc::clone(&s.time_model))
-                .borrow_mut()
-                .wake_next()
-        {}
+        wake_sleepers_until_runnable();
 
         Self::with(|state| {
             debug_assert!(

shuttle/src/runtime/runner.rs

@@ -111,6 +111,7 @@ impl<S: Scheduler + 'static, T: TimeModel + 'static> Runner<S, T> {
                     None => break,
                     Some(s) => s,
                 };
+                self.time_model.borrow_mut().new_execution();
 
                 let execution = Execution::new(self.scheduler.clone(), schedule, self.time_model.clone());
                 let f = Arc::clone(&f);

shuttle/src/runtime/thread/continuation.rs

@@ -5,6 +5,7 @@
 #![allow(deprecated)]
 
 use crate::runtime::execution::ExecutionState;
+use crate::sync::time::get_time_model;
 use generator::{Generator, Gn};
 use scoped_tls::scoped_thread_local;
 use std::cell::{Cell, RefCell};
@@ -262,7 +263,7 @@ pub(crate) fn switch() {
         let r = generator::yield_(ContinuationOutput::Yielded).unwrap();
         assert!(matches!(r, ContinuationInput::Resume));
     }
-    ExecutionState::with(|s| Rc::clone(&s.time_model)).borrow_mut().step();
+    get_time_model().borrow_mut().step()
 }
 
 #[cfg(test)]

shuttle/src/sync/time/constant_stepped.rs

@@ -4,7 +4,7 @@ use std::{
     task::Waker,
 };
 
-use tracing::{debug, warn};
+use tracing::{trace, warn};
 
 use crate::{current::TaskId, runtime::execution::ExecutionState};
 
@@ -73,12 +73,12 @@ impl TimeModel for ConstantSteppedTimeModel {
     }
 
     fn step(&mut self) {
-        debug!("step");
         self.current_time_elapsed += self.current_step_size;
+        trace!("time step to {:?}", self.current_time_elapsed);
         self.unblock_expired();
     }
 
-    fn reset(&mut self) {
+    fn new_execution(&mut self) {
         self.current_step_size = self.distribution.sample();
         self.current_time_elapsed = std::time::Duration::from_secs(0);
         self.waiters.clear();

shuttle/src/sync/time/frozen.rs

@@ -100,8 +100,8 @@ impl TimeModel for FrozenTimeModel {
 
     fn step(&mut self) {}
 
-    fn reset(&mut self) {
-        self.inner.reset();
+    fn new_execution(&mut self) {
+        self.inner.new_execution();
         self.expired.clear();
         self.triggers.clear();
     }

shuttle/src/sync/time/mod.rs

@@ -51,7 +51,7 @@ pub trait TimeModel: std::fmt::Debug {
     /// Called when all tasks are blocked to resolve timing based deadlocks (all unblocked tasks are sleeping).
     fn wake_next(&mut self) -> bool;
     /// Reset the TimeModel state for the next Shuttle iteration
-    fn reset(&mut self);
+    fn new_execution(&mut self);
     /// Callback after each scheduling step to allow the TimeModel to update itself
     fn step(&mut self);
     /// Used to create the TimeModel's Instant struct in functions like Instant::now()
@@ -71,7 +71,8 @@ pub trait TimeModel: std::fmt::Debug {
     fn as_any_mut(&mut self) -> &mut dyn std::any::Any;
 }
 
-/// Provides a reference to the current TimeModel for this execution
+/// Provides a reference to the current TimeModel for this execution.
+/// Uses `Rc::clone` so that ExecutionState isn't already borrowed when running most TimeModel methods
 pub fn get_time_model() -> Rc<RefCell<dyn TimeModel>> {
     ExecutionState::with(|s| Rc::clone(&s.time_model))
 }