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.

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

Terminate entire wasix process when a worker thread fails or calls exit #5153

Merged
merged 4 commits into from
Oct 24, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
1 change: 1 addition & 0 deletions lib/wasix/src/bin_factory/exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,7 @@ fn call_module(
if let Err(err) = call_ret {
match err.downcast::<WasiError>() {
Ok(WasiError::Exit(code)) if code.is_success() => Ok(Errno::Success),
Ok(WasiError::ThreadExit) => Ok(Errno::Success),
Ok(WasiError::Exit(code)) => {
runtime.on_taint(TaintReason::NonZeroExitCode(code));
Err(WasiError::Exit(code).into())
Expand Down
2 changes: 2 additions & 0 deletions lib/wasix/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,8 @@ pub use crate::{
pub enum WasiError {
#[error("WASI exited with code: {0}")]
Exit(ExitCode),
#[error("WASI thread exited")]
ThreadExit,
#[error("WASI deep sleep: {0:?}")]
DeepSleep(DeepSleepWork),
#[error("The WASI version could not be determined")]
Expand Down
3 changes: 3 additions & 0 deletions lib/wasix/src/runners/wasi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,9 @@ impl crate::runners::Runner for WasiRunner {
WasiRuntimeError::Wasi(WasiError::Exit(a)) => {
WasiRuntimeError::Wasi(WasiError::Exit(*a))
}
WasiRuntimeError::Wasi(WasiError::ThreadExit) => {
WasiRuntimeError::Wasi(WasiError::ThreadExit)
}
WasiRuntimeError::Wasi(WasiError::UnknownWasiVersion) => {
WasiRuntimeError::Wasi(WasiError::UnknownWasiVersion)
}
Expand Down
18 changes: 9 additions & 9 deletions lib/wasix/src/state/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1252,32 +1252,33 @@ impl WasiEnv {

/// Cleans up all the open files (if this is the main thread)
#[allow(clippy::await_holding_lock)]
pub fn blocking_on_exit(&self, exit_code: Option<ExitCode>) {
let cleanup = self.on_exit(exit_code);
pub fn blocking_on_exit(&self, process_exit_code: Option<ExitCode>) {
let cleanup = self.on_exit(process_exit_code);
InlineWaker::block_on(cleanup);
}

/// Cleans up all the open files (if this is the main thread)
#[allow(clippy::await_holding_lock)]
pub fn on_exit(&self, exit_code: Option<ExitCode>) -> BoxFuture<'static, ()> {
pub fn on_exit(&self, process_exit_code: Option<ExitCode>) -> BoxFuture<'static, ()> {
const CLEANUP_TIMEOUT: Duration = Duration::from_secs(10);

// If snap-shooting is enabled then we should record an event that the thread has exited.
#[cfg(feature = "journal")]
if self.should_journal() && self.has_active_journal() {
if let Err(err) = JournalEffector::save_thread_exit(self, self.tid(), exit_code) {
if let Err(err) = JournalEffector::save_thread_exit(self, self.tid(), process_exit_code)
{
tracing::warn!("failed to save snapshot event for thread exit - {}", err);
}

if self.thread.is_main() {
if let Err(err) = JournalEffector::save_process_exit(self, exit_code) {
if let Err(err) = JournalEffector::save_process_exit(self, process_exit_code) {
tracing::warn!("failed to save snapshot event for process exit - {}", err);
}
}
}

// If this is the main thread then also close all the files
if self.thread.is_main() {
// If the process wants to exit, also close all files and terminate it
if let Some(process_exit_code) = process_exit_code {
let process = self.process.clone();
let disable_fs_cleanup = self.disable_fs_cleanup;
let pid = self.pid();
Expand All @@ -1303,8 +1304,7 @@ impl WasiEnv {
}

// Terminate the process
let exit_code = exit_code.unwrap_or_else(|| Errno::Canceled.into());
process.terminate(exit_code);
process.terminate(process_exit_code);
})
} else {
Box::pin(async {})
Expand Down
4 changes: 2 additions & 2 deletions lib/wasix/src/state/func_env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -284,15 +284,15 @@ impl WasiFunctionEnv {
/// This function should only be called from within a syscall
/// as it can potentially execute local thread variable cleanup
/// code
pub fn on_exit(&self, store: &mut impl AsStoreMut, exit_code: Option<ExitCode>) {
pub fn on_exit(&self, store: &mut impl AsStoreMut, process_exit_code: Option<ExitCode>) {
trace!(
"wasi[{}:{}]::on_exit",
self.data(store).pid(),
self.data(store).tid()
);

// Cleans up all the open files (if this is the main thread)
self.data(store).blocking_on_exit(exit_code);
self.data(store).blocking_on_exit(process_exit_code);
}

/// Bootstraps this main thread and context with any journals that
Expand Down
19 changes: 9 additions & 10 deletions lib/wasix/src/syscalls/wasix/thread_exit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,14 @@ use crate::syscalls::*;

/// ### `thread_exit()`
/// Terminates the current running thread, if this is the last thread then
/// the process will also exit with the specified exit code. An exit code
/// of 0 indicates successful termination of the thread. The meanings of
/// other values is dependent on the environment.
/// the process will also exit with code 0.
/// The exit code parameter is a left over from a previous version of this
/// syscall, maintained here to keep the syscall backwards-compatible, but
/// is otherwise unused.
///
/// ## Parameters
///
/// * `rval` - The exit code returned by the process.
#[instrument(level = "trace", skip_all, fields(%exitcode), ret)]
pub fn thread_exit(ctx: FunctionEnvMut<'_, WasiEnv>, exitcode: ExitCode) -> Result<(), WasiError> {
tracing::debug!(tid=%ctx.data().thread.id(), %exitcode);
Err(WasiError::Exit(exitcode))
/// This syscall does not return.
#[instrument(level = "trace", skip_all, fields(%_exitcode), ret)]
pub fn thread_exit(ctx: FunctionEnvMut<'_, WasiEnv>, _exitcode: ExitCode) -> Result<(), WasiError> {
tracing::debug!(tid=%ctx.data().thread.id(), "thread exit");
Err(WasiError::ThreadExit)
}
16 changes: 14 additions & 2 deletions lib/wasix/src/syscalls/wasix/thread_spawn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,10 +203,19 @@ fn call_module<M: MemorySize>(
.map_err(|_| Errno::Overflow)
.unwrap(),
);
trace!("callback finished (ret={:?})", call_ret);

let mut ret = Errno::Success;
let mut exit_code = None;
if let Err(err) = call_ret {
match err.downcast::<WasiError>() {
Ok(WasiError::ThreadExit) => {
trace!("thread exited cleanly");
ret = Errno::Success;
}
Ok(WasiError::Exit(code)) => {
trace!(exit_code = ?code, "thread requested exit");
exit_code = Some(code);
ret = if code.is_success() {
Errno::Success
} else {
Expand All @@ -226,20 +235,23 @@ fn call_module<M: MemorySize>(
.runtime
.on_taint(TaintReason::UnknownWasiVersion);
ret = Errno::Noexec;
exit_code = Some(ExitCode::Other(128 + ret as i32));
Arshia001 marked this conversation as resolved.
Show resolved Hide resolved
}
Err(err) => {
debug!("failed with runtime error: {}", err);
env.data(&store)
.runtime
.on_taint(TaintReason::RuntimeError(err));
ret = Errno::Noexec;
exit_code = Some(ExitCode::Other(128 + ret as i32));
Arshia001 marked this conversation as resolved.
Show resolved Hide resolved
}
}
} else {
debug!("thread exited cleanly without calling thread_exit");
}
trace!("callback finished (ret={})", ret);

// Clean up the environment
env.on_exit(store, Some(ret.into()));
env.on_exit(store, exit_code);

// Return the result
Ok(ret as u32)
Expand Down
54 changes: 54 additions & 0 deletions tests/integration/cli/tests/snapshot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1281,3 +1281,57 @@ fn test_snapshot_fs_rename() {
.run_wasm(include_bytes!("./wasm/fs-rename.wasm"));
assert_json_snapshot!(snapshot);
}

#[cfg_attr(any(target_env = "musl", target_os = "windows"), ignore)]
#[test]
fn test_snapshot_exit_0_from_main() {
Arshia001 marked this conversation as resolved.
Show resolved Hide resolved
let snapshot = TestBuilder::new()
.with_name(function!())
.run_wasm(include_bytes!("./wasm/exit-0-from-main.wasm"));
assert_json_snapshot!(snapshot);
}

#[cfg_attr(any(target_env = "musl", target_os = "windows"), ignore)]
#[test]
fn test_snapshot_exit_1_from_main() {
let snapshot = TestBuilder::new()
.with_name(function!())
.run_wasm(include_bytes!("./wasm/exit-1-from-main.wasm"));
assert_json_snapshot!(snapshot);
}

#[cfg_attr(any(target_env = "musl", target_os = "windows"), ignore)]
#[test]
fn test_snapshot_exit_0_from_worker() {
let snapshot = TestBuilder::new()
.with_name(function!())
.run_wasm(include_bytes!("./wasm/exit-0-from-worker.wasm"));
assert_json_snapshot!(snapshot);
}

#[cfg_attr(any(target_env = "musl", target_os = "windows"), ignore)]
#[test]
fn test_snapshot_exit_1_from_worker() {
let snapshot = TestBuilder::new()
.with_name(function!())
.run_wasm(include_bytes!("./wasm/exit-1-from-worker.wasm"));
assert_json_snapshot!(snapshot);
}

#[cfg_attr(any(target_env = "musl", target_os = "windows"), ignore)]
#[test]
fn test_snapshot_worker_terminating_normally() {
let snapshot = TestBuilder::new()
.with_name(function!())
.run_wasm(include_bytes!("./wasm/worker-terminating-normally.wasm"));
assert_json_snapshot!(snapshot);
}

#[cfg_attr(any(target_env = "musl", target_os = "windows"), ignore)]
#[test]
fn test_snapshot_worker_panicking() {
let snapshot = TestBuilder::new()
.with_name(function!())
.run_wasm(include_bytes!("./wasm/worker-panicking.wasm"));
assert_json_snapshot!(snapshot);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
---
source: tests/integration/cli/tests/snapshot.rs
expression: snapshot
---
{
"spec": {
"name": "snapshot::test_snapshot_exit_0_from_main",
"use_packages": [],
"include_webcs": [],
"cli_args": [],
"enable_threads": true,
"enable_network": false
},
"result": {
"Success": {
"stdout": "",
"stderr": "",
"exit_code": 0
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
---
source: tests/integration/cli/tests/snapshot.rs
assertion_line: 1309
expression: snapshot
---
{
"spec": {
"name": "snapshot::test_snapshot_exit_0_from_worker",
"use_packages": [],
"include_webcs": [],
"cli_args": [],
"enable_threads": true,
"enable_network": false
},
"result": {
"Success": {
"stdout": "",
"stderr": "",
"exit_code": 0
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
---
source: tests/integration/cli/tests/snapshot.rs
assertion_line: 1300
expression: snapshot
---
{
"spec": {
"name": "snapshot::test_snapshot_exit_1_from_main",
"use_packages": [],
"include_webcs": [],
"cli_args": [],
"enable_threads": true,
"enable_network": false
},
"result": {
"Success": {
"stdout": "",
"stderr": "",
"exit_code": 1
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
---
source: tests/integration/cli/tests/snapshot.rs
assertion_line: 1318
expression: snapshot
---
{
"spec": {
"name": "snapshot::test_snapshot_exit_1_from_worker",
"use_packages": [],
"include_webcs": [],
"cli_args": [],
"enable_threads": true,
"enable_network": false
},
"result": {
"Success": {
"stdout": "",
"stderr": "",
"exit_code": 1
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
---
source: tests/integration/cli/tests/snapshot.rs
assertion_line: 1336
expression: snapshot
---
{
"spec": {
"name": "snapshot::test_snapshot_worker_panicking",
"use_packages": [],
"include_webcs": [],
"cli_args": [],
"enable_threads": true,
"enable_network": false
},
"result": {
"Success": {
"stdout": "",
"stderr": "thread '<unnamed>' panicked at src/bin/worker-panicking.rs:3:9:\nchild thread panicking\nnote: run with `RUST_BACKTRACE=1` environment variable to display a backtrace\n",
"exit_code": 173
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
---
source: tests/integration/cli/tests/snapshot.rs
assertion_line: 1327
expression: snapshot
---
{
"spec": {
"name": "snapshot::test_snapshot_worker_terminating_normally",
"use_packages": [],
"include_webcs": [],
"cli_args": [],
"enable_threads": true,
"enable_network": false
},
"result": {
"Success": {
"stdout": "In child thread\nIn main thread\n",
"stderr": "",
"exit_code": 0
}
}
}
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
3 changes: 3 additions & 0 deletions tests/rust-wasi-tests/src/bin/exit-0-from-main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
fn main() {
std::process::exit(0);
}
3 changes: 3 additions & 0 deletions tests/rust-wasi-tests/src/bin/exit-0-from-worker.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
fn main() {
std::thread::spawn(|| std::process::exit(0)).join().unwrap();
}
3 changes: 3 additions & 0 deletions tests/rust-wasi-tests/src/bin/exit-1-from-main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
fn main() {
std::process::exit(1);
}
3 changes: 3 additions & 0 deletions tests/rust-wasi-tests/src/bin/exit-1-from-worker.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
fn main() {
std::thread::spawn(|| std::process::exit(1)).join().unwrap();
}
11 changes: 11 additions & 0 deletions tests/rust-wasi-tests/src/bin/worker-panicking.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
fn main() {
std::thread::spawn(|| {
panic!("child thread panicking");
})
.join()
.unwrap();

println!("In main thread");

std::process::exit(0);
}
Loading
Loading