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.

Sign up for GitHub

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

Fixed an issue where blocking polls were being treated as nonblocking #3741

Merged
merged 1 commit into from
Apr 5, 2023
Merged

Fixed an issue where blocking polls were being treated as nonblocking #3741

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 19 additions & 13 deletions lib/wasi/src/syscalls/wasi/poll_oneoff.rs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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