From 8da9f702da9ef45dceec8ae3ea4305459023ee66 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Tue, 24 Oct 2023 17:33:00 -0700 Subject: [PATCH 1/8] monotonic clock: introduce `duration` type, split `subscribe` We are introducing a `duration` type because it has a distinct meaning from `instant`: an `instant` can only be compared to other `instant`s from the exact same `monotonic-clock`, whereas a `duration` represents a duration of time which can be compared to any other duration of time. The `duration` type is motivated, in part, by a desire to reuse it to specify durations such as timeouts in other WASI proposals. Instead of taking a boolean specifying whether the u64 is an absolute or relative time, `subscribe-instant` takes an `instant` type and `subscribe-duration` takes a `duration` type. --- .../wasi/wit/deps/clocks/monotonic-clock.wit | 26 ++++++++++++++----- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/crates/wasi/wit/deps/clocks/monotonic-clock.wit b/crates/wasi/wit/deps/clocks/monotonic-clock.wit index a3f7fe25c65b..df3758b8a8a6 100644 --- a/crates/wasi/wit/deps/clocks/monotonic-clock.wit +++ b/crates/wasi/wit/deps/clocks/monotonic-clock.wit @@ -11,22 +11,34 @@ interface monotonic-clock { use wasi:io/poll@0.2.0-rc-2023-11-05.{pollable}; - /// A timestamp in nanoseconds. + /// An instant in time, in nanoseconds. An instant is relative to an + /// unspecified initial value, and can only be compared to instances from + /// the same monotonic-clock. type instant = u64; + /// A duration of time, in nanoseconds. + type duration = u64; + /// Read the current value of the clock. /// /// The clock is monotonic, therefore calling this function repeatedly will /// produce a sequence of non-decreasing values. now: func() -> instant; - /// Query the resolution of the clock. - resolution: func() -> instant; + /// Query the resolution of the clock. Returns the duration of time + /// corresponding to a clock tick. + resolution: func() -> duration; - /// Create a `pollable` which will resolve once the specified time has been - /// reached. - subscribe: func( + /// Create a `pollable` which will resolve once the specified instant + /// occured. + subscribe-instant: func( when: instant, - absolute: bool + ) -> pollable; + + /// Create a `pollable` which will resolve once the given duration has + /// elapsed, starting at the time at which this function was called. + /// occured. + subscribe-duration: func( + when: duration, ) -> pollable; } From 5a70f9f0252502d7a25ad005767474982e785515 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Tue, 24 Oct 2023 17:52:23 -0700 Subject: [PATCH 2/8] wasmtime-wasi: implement changes to monotonic clock interface --- crates/wasi/src/preview2/host/clocks.rs | 51 +++++++++++++++++++------ crates/wasi/src/preview2/preview1.rs | 12 ++++-- 2 files changed, 48 insertions(+), 15 deletions(-) diff --git a/crates/wasi/src/preview2/host/clocks.rs b/crates/wasi/src/preview2/host/clocks.rs index 1a07013d8ab6..7e2e5583a54e 100644 --- a/crates/wasi/src/preview2/host/clocks.rs +++ b/crates/wasi/src/preview2/host/clocks.rs @@ -1,7 +1,7 @@ #![allow(unused_variables)] use crate::preview2::bindings::{ - clocks::monotonic_clock::{self, Instant}, + clocks::monotonic_clock::{self, Duration as WasiDuration, Instant}, clocks::timezone::{self, TimezoneDisplay}, clocks::wall_clock::{self, Datetime}, }; @@ -52,30 +52,57 @@ impl monotonic_clock::Host for T { Ok(self.ctx().monotonic_clock.resolution()) } - fn subscribe(&mut self, when: Instant, absolute: bool) -> anyhow::Result> { + fn subscribe_instant(&mut self, when: Instant) -> anyhow::Result> { let clock_now = self.ctx().monotonic_clock.now(); - let duration = if absolute { - Duration::from_nanos(when - clock_now) + let deadline = if when <= clock_now { + let duration = Duration::from_nanos(when - clock_now); + tokio::time::Instant::now() + .checked_add(duration) + // This should only reachable if we are running on a platform + // where std::time::Instant (of which tokio's Instant is a + // newtype wrapper) represents less than a u64 in nanoseconds, + // which appears to always be true in practice, but isnt a + // guarantee provided by std. + .ok_or_else(|| anyhow::anyhow!("time overflow: duration {duration:?}"))? } else { - Duration::from_nanos(when) + tokio::time::Instant::now() }; - let deadline = tokio::time::Instant::now() - .checked_add(duration) - .ok_or_else(|| anyhow::anyhow!("time overflow: duration {duration:?}"))?; // NB: this resource created here is not actually exposed to wasm, it's // only an internal implementation detail used to match the signature // expected by `subscribe`. - let sleep = self.table_mut().push(Sleep(deadline))?; + let sleep = self.table_mut().push(Deadline::Instant(deadline))?; + subscribe(self.table_mut(), sleep) + } + + fn subscribe_duration(&mut self, duration: WasiDuration) -> anyhow::Result> { + let clock_now = self.ctx().monotonic_clock.now(); + let duration = Duration::from_nanos(duration); + let sleep = if let Some(deadline) = tokio::time::Instant::now().checked_add(duration) { + // NB: this resource created here is not actually exposed to wasm, it's + // only an internal implementation detail used to match the signature + // expected by `subscribe`. + self.table_mut().push(Deadline::Instant(deadline))? + } else { + // If the user specifies a time so far in the future we can't + // represent it, wait forever rather than trap. + self.table_mut().push(Deadline::Never)? + }; subscribe(self.table_mut(), sleep) } } -struct Sleep(tokio::time::Instant); +enum Deadline { + Instant(tokio::time::Instant), + Never, +} #[async_trait::async_trait] -impl Subscribe for Sleep { +impl Subscribe for Deadline { async fn ready(&mut self) { - tokio::time::sleep_until(self.0).await; + match self { + Deadline::Instant(instant) => tokio::time::sleep_until(*instant).await, + Deadline::Never => std::future::pending().await, + } } } diff --git a/crates/wasi/src/preview2/preview1.rs b/crates/wasi/src/preview2/preview1.rs index 282f50ae75f6..b92ee31e94bf 100644 --- a/crates/wasi/src/preview2/preview1.rs +++ b/crates/wasi/src/preview2/preview1.rs @@ -2069,9 +2069,15 @@ impl< } _ => return Err(types::Errno::Inval.into()), }; - monotonic_clock::Host::subscribe(self, timeout, absolute) - .context("failed to call `monotonic_clock::subscribe`") - .map_err(types::Error::trap)? + if absolute { + monotonic_clock::Host::subscribe_instant(self, timeout) + .context("failed to call `monotonic_clock::subscribe_instant`") + .map_err(types::Error::trap)? + } else { + monotonic_clock::Host::subscribe_duration(self, timeout) + .context("failed to call `monotonic_clock::subscribe_duration`") + .map_err(types::Error::trap)? + } } types::SubscriptionU::FdRead(types::SubscriptionFdReadwrite { file_descriptor, From bfb86ad9c157929ff716ddee0cb8ce5e269cc783 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Tue, 24 Oct 2023 17:53:52 -0700 Subject: [PATCH 3/8] adapter: fixes for subscribe duration and instant --- crates/wasi-preview1-component-adapter/src/lib.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/crates/wasi-preview1-component-adapter/src/lib.rs b/crates/wasi-preview1-component-adapter/src/lib.rs index 08978baa51a3..66a697d9b79c 100644 --- a/crates/wasi-preview1-component-adapter/src/lib.rs +++ b/crates/wasi-preview1-component-adapter/src/lib.rs @@ -1748,13 +1748,10 @@ pub unsafe extern "C" fn poll_oneoff( clock.timeout }; - monotonic_clock::subscribe(timeout, false) + monotonic_clock::subscribe_duration(timeout) } - CLOCKID_MONOTONIC => { - let s = monotonic_clock::subscribe(clock.timeout, absolute); - s - } + CLOCKID_MONOTONIC => monotonic_clock::subscribe_instant(clock.timeout), _ => return Err(ERRNO_INVAL), } From fd3b7d5be5d5629d207bc74b2bc20ac1e9010fa4 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Fri, 27 Oct 2023 15:23:56 -0700 Subject: [PATCH 4/8] factor subscribe_to_duration out into a helper function. --- crates/wasi/src/preview2/host/clocks.rs | 51 +++++++++++-------------- 1 file changed, 22 insertions(+), 29 deletions(-) diff --git a/crates/wasi/src/preview2/host/clocks.rs b/crates/wasi/src/preview2/host/clocks.rs index 7e2e5583a54e..f288701a7ec7 100644 --- a/crates/wasi/src/preview2/host/clocks.rs +++ b/crates/wasi/src/preview2/host/clocks.rs @@ -43,6 +43,23 @@ impl wall_clock::Host for T { } } +fn subscribe_to_duration( + table: &mut crate::preview2::Table, + duration: tokio::time::Duration, +) -> anyhow::Result> { + let sleep = if let Some(deadline) = tokio::time::Instant::now().checked_add(duration) { + // NB: this resource created here is not actually exposed to wasm, it's + // only an internal implementation detail used to match the signature + // expected by `subscribe`. + table.push(Deadline::Instant(deadline))? + } else { + // If the user specifies a time so far in the future we can't + // represent it, wait forever rather than trap. + table.push(Deadline::Never)? + }; + subscribe(table, sleep) +} + impl monotonic_clock::Host for T { fn now(&mut self) -> anyhow::Result { Ok(self.ctx().monotonic_clock.now()) @@ -54,40 +71,16 @@ impl monotonic_clock::Host for T { fn subscribe_instant(&mut self, when: Instant) -> anyhow::Result> { let clock_now = self.ctx().monotonic_clock.now(); - let deadline = if when <= clock_now { - let duration = Duration::from_nanos(when - clock_now); - tokio::time::Instant::now() - .checked_add(duration) - // This should only reachable if we are running on a platform - // where std::time::Instant (of which tokio's Instant is a - // newtype wrapper) represents less than a u64 in nanoseconds, - // which appears to always be true in practice, but isnt a - // guarantee provided by std. - .ok_or_else(|| anyhow::anyhow!("time overflow: duration {duration:?}"))? + let duration = if when <= clock_now { + Duration::from_nanos(when - clock_now) } else { - tokio::time::Instant::now() + Duration::from_nanos(0) }; - // NB: this resource created here is not actually exposed to wasm, it's - // only an internal implementation detail used to match the signature - // expected by `subscribe`. - let sleep = self.table_mut().push(Deadline::Instant(deadline))?; - subscribe(self.table_mut(), sleep) + subscribe_to_duration(&mut self.table_mut(), duration) } fn subscribe_duration(&mut self, duration: WasiDuration) -> anyhow::Result> { - let clock_now = self.ctx().monotonic_clock.now(); - let duration = Duration::from_nanos(duration); - let sleep = if let Some(deadline) = tokio::time::Instant::now().checked_add(duration) { - // NB: this resource created here is not actually exposed to wasm, it's - // only an internal implementation detail used to match the signature - // expected by `subscribe`. - self.table_mut().push(Deadline::Instant(deadline))? - } else { - // If the user specifies a time so far in the future we can't - // represent it, wait forever rather than trap. - self.table_mut().push(Deadline::Never)? - }; - subscribe(self.table_mut(), sleep) + subscribe_to_duration(&mut self.table_mut(), Duration::from_nanos(duration)) } } From 0a3ac7a38be1c5704f39abdd93e8750181fc44d4 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Fri, 27 Oct 2023 15:30:22 -0700 Subject: [PATCH 5/8] sync wits into wasi-http --- .../wit/deps/clocks/monotonic-clock.wit | 26 ++++++++++++++----- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/crates/wasi-http/wit/deps/clocks/monotonic-clock.wit b/crates/wasi-http/wit/deps/clocks/monotonic-clock.wit index a3f7fe25c65b..df3758b8a8a6 100644 --- a/crates/wasi-http/wit/deps/clocks/monotonic-clock.wit +++ b/crates/wasi-http/wit/deps/clocks/monotonic-clock.wit @@ -11,22 +11,34 @@ interface monotonic-clock { use wasi:io/poll@0.2.0-rc-2023-11-05.{pollable}; - /// A timestamp in nanoseconds. + /// An instant in time, in nanoseconds. An instant is relative to an + /// unspecified initial value, and can only be compared to instances from + /// the same monotonic-clock. type instant = u64; + /// A duration of time, in nanoseconds. + type duration = u64; + /// Read the current value of the clock. /// /// The clock is monotonic, therefore calling this function repeatedly will /// produce a sequence of non-decreasing values. now: func() -> instant; - /// Query the resolution of the clock. - resolution: func() -> instant; + /// Query the resolution of the clock. Returns the duration of time + /// corresponding to a clock tick. + resolution: func() -> duration; - /// Create a `pollable` which will resolve once the specified time has been - /// reached. - subscribe: func( + /// Create a `pollable` which will resolve once the specified instant + /// occured. + subscribe-instant: func( when: instant, - absolute: bool + ) -> pollable; + + /// Create a `pollable` which will resolve once the given duration has + /// elapsed, starting at the time at which this function was called. + /// occured. + subscribe-duration: func( + when: duration, ) -> pollable; } From ee3f480edb93e59c0fa40f7e0720048c2897fc9f Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Mon, 30 Oct 2023 14:22:51 -0700 Subject: [PATCH 6/8] component adapter: fix for interpreting flag for monotonic instant vs duration --- crates/wasi-preview1-component-adapter/src/lib.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/crates/wasi-preview1-component-adapter/src/lib.rs b/crates/wasi-preview1-component-adapter/src/lib.rs index 66a697d9b79c..267a3ac6570f 100644 --- a/crates/wasi-preview1-component-adapter/src/lib.rs +++ b/crates/wasi-preview1-component-adapter/src/lib.rs @@ -1751,7 +1751,13 @@ pub unsafe extern "C" fn poll_oneoff( monotonic_clock::subscribe_duration(timeout) } - CLOCKID_MONOTONIC => monotonic_clock::subscribe_instant(clock.timeout), + CLOCKID_MONOTONIC => { + if absolute { + monotonic_clock::subscribe_instant(clock.timeout) + } else { + monotonic_clock::subscribe_duration(clock.timeout) + } + } _ => return Err(ERRNO_INVAL), } From 4b4c3a738612dc34c0d577bb32a81488f834570c Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Mon, 30 Oct 2023 14:23:49 -0700 Subject: [PATCH 7/8] subscribe_instant impl: fix logic for when time is in the past caught by subtract overflow panic, thankfully --- crates/wasi/src/preview2/host/clocks.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/wasi/src/preview2/host/clocks.rs b/crates/wasi/src/preview2/host/clocks.rs index f288701a7ec7..52942f365a12 100644 --- a/crates/wasi/src/preview2/host/clocks.rs +++ b/crates/wasi/src/preview2/host/clocks.rs @@ -71,7 +71,7 @@ impl monotonic_clock::Host for T { fn subscribe_instant(&mut self, when: Instant) -> anyhow::Result> { let clock_now = self.ctx().monotonic_clock.now(); - let duration = if when <= clock_now { + let duration = if when > clock_now { Duration::from_nanos(when - clock_now) } else { Duration::from_nanos(0) From 0c05fb759a15c0c6c3cb1f57122249b956054c58 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Mon, 30 Oct 2023 14:24:43 -0700 Subject: [PATCH 8/8] test-programs: compile against new subscribe instant, duration methods --- .../src/bin/preview1_poll_oneoff_files.rs | 4 ++-- .../test-programs/src/bin/preview2_ip_name_lookup.rs | 2 +- crates/test-programs/src/bin/preview2_sleep.rs | 10 +++++----- crates/test-programs/src/sockets.rs | 4 ++-- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/crates/test-programs/src/bin/preview1_poll_oneoff_files.rs b/crates/test-programs/src/bin/preview1_poll_oneoff_files.rs index d1dfccaef7da..57778f88b013 100644 --- a/crates/test-programs/src/bin/preview1_poll_oneoff_files.rs +++ b/crates/test-programs/src/bin/preview1_poll_oneoff_files.rs @@ -84,7 +84,7 @@ unsafe fn test_timeout() { ); assert!( after - before >= timeout, - "poll_oneoff should sleep for the specified interval" + "poll_oneoff should sleep for the specified interval of {timeout}. before: {before}, after: {after}" ); } @@ -122,7 +122,7 @@ unsafe fn test_sleep() { ); assert!( after - before >= timeout, - "poll_oneoff should sleep for the specified interval" + "poll_oneoff should sleep for the specified interval of {timeout}. before: {before}, after: {after}" ); } diff --git a/crates/test-programs/src/bin/preview2_ip_name_lookup.rs b/crates/test-programs/src/bin/preview2_ip_name_lookup.rs index 219ebba459f2..fc0b25de85a4 100644 --- a/crates/test-programs/src/bin/preview2_ip_name_lookup.rs +++ b/crates/test-programs/src/bin/preview2_ip_name_lookup.rs @@ -19,7 +19,7 @@ fn main() { // the resolution and allows errors. let addresses = ip_name_lookup::resolve_addresses(&network, "github.com", None, false).unwrap(); let lookup = addresses.subscribe(); - let timeout = monotonic_clock::subscribe(1_000_000_000, false); + let timeout = monotonic_clock::subscribe_duration(1_000_000_000); let ready = poll::poll_list(&[&lookup, &timeout]); assert!(ready.len() > 0); match ready[0] { diff --git a/crates/test-programs/src/bin/preview2_sleep.rs b/crates/test-programs/src/bin/preview2_sleep.rs index a5f4d76a3f5c..15de238dc8cf 100644 --- a/crates/test-programs/src/bin/preview2_sleep.rs +++ b/crates/test-programs/src/bin/preview2_sleep.rs @@ -8,20 +8,20 @@ fn main() { fn sleep_10ms() { let dur = 10_000_000; - let p = monotonic_clock::subscribe(monotonic_clock::now() + dur, true); + let p = monotonic_clock::subscribe_instant(monotonic_clock::now() + dur); poll::poll_one(&p); - let p = monotonic_clock::subscribe(dur, false); + let p = monotonic_clock::subscribe_duration(dur); poll::poll_one(&p); } fn sleep_0ms() { - let p = monotonic_clock::subscribe(monotonic_clock::now(), true); + let p = monotonic_clock::subscribe_instant(monotonic_clock::now()); poll::poll_one(&p); - let p = monotonic_clock::subscribe(0, false); + let p = monotonic_clock::subscribe_duration(0); poll::poll_one(&p); } fn sleep_backwards_in_time() { - let p = monotonic_clock::subscribe(monotonic_clock::now() - 1, true); + let p = monotonic_clock::subscribe_instant(monotonic_clock::now() - 1); poll::poll_one(&p); } diff --git a/crates/test-programs/src/sockets.rs b/crates/test-programs/src/sockets.rs index 3033582b6066..74a65d74fa54 100644 --- a/crates/test-programs/src/sockets.rs +++ b/crates/test-programs/src/sockets.rs @@ -159,7 +159,7 @@ impl OutgoingDatagramStream { } pub fn blocking_send(&self, mut datagrams: &[OutgoingDatagram]) -> Result<(), ErrorCode> { - let timeout = monotonic_clock::subscribe(TIMEOUT_NS, false); + let timeout = monotonic_clock::subscribe_duration(TIMEOUT_NS); while !datagrams.is_empty() { let permit = self.blocking_check_send(&timeout)?; @@ -180,7 +180,7 @@ impl OutgoingDatagramStream { impl IncomingDatagramStream { pub fn blocking_receive(&self, count: Range) -> Result, ErrorCode> { - let timeout = monotonic_clock::subscribe(TIMEOUT_NS, false); + let timeout = monotonic_clock::subscribe_duration(TIMEOUT_NS); let pollable = self.subscribe(); let mut datagrams = vec![];