Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make test-util paused time fully deterministic #3492

Merged
merged 10 commits into from Feb 5, 2021
Merged
31 changes: 31 additions & 0 deletions tokio/src/runtime/builder.rs
Expand Up @@ -47,6 +47,9 @@ pub struct Builder {
/// Whether or not to enable the time driver
enable_time: bool,

/// Whether or not the clock should start paused.
start_paused: bool,

/// The number of worker threads, used by Runtime.
///
/// Only used when not using the current-thread executor.
Expand Down Expand Up @@ -110,6 +113,9 @@ impl Builder {
// Time defaults to "off"
enable_time: false,

// The clock starts not-paused
start_paused: false,

// Default to lazy auto-detection (one thread per CPU core)
worker_threads: None,

Expand Down Expand Up @@ -386,6 +392,7 @@ impl Builder {
},
enable_io: self.enable_io,
enable_time: self.enable_time,
start_paused: self.start_paused,
}
}

Expand Down Expand Up @@ -489,6 +496,30 @@ cfg_time! {
}
}

cfg_test_util! {
impl Builder {
/// Controls if the runtime's clock starts paused or advancing.
///
/// Pausing time requires the current-thread runtime; construction of
/// the runtime will panic otherwise.
Comment on lines +503 to +504
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is this panic?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The call to clock.pause in Clock::new will panic when running on a multi threaded runtime.

///
/// # Examples
///
/// ```
/// use tokio::runtime;
///
/// let rt = runtime::Builder::new_current_thread()
/// .enable_time()
/// .start_paused(true)
/// .unwrap();
/// ```
pub fn start_paused(&mut self, start_paused: bool) -> &mut Self {
self.start_paused = start_paused;
self
}
}
}

cfg_rt_multi_thread! {
impl Builder {
fn build_threaded_runtime(&mut self) -> io::Result<Runtime> {
Expand Down
5 changes: 5 additions & 0 deletions tokio/src/runtime/driver.rs
Expand Up @@ -162,13 +162,18 @@ pub(crate) struct Cfg {
pub(crate) enable_io: bool,
pub(crate) enable_time: bool,
pub(crate) enable_pause_time: bool,
pub(crate) start_paused: bool,
}

impl Driver {
pub(crate) fn new(cfg: Cfg) -> io::Result<(Self, Resources)> {
let (io_stack, io_handle, signal_handle) = create_io_stack(cfg.enable_io)?;

let clock = create_clock(cfg.enable_pause_time);
if cfg.start_paused {
clock.pause();
}

let (time_driver, time_handle) =
create_time_driver(cfg.enable_time, io_stack, clock.clone());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this makes it deterministic because pause is called before create_time_driver sets the start time?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That, along with changing the ClockTime constructor to pull start_time out of the clock passed to it rather than going for the global clock (which isn't installed at that point so it falls back to the system clock).


Expand Down
2 changes: 1 addition & 1 deletion tokio/src/time/driver/mod.rs
Expand Up @@ -102,8 +102,8 @@ pub(self) struct ClockTime {
impl ClockTime {
pub(self) fn new(clock: Clock) -> Self {
Self {
start_time: clock.now(),
clock,
start_time: super::clock::now(),
}
}

Expand Down