Skip to content

Commit

Permalink
Merge pull request #3741 from wasmerio/poll-fixes
Browse files Browse the repository at this point in the history 
Fixed an issue where blocking polls were being treated as nonblocking
  • Loading branch information
@syrusakbary
syrusakbary authored Apr 5, 2023
2 parents eb7282a + e651e32 commit 12dbb78
Show file tree
Hide file tree
Showing 41 changed files with 157 additions and 561 deletions.
32 changes: 19 additions & 13 deletions lib/wasi/src/syscalls/wasi/poll_oneoff.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ pub(crate) fn poll_oneoff_internal(
.filter(|a| a.2.type_ == Eventtype::Clock)
.count();
let mut clock_subs: Vec<(SubscriptionClock, u64)> = Vec::with_capacity(subs.len());
let mut time_to_sleep = None;
let mut time_to_sleep = Duration::ZERO;

// First we extract all the subscriptions into an array so that they
// can be processed
Expand Down Expand Up @@ -204,9 +204,9 @@ pub(crate) fn poll_oneoff_internal(
// If the timeout duration is zero then this is an immediate check rather than
// a sleep itself
if clock_info.timeout == 0 {
time_to_sleep = Some(Duration::ZERO);
time_to_sleep = Duration::MAX;
} else {
time_to_sleep = Some(Duration::from_nanos(clock_info.timeout));
time_to_sleep = Duration::from_nanos(clock_info.timeout);
clock_subs.push((clock_info, s.userdata));
}
continue;
Expand Down Expand Up @@ -276,19 +276,25 @@ pub(crate) fn poll_oneoff_internal(
fd_guards
};

if let Some(time_to_sleep) = time_to_sleep.as_ref() {
if *time_to_sleep == Duration::ZERO {
// If the time is infinite then we omit the time_to_sleep parameter
let asyncify_time = match time_to_sleep {
Duration::ZERO => {
Span::current().record("timeout_ns", "nonblocking");
} else {
Span::current().record("timeout_ns", time_to_sleep.as_millis());
Some(Duration::ZERO)
}
} else {
Span::current().record("timeout_ns", "infinite");
}
Duration::MAX => {
Span::current().record("timeout_ns", "infinite");
None
}
time => {
Span::current().record("timeout_ns", time.as_millis());
Some(time)
}
};

// Block polling the file descriptors
let batch = PollBatch::new(pid, tid, &mut guards);
__asyncify(ctx, time_to_sleep, batch)?
__asyncify(ctx, asyncify_time, batch)?
};

let mut env = ctx.data();
Expand All @@ -303,7 +309,7 @@ pub(crate) fn poll_oneoff_internal(
}
Err(Errno::Timedout) => {
// The timeout has triggerred so lets add that event
if clock_subs.is_empty() && time_to_sleep != Some(Duration::ZERO) {
if clock_subs.is_empty() {
tracing::warn!("triggered_timeout (without any clock subscriptions)",);
}
let mut evts = Vec::new();
Expand All @@ -326,7 +332,7 @@ pub(crate) fn poll_oneoff_internal(
Ok(Ok(evts))
}
// If nonblocking the Errno::Again needs to be turned into an empty list
Err(Errno::Again) if time_to_sleep == Some(Duration::ZERO) => Ok(Ok(Default::default())),
Err(Errno::Again) => Ok(Ok(Default::default())),
// Otherwise process the rror
Err(err) => Ok(Err(err)),
}
Expand Down
60 changes: 58 additions & 2 deletions tests/integration/cli/tests/snapshot.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,11 @@ pub struct TestSpec {
pub use_packages: Vec<String>,
pub include_webcs: Vec<TestIncludeWeb>,
pub cli_args: Vec<String>,
#[serde(skip)]
pub stdin: Option<Vec<u8>>,
#[serde(skip_serializing_if = "Option::is_none")]
#[serde(default)]
pub stdin_hash: Option<String>,
pub debug_output: bool,
pub enable_threads: bool,
pub enable_network: bool,
Expand Down Expand Up @@ -91,6 +95,19 @@ pub struct TestSnapshot {
pub result: TestResult,
}

impl TestSnapshot {
pub fn convert_stdout_to_hash(&mut self) {
self.result = match &self.result {
TestResult::Success(a) => TestResult::Success(TestOutput {
stdout: format!("hash: {:x}", md5::compute(a.stdout.as_bytes())),
stderr: a.stderr.clone(),
exit_code: a.exit_code,
}),
res => res.clone(),
};
}
}

pub struct TestBuilder {
spec: TestSpec,
}
Expand All @@ -107,6 +124,7 @@ impl TestBuilder {
include_webcs: Vec::new(),
cli_args: Vec::new(),
stdin: None,
stdin_hash: None,
debug_output: false,
enable_threads: true,
enable_network: false,
Expand All @@ -125,8 +143,14 @@ impl TestBuilder {
self
}

pub fn stdin_str(mut self, s: impl Into<String>) -> Self {
self.spec.stdin = Some(s.into().into_bytes());
pub fn stdin_str(self, s: impl Into<String>) -> Self {
let str = s.into();
self.stdin(str.as_bytes())
}

pub fn stdin(mut self, s: &[u8]) -> Self {
self.spec.stdin_hash = Some(format!("{:x}", md5::compute(s)));
self.spec.stdin = Some(s.to_vec());
self
}

Expand Down Expand Up @@ -478,6 +502,38 @@ fn test_snapshot_execve() {
assert_json_snapshot!(snapshot);
}

#[cfg(not(any(target_env = "musl", target_os = "macos", target_os = "windows")))]
#[test]
fn test_snapshot_minimodem_tx() {
let mut snapshot = TestBuilder::new()
.with_name(function!())
.stdin_str("This message wont get through")
.arg("--tx")
.arg("--tx-carrier")
.arg("--stdio")
.arg("--float-samples")
.arg("same")
.run_wasm(include_bytes!("./wasm/minimodem.wasm"));
snapshot.convert_stdout_to_hash();

assert_json_snapshot!(snapshot);
}

#[cfg(not(any(target_env = "musl", target_os = "macos", target_os = "windows")))]
#[test]
fn test_snapshot_minimodem_rx() {
let snapshot = TestBuilder::new()
.with_name(function!())
.arg("--rx")
.arg("--tx-carrier")
.arg("--stdio")
.arg("--float-samples")
.arg("same")
.stdin(include_bytes!("./wasm/minimodem.data"))
.run_wasm(include_bytes!("./wasm/minimodem.wasm"));
assert_json_snapshot!(snapshot);
}

#[cfg(not(any(target_env = "musl", target_os = "macos", target_os = "windows")))]
#[test]
fn test_snapshot_web_server() {
Expand Down
35 changes: 1 addition & 34 deletions tests/integration/cli/tests/snapshots/snapshot__snapshot_bash_bash.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,40 +18,7 @@ expression: snapshot
}
],
"cli_args": [],
"stdin": [
47,
98,
105,
110,
47,
98,
97,
115,
104,
10,
101,
99,
104,
111,
32,
104,
105,
10,
101,
120,
105,
116,
10,
101,
99,
104,
111,
32,
104,
105,
50,
10
],
"stdin_hash": "eb83da76ee5fbab28288ecf6385c69db",
"debug_output": false,
"enable_threads": true,
"enable_network": false
Expand Down
31 changes: 1 addition & 30 deletions tests/integration/cli/tests/snapshots/snapshot__snapshot_bash_dash.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,36 +18,7 @@ expression: snapshot
}
],
"cli_args": [],
"stdin": [
47,
98,
105,
110,
47,
100,
97,
115,
104,
10,
101,
99,
104,
111,
32,
104,
105,
10,
101,
120,
105,
116,
10,
101,
120,
105,
116,
10
],
"stdin_hash": "702da725f176c20024965c8830b620ae",
"debug_output": false,
"enable_threads": true,
"enable_network": false
Expand Down
14 changes: 1 addition & 13 deletions tests/integration/cli/tests/snapshots/snapshot__snapshot_bash_echo.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,7 @@ expression: snapshot
"use_packages": [],
"include_webcs": [],
"cli_args": [],
"stdin": [
101,
99,
104,
111,
32,
104,
101,
108,
108,
111,
10
],
"stdin_hash": "664d0430ee33458602e580520841a2d4",
"debug_output": false,
"enable_threads": true,
"enable_network": false
Expand Down
11 changes: 1 addition & 10 deletions tests/integration/cli/tests/snapshots/snapshot__snapshot_bash_ls.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,7 @@ expression: snapshot
}
],
"cli_args": [],
"stdin": [
108,
115,
10,
101,
120,
105,
116,
10
],
"stdin_hash": "ceb5d83a0368a10c813f8e05c42a3a7d",
"debug_output": false,
"enable_threads": true,
"enable_network": false
Expand Down
25 changes: 1 addition & 24 deletions tests/integration/cli/tests/snapshots/snapshot__snapshot_bash_pipe.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,30 +15,7 @@ expression: snapshot
}
],
"cli_args": [],
"stdin": [
101,
99,
104,
111,
32,
104,
101,
108,
108,
111,
32,
124,
32,
99,
97,
116,
10,
101,
120,
105,
116,
10
],
"stdin_hash": "d83523479d9f84e9f94230d5aed27347",
"debug_output": false,
"enable_threads": true,
"enable_network": false
Expand Down
51 changes: 1 addition & 50 deletions tests/integration/cli/tests/snapshots/snapshot__snapshot_bash_python.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,56 +18,7 @@ expression: snapshot
}
],
"cli_args": [],
"stdin": [
119,
97,
115,
109,
101,
114,
32,
114,
117,
110,
32,
115,
121,
114,
117,
115,
97,
107,
98,
97,
114,
121,
47,
112,
121,
116,
104,
111,
110,
32,
45,
45,
32,
45,
99,
32,
39,
112,
114,
105,
110,
116,
40,
49,
48,
41,
39,
10
],
"stdin_hash": "bda2b0db976f1724ef16ae1491211d52",
"debug_output": false,
"enable_threads": true,
"enable_network": false
Expand Down
10 changes: 1 addition & 9 deletions tests/integration/cli/tests/snapshots/snapshot__snapshot_catsay.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,7 @@ expression: snapshot
"use_packages": [],
"include_webcs": [],
"cli_args": [],
"stdin": [
109,
101,
111,
111,
111,
119,
119
],
"stdin_hash": "ed17ce0ab9af325cceb217a26ee9b8a1",
"debug_output": false,
"enable_threads": true,
"enable_network": false
Expand Down
Loading

0 comments on commit 12dbb78

Please sign in to comment.