From 016ed8966a995c629ddb5a8b97e4224e2a60a5d5 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Tue, 19 Jan 2021 15:11:54 -0800 Subject: [PATCH] poll now handles monotonic clocks, not system clocks I initially had it backwards. It is not sensible to poll on a system clock timeout because the system clock is not necessarily monotonic! only a monotonic clock makes sense for a timeout. --- crates/wasi-c2/src/sched.rs | 20 ++++++++++---------- crates/wasi-c2/src/sched/subscription.rs | 22 +++++++++++----------- crates/wasi-c2/src/sched/sync.rs | 6 +++--- crates/wasi-c2/src/snapshots/preview_1.rs | 18 ++++++++++++------ 4 files changed, 36 insertions(+), 30 deletions(-) diff --git a/crates/wasi-c2/src/sched.rs b/crates/wasi-c2/src/sched.rs index 3e6a82535f..bb678c5d8b 100644 --- a/crates/wasi-c2/src/sched.rs +++ b/crates/wasi-c2/src/sched.rs @@ -1,14 +1,14 @@ -use crate::clocks::WasiSystemClock; +use crate::clocks::WasiMonotonicClock; use crate::file::WasiFile; use crate::Error; -use cap_std::time::{Duration, SystemTime}; +use cap_std::time::{Duration, Instant}; use std::cell::Ref; pub mod subscription; mod sync; pub use sync::SyncSched; -use subscription::{RwSubscription, Subscription, SubscriptionResult, SystemTimerSubscription}; +use subscription::{MonotonicClockSubscription, RwSubscription, Subscription, SubscriptionResult}; pub trait WasiSched { fn poll_oneoff(&self, poll: &Poll) -> Result<(), Error>; @@ -36,15 +36,15 @@ impl<'a> Poll<'a> { pub fn new() -> Self { Self { subs: Vec::new() } } - pub fn subscribe_system_timer( + pub fn subscribe_monotonic_clock( &mut self, - clock: &'a dyn WasiSystemClock, - deadline: SystemTime, + clock: &'a dyn WasiMonotonicClock, + deadline: Instant, precision: Duration, ud: Userdata, ) { self.subs.push(( - Subscription::SystemTimer(SystemTimerSubscription { + Subscription::MonotonicClock(MonotonicClockSubscription { clock, deadline, precision, @@ -69,15 +69,15 @@ impl<'a> Poll<'a> { pub fn is_empty(&self) -> bool { self.subs.is_empty() } - pub fn earliest_system_timer(&'a self) -> Option<&SystemTimerSubscription<'a>> { + pub fn earliest_clock_deadline(&'a self) -> Option<&MonotonicClockSubscription<'a>> { let mut subs = self .subs .iter() .filter_map(|(s, _ud)| match s { - Subscription::SystemTimer(t) => Some(t), + Subscription::MonotonicClock(t) => Some(t), _ => None, }) - .collect::>>(); + .collect::>>(); subs.sort_by(|a, b| a.deadline.cmp(&b.deadline)); subs.into_iter().next() // First element is earliest } diff --git a/crates/wasi-c2/src/sched/subscription.rs b/crates/wasi-c2/src/sched/subscription.rs index d5eae8950a..7994dfcfc3 100644 --- a/crates/wasi-c2/src/sched/subscription.rs +++ b/crates/wasi-c2/src/sched/subscription.rs @@ -1,8 +1,8 @@ -use crate::clocks::WasiSystemClock; +use crate::clocks::WasiMonotonicClock; use crate::file::WasiFile; use crate::Error; use bitflags::bitflags; -use cap_std::time::{Duration, SystemTime}; +use cap_std::time::{Duration, Instant}; use std::cell::{Cell, Ref}; bitflags! { @@ -34,18 +34,18 @@ impl<'a> RwSubscription<'a> { } } -pub struct SystemTimerSubscription<'a> { - pub clock: &'a dyn WasiSystemClock, - pub deadline: SystemTime, +pub struct MonotonicClockSubscription<'a> { + pub clock: &'a dyn WasiMonotonicClock, + pub deadline: Instant, pub precision: Duration, } -impl<'a> SystemTimerSubscription<'a> { - pub fn now(&self) -> SystemTime { +impl<'a> MonotonicClockSubscription<'a> { + pub fn now(&self) -> Instant { self.clock.now(self.precision) } pub fn result(&self) -> Option> { - if self.now().duration_since(self.deadline).is_ok() { + if self.now().checked_duration_since(self.deadline).is_some() { Some(Ok(())) } else { None @@ -56,13 +56,13 @@ impl<'a> SystemTimerSubscription<'a> { pub enum Subscription<'a> { Read(RwSubscription<'a>), Write(RwSubscription<'a>), - SystemTimer(SystemTimerSubscription<'a>), + MonotonicClock(MonotonicClockSubscription<'a>), } pub enum SubscriptionResult { Read(Result<(u64, RwEventFlags), Error>), Write(Result<(u64, RwEventFlags), Error>), - SystemTimer(Result<(), Error>), + MonotonicClock(Result<(), Error>), } impl SubscriptionResult { @@ -70,7 +70,7 @@ impl SubscriptionResult { match s { Subscription::Read(s) => s.result().map(SubscriptionResult::Read), Subscription::Write(s) => s.result().map(SubscriptionResult::Write), - Subscription::SystemTimer(s) => s.result().map(SubscriptionResult::SystemTimer), + Subscription::MonotonicClock(s) => s.result().map(SubscriptionResult::MonotonicClock), } } } diff --git a/crates/wasi-c2/src/sched/sync.rs b/crates/wasi-c2/src/sched/sync.rs index ae5a791884..db2c65e5e2 100644 --- a/crates/wasi-c2/src/sched/sync.rs +++ b/crates/wasi-c2/src/sched/sync.rs @@ -22,7 +22,7 @@ mod unix { return Ok(()); } let mut pollfds = Vec::new(); - let timeout = poll.earliest_system_timer(); + let timeout = poll.earliest_clock_deadline(); for s in poll.rw_subscriptions() { match s { Subscription::Read(f) => { @@ -34,7 +34,7 @@ mod unix { let raw_fd = wasi_file_raw_fd(f.file.deref()).ok_or(Error::Inval)?; pollfds.push(unsafe { PollFd::new(raw_fd, PollFlags::POLLOUT) }); } - Subscription::SystemTimer { .. } => unreachable!(), + Subscription::MonotonicClock { .. } => unreachable!(), } } @@ -42,7 +42,7 @@ mod unix { let poll_timeout = if let Some(t) = timeout { let duration = t .deadline - .duration_since(t.clock.now(t.precision)) + .checked_duration_since(t.clock.now(t.precision)) .unwrap_or(Duration::from_secs(0)); (duration.as_millis() + 1) // XXX try always rounding up? .try_into() diff --git a/crates/wasi-c2/src/snapshots/preview_1.rs b/crates/wasi-c2/src/snapshots/preview_1.rs index 77c24def51..dbe4d076b0 100644 --- a/crates/wasi-c2/src/snapshots/preview_1.rs +++ b/crates/wasi-c2/src/snapshots/preview_1.rs @@ -895,7 +895,7 @@ impl<'a> wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { return Err(Error::Inval); } - use cap_std::time::{Duration, SystemClock}; + use cap_std::time::Duration; let table = self.table(); let mut poll = Poll::new(); @@ -905,15 +905,16 @@ impl<'a> wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { let sub = sub_ptr.read()?; match sub.u { types::SubscriptionU::Clock(clocksub) => match clocksub.id { - types::Clockid::Realtime => { - let clock = self.clocks.system.deref(); + types::Clockid::Monotonic => { + let clock = self.clocks.monotonic.deref(); let precision = Duration::from_micros(clocksub.precision); let duration = Duration::from_micros(clocksub.timeout); let deadline = if clocksub .flags .contains(types::Subclockflags::SUBSCRIPTION_CLOCK_ABSTIME) { - SystemClock::UNIX_EPOCH + self.clocks + .creation_time .checked_add(duration) .ok_or(Error::Overflow)? } else { @@ -922,7 +923,12 @@ impl<'a> wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { .checked_add(duration) .ok_or(Error::Overflow)? }; - poll.subscribe_system_timer(clock, deadline, precision, sub.userdata.into()) + poll.subscribe_monotonic_clock( + clock, + deadline, + precision, + sub.userdata.into(), + ) } _ => Err(Error::Inval)?, }, @@ -1000,7 +1006,7 @@ impl<'a> wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { }, } } - SubscriptionResult::SystemTimer(r) => { + SubscriptionResult::MonotonicClock(r) => { let type_ = types::Eventtype::Clock; types::Event { userdata,