Skip to content

Commit

Permalink
wasi-threads: factor out process exit logic
Browse files Browse the repository at this point in the history
As is being discussed [elsewhere], either calling `proc_exit` or
trapping in any thread should halt execution of all threads. The
Wasmtime CLI already has logic for adapting a WebAssembly error code to
a code expected in each OS. This change factors out this logic to a new
function, `maybe_exit_on_error`, for use within the `wasi-threads`
implementation.

This will work reasonably well for CLI users of Wasmtime +
`wasi-threads`, but embedders will want something better in the future:
when a `wasi-threads` threads fails, they may not want their application
to exit. Handling this is tricky, because it will require cancelling the
threads spawned by the `wasi-threads` implementation, something that is
not trivial to do in Rust. With this change, we defer that work until
later in order to provide a working implementation of `wasi-threads` for
experimentation.

[elsewhere]: WebAssembly/wasi-threads#17
  • Loading branch information
abrown committed Jan 27, 2023
1 parent f7d603b commit a718c5c
Show file tree
Hide file tree
Showing 7 changed files with 60 additions and 49 deletions.
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ wasmtime-wasi-threads = { workspace = true, optional = true }
clap = { workspace = true, features = ["color", "suggestions", "derive"] }
anyhow = { workspace = true }
target-lexicon = { workspace = true }
libc = "0.2.60"
humantime = "2.0.0"
once_cell = { workspace = true }
listenfd = "1.0.0"
Expand Down Expand Up @@ -69,6 +68,7 @@ wasmtime-component-util = { workspace = true }
component-macro-test = { path = "crates/misc/component-macro-test" }
component-test-util = { workspace = true }
bstr = "0.2.17"
libc = "0.2.60"

[target.'cfg(windows)'.dev-dependencies]
windows-sys = { workspace = true, features = ["Win32_System_Memory"] }
Expand Down
1 change: 1 addition & 0 deletions crates/wasi-threads/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ log = { workspace = true }
rand = "0.8"
wasi-common = { workspace = true }
wasmtime = { workspace = true }
wasmtime-wasi = { workspace = true }

[badges]
maintenance = { status = "experimental" }
16 changes: 6 additions & 10 deletions crates/wasi-threads/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
use anyhow::{anyhow, Result};
use rand::Rng;
use std::thread;
use wasi_common::I32Exit;
use wasmtime::{Caller, Linker, Module, SharedMemory, Store};
use wasmtime_wasi::maybe_exit_on_error;

// This name is a function export designated by the wasi-threads specification:
// https://github.com/WebAssembly/wasi-threads/#detailed-design-discussion
Expand Down Expand Up @@ -63,15 +63,11 @@ impl<T: Clone + Send + 'static> WasiThreadsCtx<T> {
);
match thread_entry_point.call(&mut store, (wasi_thread_id, thread_start_arg)) {
Ok(_) => {}
Err(trap) => {
if let Some(I32Exit(0)) = trap.downcast_ref::<I32Exit>() {
log::trace!(
"exited thread id = {} via `wasi::proc_exit` call",
wasi_thread_id
)
} else {
panic!("{}", fail(trap.to_string()))
}
Err(e) => {
log::trace!("exiting thread id = {} due to error", wasi_thread_id);
let e = maybe_exit_on_error(e);
eprintln!("Error: {:?}", e);
std::process::exit(1);
}
}
})?;
Expand Down
1 change: 1 addition & 0 deletions crates/wasi/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ include = ["src/**/*", "README.md", "LICENSE", "build.rs"]
build = "build.rs"

[dependencies]
libc = "0.2.60"
wasi-common = { workspace = true }
wasi-cap-std-sync = { workspace = true, optional = true }
wasi-tokio = { workspace = true, optional = true }
Expand Down
42 changes: 42 additions & 0 deletions crates/wasi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,3 +82,45 @@ pub mod snapshots {
}
}
}

/// Exit the process with a conventional OS error code as long as Wasmtime
/// understands the error. If the error is not an `I32Exit` or `Trap`, return
/// the error back to the caller for it to decide what to do.
///
/// Note: this function is designed for CLI usage of Wasmtime; embedders of
/// Wasmtime may want different error handling.
pub fn maybe_exit_on_error(e: anyhow::Error) -> anyhow::Error {
use std::process;
use wasmtime::Trap;

// If a specific WASI error code was requested then that's
// forwarded through to the process here without printing any
// extra error information.
if let Some(exit) = e.downcast_ref::<I32Exit>() {
// Print the error message in the usual way.
// On Windows, exit status 3 indicates an abort (see below),
// so return 1 indicating a non-zero status to avoid ambiguity.
if cfg!(windows) && exit.0 >= 3 {
process::exit(1);
}
process::exit(exit.0);
}

// If the program exited because of a trap, return an error code
// to the outside environment indicating a more severe problem
// than a simple failure.
if e.is::<Trap>() {
eprintln!("Error: {:?}", e);

if cfg!(unix) {
// On Unix, return the error code of an abort.
process::exit(128 + libc::SIGABRT);
} else if cfg!(windows) {
// On Windows, return 3.
// https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/abort?view=vs-2019
process::exit(3);
}
}

e
}
45 changes: 7 additions & 38 deletions src/commands/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,15 @@
use anyhow::{anyhow, bail, Context as _, Result};
use clap::Parser;
use once_cell::sync::Lazy;
use std::ffi::OsStr;
use std::path::{Component, Path, PathBuf};
use std::sync::Arc;
use std::thread;
use std::time::Duration;
use std::{
ffi::OsStr,
path::{Component, Path, PathBuf},
process,
};
use wasmtime::{Engine, Func, Linker, Module, Store, Trap, Val, ValType};
use wasmtime::{Engine, Func, Linker, Module, Store, Val, ValType};
use wasmtime_cli_flags::{CommonOptions, WasiModules};
use wasmtime_wasi::maybe_exit_on_error;
use wasmtime_wasi::sync::{ambient_authority, Dir, TcpListener, WasiCtxBuilder};
use wasmtime_wasi::I32Exit;

#[cfg(feature = "wasi-nn")]
use wasmtime_wasi_nn::WasiNnCtx;
Expand Down Expand Up @@ -222,38 +219,10 @@ impl RunCommand {
{
Ok(()) => (),
Err(e) => {
// If a specific WASI error code was requested then that's
// forwarded through to the process here without printing any
// extra error information.
if let Some(exit) = e.downcast_ref::<I32Exit>() {
// Print the error message in the usual way.
// On Windows, exit status 3 indicates an abort (see below),
// so return 1 indicating a non-zero status to avoid ambiguity.
if cfg!(windows) && exit.0 >= 3 {
process::exit(1);
}
process::exit(exit.0);
}

// If the program exited because of a trap, return an error code
// to the outside environment indicating a more severe problem
// than a simple failure.
if e.is::<Trap>() {
eprintln!("Error: {:?}", e);

if cfg!(unix) {
// On Unix, return the error code of an abort.
process::exit(128 + libc::SIGABRT);
} else if cfg!(windows) {
// On Windows, return 3.
// https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/abort?view=vs-2019
process::exit(3);
}
}

// Otherwise fall back on Rust's default error printing/return
// Exit the process if Wasmtime understands the error;
// otherwise, fall back on Rust's default error printing/return
// code.
return Err(e);
return Err(maybe_exit_on_error(e));
}
}

Expand Down

0 comments on commit a718c5c

Please sign in to comment.