From c62c9d16baaaf1986747717bc13b69c0be6a9de0 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Mon, 22 Mar 2021 09:11:55 -0700 Subject: [PATCH 1/3] Support "sleep" forms of `poll_oneoff`. Add support for `poll_oneoff` calls which just sleep on a relative timeout. This fixes a bug handling code compiled with WASI libc's `sleep` family of functions, which call `poll_oneoff` with a `CLOCK_REALTIME` timer, which wasn't previously implemented. --- .../wasi-tests/src/bin/poll_oneoff.rs | 41 ++++++++++++++++++- crates/wasi-common/src/snapshots/preview_1.rs | 25 +++++++++++ 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/crates/test-programs/wasi-tests/src/bin/poll_oneoff.rs b/crates/test-programs/wasi-tests/src/bin/poll_oneoff.rs index a680cf630900..b5d7af706644 100644 --- a/crates/test-programs/wasi-tests/src/bin/poll_oneoff.rs +++ b/crates/test-programs/wasi-tests/src/bin/poll_oneoff.rs @@ -26,9 +26,10 @@ unsafe fn test_empty_poll() { } unsafe fn test_timeout() { + let timeout = 5_000_000u64; // 5 milliseconds let clock = wasi::SubscriptionClock { id: wasi::CLOCKID_MONOTONIC, - timeout: 5_000_000u64, // 5 milliseconds + timeout, precision: 0, flags: 0, }; @@ -39,7 +40,9 @@ unsafe fn test_timeout() { u: wasi::SubscriptionUU { clock }, }, }]; + let before = wasi::clock_time_get(wasi::CLOCKID_MONOTONIC, 0).unwrap(); let out = poll_oneoff_impl(&r#in).unwrap(); + let after = wasi::clock_time_get(wasi::CLOCKID_MONOTONIC, 0).unwrap(); assert_eq!(out.len(), 1, "should return 1 event"); let event = &out[0]; assert_errno!(event.error, wasi::ERRNO_SUCCESS); @@ -52,6 +55,42 @@ unsafe fn test_timeout() { event.userdata, CLOCK_ID, "the event.userdata should contain clock_id specified by the user" ); + assert!(after - before >= timeout, "poll_oneoff should sleep for the specified interval"); +} + +// Like test_timeout, but uses `CLOCKID_REALTIME`, as WASI libc's sleep +// functions do. +unsafe fn test_sleep() { + let timeout = 5_000_000u64; // 5 milliseconds + let clock = wasi::SubscriptionClock { + id: wasi::CLOCKID_REALTIME, + timeout, + precision: 0, + flags: 0, + }; + let r#in = [wasi::Subscription { + userdata: CLOCK_ID, + u: wasi::SubscriptionU { + tag: wasi::EVENTTYPE_CLOCK, + u: wasi::SubscriptionUU { clock }, + }, + }]; + let before = wasi::clock_time_get(wasi::CLOCKID_MONOTONIC, 0).unwrap(); + let out = poll_oneoff_impl(&r#in).unwrap(); + let after = wasi::clock_time_get(wasi::CLOCKID_MONOTONIC, 0).unwrap(); + assert_eq!(out.len(), 1, "should return 1 event"); + let event = &out[0]; + assert_errno!(event.error, wasi::ERRNO_SUCCESS); + assert_eq!( + event.r#type, + wasi::EVENTTYPE_CLOCK, + "the event.type should equal clock" + ); + assert_eq!( + event.userdata, CLOCK_ID, + "the event.userdata should contain clock_id specified by the user" + ); + assert!(after - before >= timeout, "poll_oneoff should sleep for the specified interval"); } unsafe fn test_fd_readwrite(fd: wasi::Fd, error_code: wasi::Errno) { diff --git a/crates/wasi-common/src/snapshots/preview_1.rs b/crates/wasi-common/src/snapshots/preview_1.rs index d5ec0a586a7f..70d422c947f0 100644 --- a/crates/wasi-common/src/snapshots/preview_1.rs +++ b/crates/wasi-common/src/snapshots/preview_1.rs @@ -16,6 +16,7 @@ use std::cell::{Ref, RefMut}; use std::convert::{TryFrom, TryInto}; use std::io::{IoSlice, IoSliceMut}; use std::ops::{Deref, DerefMut}; +use std::thread; use tracing::debug; use wiggle::GuestPtr; @@ -909,6 +910,30 @@ impl<'a> wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { return Err(Error::invalid_argument().context("nsubscriptions must be nonzero")); } + // Special-case a `poll_oneoff` which is just sleeping on a single + // relative timer event, such as what WASI libc uses to implement sleep + // functions. This supports all clock IDs, because POSIX says that + // `clock_settime` doesn't effect relative sleeps. + if nsubscriptions == 1 { + let sub = subs.read()?; + if let types::SubscriptionU::Clock(clocksub) = sub.u { + if !clocksub + .flags + .contains(types::Subclockflags::SUBSCRIPTION_CLOCK_ABSTIME) + { + thread::sleep(Duration::from_nanos(clocksub.timeout)); + + events.write(types::Event { + userdata: sub.userdata, + error: types::Errno::Success, + type_: types::Eventtype::Clock, + fd_readwrite: fd_readwrite_empty(), + })?; + return Ok(1); + } + } + } + let table = self.table(); let mut poll = Poll::new(); From e4d6d31274e8f0314042708925aca06cd707a60a Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Mon, 22 Mar 2021 10:44:42 -0700 Subject: [PATCH 2/3] Arrange for the new test to be called. --- crates/test-programs/wasi-tests/src/bin/poll_oneoff.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/test-programs/wasi-tests/src/bin/poll_oneoff.rs b/crates/test-programs/wasi-tests/src/bin/poll_oneoff.rs index b5d7af706644..6407a32f3cb0 100644 --- a/crates/test-programs/wasi-tests/src/bin/poll_oneoff.rs +++ b/crates/test-programs/wasi-tests/src/bin/poll_oneoff.rs @@ -195,6 +195,7 @@ unsafe fn test_fd_readwrite_invalid_fd() { unsafe fn test_poll_oneoff(dir_fd: wasi::Fd) { test_timeout(); + test_sleep(); test_empty_poll(); test_fd_readwrite_valid_fd(dir_fd); test_fd_readwrite_invalid_fd(); From 4ecb3677a37e29acd455ec84e9cd993fa1229632 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Mon, 22 Mar 2021 11:23:45 -0700 Subject: [PATCH 3/3] Add a `sleep` function to the `WasiSched` trait. --- .../cap-std-sync/src/sched/unix.rs | 3 +++ .../cap-std-sync/src/sched/windows.rs | 3 +++ crates/wasi-common/src/sched.rs | 1 + crates/wasi-common/src/snapshots/preview_0.rs | 24 +++++++++++++++++++ crates/wasi-common/src/snapshots/preview_1.rs | 3 +-- 5 files changed, 32 insertions(+), 2 deletions(-) diff --git a/crates/wasi-common/cap-std-sync/src/sched/unix.rs b/crates/wasi-common/cap-std-sync/src/sched/unix.rs index 15bbb2913882..ab8be60a0a41 100644 --- a/crates/wasi-common/cap-std-sync/src/sched/unix.rs +++ b/crates/wasi-common/cap-std-sync/src/sched/unix.rs @@ -108,6 +108,9 @@ impl WasiSched for SyncSched { std::thread::yield_now(); Ok(()) } + fn sleep(&self, duration: Duration) { + std::thread::sleep(duration) + } } fn wasi_file_raw_fd(f: &dyn WasiFile) -> Option { diff --git a/crates/wasi-common/cap-std-sync/src/sched/windows.rs b/crates/wasi-common/cap-std-sync/src/sched/windows.rs index 03eb41514ceb..93545c404019 100644 --- a/crates/wasi-common/cap-std-sync/src/sched/windows.rs +++ b/crates/wasi-common/cap-std-sync/src/sched/windows.rs @@ -128,6 +128,9 @@ impl WasiSched for SyncSched { thread::yield_now(); Ok(()) } + fn sleep(&self, duration: Duration) { + std::thread::sleep(duration) + } } fn wasi_file_raw_handle(f: &dyn WasiFile) -> Option { diff --git a/crates/wasi-common/src/sched.rs b/crates/wasi-common/src/sched.rs index db2a81f9dbc1..381bb61449a2 100644 --- a/crates/wasi-common/src/sched.rs +++ b/crates/wasi-common/src/sched.rs @@ -10,6 +10,7 @@ use subscription::{MonotonicClockSubscription, RwSubscription, Subscription, Sub pub trait WasiSched { fn poll_oneoff(&self, poll: &Poll) -> Result<(), Error>; fn sched_yield(&self) -> Result<(), Error>; + fn sleep(&self, duration: Duration); } pub struct Userdata(u64); diff --git a/crates/wasi-common/src/snapshots/preview_0.rs b/crates/wasi-common/src/snapshots/preview_0.rs index 65f4823a3e60..d027997122f7 100644 --- a/crates/wasi-common/src/snapshots/preview_0.rs +++ b/crates/wasi-common/src/snapshots/preview_0.rs @@ -738,6 +738,30 @@ impl<'a> wasi_unstable::WasiUnstable for WasiCtx { return Err(Error::invalid_argument().context("nsubscriptions must be nonzero")); } + // Special-case a `poll_oneoff` which is just sleeping on a single + // relative timer event, such as what WASI libc uses to implement sleep + // functions. This supports all clock IDs, because POSIX says that + // `clock_settime` doesn't effect relative sleeps. + if nsubscriptions == 1 { + let sub = subs.read()?; + if let types::SubscriptionU::Clock(clocksub) = sub.u { + if !clocksub + .flags + .contains(types::Subclockflags::SUBSCRIPTION_CLOCK_ABSTIME) + { + self.sched.sleep(Duration::from_nanos(clocksub.timeout)); + + events.write(types::Event { + userdata: sub.userdata, + error: types::Errno::Success, + type_: types::Eventtype::Clock, + fd_readwrite: fd_readwrite_empty(), + })?; + return Ok(1); + } + } + } + let table = self.table(); let mut poll = Poll::new(); diff --git a/crates/wasi-common/src/snapshots/preview_1.rs b/crates/wasi-common/src/snapshots/preview_1.rs index 70d422c947f0..ca91b84cc7b1 100644 --- a/crates/wasi-common/src/snapshots/preview_1.rs +++ b/crates/wasi-common/src/snapshots/preview_1.rs @@ -16,7 +16,6 @@ use std::cell::{Ref, RefMut}; use std::convert::{TryFrom, TryInto}; use std::io::{IoSlice, IoSliceMut}; use std::ops::{Deref, DerefMut}; -use std::thread; use tracing::debug; use wiggle::GuestPtr; @@ -921,7 +920,7 @@ impl<'a> wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { .flags .contains(types::Subclockflags::SUBSCRIPTION_CLOCK_ABSTIME) { - thread::sleep(Duration::from_nanos(clocksub.timeout)); + self.sched.sleep(Duration::from_nanos(clocksub.timeout)); events.write(types::Event { userdata: sub.userdata,