From e82d077643da19d17c49290d8486b257c65d8007 Mon Sep 17 00:00:00 2001 From: Christoph Herzog Date: Thu, 25 Apr 2024 15:53:14 +0200 Subject: [PATCH 1/3] feat(wasix): Improve SpawnError information Extend SpawnError to add more context and information for some errors that happen during instance spawning. Makes debugging and error reporting a lot better. --- lib/wasix/src/bin_factory/exec.rs | 71 +++++++------------------------ lib/wasix/src/bin_factory/mod.rs | 53 ++++++++++++++++++++++- lib/wasix/src/lib.rs | 61 ++++++++++++++++++++++++-- lib/wasix/src/runtime/mod.rs | 17 +++++--- 4 files changed, 135 insertions(+), 67 deletions(-) diff --git a/lib/wasix/src/bin_factory/exec.rs b/lib/wasix/src/bin_factory/exec.rs index 6d5db50c289..ee5fa44ab8c 100644 --- a/lib/wasix/src/bin_factory/exec.rs +++ b/lib/wasix/src/bin_factory/exec.rs @@ -1,4 +1,4 @@ -use std::{pin::Pin, sync::Arc}; +use std::sync::Arc; use crate::{ os::task::{ @@ -14,12 +14,11 @@ use crate::{ syscalls::rewind_ext, RewindState, SpawnError, WasiError, WasiRuntimeError, }; -use futures::Future; use tracing::*; -use wasmer::{Function, FunctionEnvMut, Memory32, Memory64, Module, Store}; +use wasmer::{Function, Memory32, Memory64, Module, Store}; use wasmer_wasix_types::wasi::Errno; -use super::{BinFactory, BinaryPackage}; +use super::BinaryPackage; use crate::{Runtime, WasiEnv, WasiFunctionEnv}; #[tracing::instrument(level = "trace", skip_all, fields(%name, package_id=%binary.id))] @@ -59,7 +58,9 @@ pub async fn spawn_load_wasm<'a>( "Unable to spawn a command because its package has no entrypoint", ); env.on_exit(Some(Errno::Noexec.into())).await; - return Err(SpawnError::CompileError); + return Err(SpawnError::MissingEntrypoint { + package_id: binary.id.clone(), + }); }; Ok(wasm) } @@ -70,19 +71,18 @@ pub async fn spawn_load_module( wasm: &[u8], runtime: &Arc, ) -> Result { - let module = match runtime.load_module(wasm).await { - Ok(module) => module, + match runtime.load_module(wasm).await { + Ok(module) => Ok(module), Err(err) => { tracing::error!( command = name, - error = &*err, + error = &err as &dyn std::error::Error, "Failed to compile the module", ); env.on_exit(Some(Errno::Noexec.into())).await; - return Err(SpawnError::CompileError); + Err(err) } - }; - Ok(module) + } } pub async fn spawn_union_fs(env: &WasiEnv, binary: &BinaryPackage) -> Result<(), SpawnError> { @@ -93,7 +93,10 @@ pub async fn spawn_union_fs(env: &WasiEnv, binary: &BinaryPackage) -> Result<(), .await .map_err(|err| { tracing::warn!("failed to union file system - {err}"); - SpawnError::FileSystemError + SpawnError::FileSystemError(crate::ExtendedFsError::with_msg( + err, + "could not union filesystems", + )) })?; tracing::debug!("{:?}", env.state.fs); Ok(()) @@ -326,47 +329,3 @@ fn call_module( debug!("wasi[{pid}]::main() has exited with {code}"); handle.thread.set_status_finished(ret.map(|a| a.into())); } - -impl BinFactory { - pub fn spawn<'a>( - &'a self, - name: String, - store: Store, - env: WasiEnv, - ) -> Pin> + 'a>> { - Box::pin(async move { - // Find the binary (or die trying) and make the spawn type - let binary = self - .get_binary(name.as_str(), Some(env.fs_root())) - .await - .ok_or(SpawnError::NotFound); - if binary.is_err() { - env.on_exit(Some(Errno::Noent.into())).await; - } - let binary = binary?; - - // Execute - spawn_exec(binary, name.as_str(), store, env, &self.runtime).await - }) - } - - pub fn try_built_in( - &self, - name: String, - parent_ctx: Option<&FunctionEnvMut<'_, WasiEnv>>, - store: &mut Option, - builder: &mut Option, - ) -> Result { - // We check for built in commands - if let Some(parent_ctx) = parent_ctx { - if self.commands.exists(name.as_str()) { - return self - .commands - .exec(parent_ctx, name.as_str(), store, builder); - } - } else if self.commands.exists(name.as_str()) { - tracing::warn!("builtin command without a parent ctx - {}", name); - } - Err(SpawnError::NotFound) - } -} diff --git a/lib/wasix/src/bin_factory/mod.rs b/lib/wasix/src/bin_factory/mod.rs index 26ee4a8fd23..16311cd4fad 100644 --- a/lib/wasix/src/bin_factory/mod.rs +++ b/lib/wasix/src/bin_factory/mod.rs @@ -1,12 +1,16 @@ use std::{ collections::HashMap, + future::Future, ops::Deref, path::Path, + pin::Pin, sync::{Arc, RwLock}, }; use anyhow::Context; use virtual_fs::{AsyncReadExt, FileSystem}; +use wasmer::FunctionEnvMut; +use wasmer_wasix_types::wasi::Errno; use webc::Container; mod binary_package; @@ -18,7 +22,10 @@ pub use self::{ run_exec, spawn_exec, spawn_exec_module, spawn_load_module, spawn_load_wasm, spawn_union_fs, }, }; -use crate::{os::command::Commands, Runtime}; +use crate::{ + os::{command::Commands, task::TaskJoinHandle}, + Runtime, SpawnError, WasiEnv, +}; #[derive(Debug, Clone)] pub struct BinFactory { @@ -94,6 +101,50 @@ impl BinFactory { cache.insert(name, None); None } + + pub fn spawn<'a>( + &'a self, + name: String, + store: wasmer::Store, + env: WasiEnv, + ) -> Pin> + 'a>> { + Box::pin(async move { + // Find the binary (or die trying) and make the spawn type + let res = self + .get_binary(name.as_str(), Some(env.fs_root())) + .await + .ok_or_else(|| SpawnError::BinaryNotFound { + binary: name.clone(), + }); + if res.is_err() { + env.on_exit(Some(Errno::Noent.into())).await; + } + let binary = res?; + + // Execute + spawn_exec(binary, name.as_str(), store, env, &self.runtime).await + }) + } + + pub fn try_built_in( + &self, + name: String, + parent_ctx: Option<&FunctionEnvMut<'_, WasiEnv>>, + store: &mut Option, + builder: &mut Option, + ) -> Result { + // We check for built in commands + if let Some(parent_ctx) = parent_ctx { + if self.commands.exists(name.as_str()) { + return self + .commands + .exec(parent_ctx, name.as_str(), store, builder); + } + } else if self.commands.exists(name.as_str()) { + tracing::warn!("builtin command without a parent ctx - {}", name); + } + Err(SpawnError::BinaryNotFound { binary: name }) + } } async fn load_package_from_filesystem( diff --git a/lib/wasix/src/lib.rs b/lib/wasix/src/lib.rs index 258fd19541e..16441e41889 100644 --- a/lib/wasix/src/lib.rs +++ b/lib/wasix/src/lib.rs @@ -137,9 +137,14 @@ pub enum SpawnError { /// Failed to fetch the Wasmer process #[error("fetch failed")] FetchFailed, + #[error(transparent)] + CacheError(crate::runtime::module_cache::CacheError), /// Failed to compile the Wasmer process #[error("compile error")] - CompileError, + CompileError { + module_hash: crate::runtime::module_cache::ModuleHash, + error: wasmer::CompileError, + }, /// Invalid ABI #[error("Wasmer process has an invalid ABI")] InvalidABI, @@ -152,6 +157,16 @@ pub enum SpawnError { /// Not found #[error("not found")] NotFound, + /// Tried to run the specified binary as a new WASI thread/process, but + /// the binary name was not found. + #[error("could not find binary '{binary}'")] + BinaryNotFound { binary: String }, + #[error("could not find an entrypoint in the package '{package_id}'")] + MissingEntrypoint { + package_id: wasmer_config::package::PackageId, + }, + #[error("could not load ")] + ModuleLoad { message: String }, /// Bad request #[error("bad request")] BadRequest, @@ -162,8 +177,8 @@ pub enum SpawnError { #[error("internal error")] InternalError, /// An error occurred while preparing the file system - #[error("file system error")] - FileSystemError, + #[error(transparent)] + FileSystemError(ExtendedFsError), /// Memory allocation failed #[error("memory allocation failed")] MemoryAllocationFailed, @@ -179,6 +194,46 @@ pub enum SpawnError { Other(#[from] Box), } +#[derive(Debug)] +pub struct ExtendedFsError { + pub error: virtual_fs::FsError, + pub message: Option, +} + +impl ExtendedFsError { + pub fn with_msg(error: virtual_fs::FsError, msg: impl Into) -> Self { + Self { + error, + message: Some(msg.into()), + } + } + + pub fn new(error: virtual_fs::FsError) -> Self { + Self { + error, + message: None, + } + } +} + +impl std::fmt::Display for ExtendedFsError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> { + write!(f, "fs error: {}", self.error)?; + + if let Some(msg) = &self.message { + write!(f, " | {}", msg)?; + } + + Ok(()) + } +} + +impl std::error::Error for ExtendedFsError { + fn cause(&self) -> Option<&dyn std::error::Error> { + Some(&self.error) + } +} + impl SpawnError { /// Returns `true` if the spawn error is [`NotFound`]. /// diff --git a/lib/wasix/src/runtime/mod.rs b/lib/wasix/src/runtime/mod.rs index 20e7cb8e57e..517b2faa2bd 100644 --- a/lib/wasix/src/runtime/mod.rs +++ b/lib/wasix/src/runtime/mod.rs @@ -31,7 +31,7 @@ use crate::{ package_loader::{PackageLoader, UnsupportedPackageLoader}, resolver::{MultiSource, Source, WapmSource}, }, - WasiTtyState, + SpawnError, WasiTtyState, }; #[derive(Clone)] @@ -101,7 +101,7 @@ where } /// Load a a Webassembly module, trying to use a pre-compiled version if possible. - fn load_module<'a>(&'a self, wasm: &'a [u8]) -> BoxFuture<'a, anyhow::Result> { + fn load_module<'a>(&'a self, wasm: &'a [u8]) -> BoxFuture<'a, Result> { let engine = self.engine(); let module_cache = self.module_cache(); let hash = ModuleHash::hash(wasm); @@ -114,7 +114,7 @@ where /// Load a a Webassembly module, trying to use a pre-compiled version if possible. /// /// Non-async version of [`Self::load_module`]. - fn load_module_sync(&self, wasm: &[u8]) -> Result { + fn load_module_sync(&self, wasm: &[u8]) -> Result { InlineWaker::block_on(self.load_module(wasm)) } @@ -152,7 +152,7 @@ pub async fn load_module( module_cache: &(dyn ModuleCache + Send + Sync), wasm: &[u8], wasm_hash: ModuleHash, -) -> Result { +) -> Result { let result = module_cache.load(wasm_hash, engine).await; match result { @@ -167,7 +167,10 @@ pub async fn load_module( } } - let module = Module::new(&engine, wasm)?; + let module = Module::new(&engine, wasm).map_err(|err| crate::SpawnError::CompileError { + module_hash: wasm_hash, + error: err, + })?; if let Err(e) = module_cache.save(wasm_hash, engine, &module).await { tracing::warn!( @@ -546,7 +549,7 @@ impl Runtime for OverriddenRuntime { } } - fn load_module<'a>(&'a self, wasm: &'a [u8]) -> BoxFuture<'a, anyhow::Result> { + fn load_module<'a>(&'a self, wasm: &'a [u8]) -> BoxFuture<'a, Result> { if self.engine.is_some() || self.module_cache.is_some() { let engine = self.engine(); let module_cache = self.module_cache(); @@ -559,7 +562,7 @@ impl Runtime for OverriddenRuntime { } } - fn load_module_sync(&self, wasm: &[u8]) -> Result { + fn load_module_sync(&self, wasm: &[u8]) -> Result { if self.engine.is_some() || self.module_cache.is_some() { InlineWaker::block_on(self.load_module(wasm)) } else { From 8dfd7c22f983445c0ab526494d4e9dbd8a243bf2 Mon Sep 17 00:00:00 2001 From: Christoph Herzog Date: Thu, 25 Apr 2024 15:55:19 +0200 Subject: [PATCH 2/3] chore: Bump wasmer-wasix to 0.19.0 --- lib/wasix/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/wasix/Cargo.toml b/lib/wasix/Cargo.toml index 29d6fcf5545..6efa5a1a971 100644 --- a/lib/wasix/Cargo.toml +++ b/lib/wasix/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "wasmer-wasix" -version = "0.18.4" +version = "0.19.0" description = "WASI and WASIX implementation library for Wasmer WebAssembly runtime" categories = ["wasm", "os"] keywords = ["wasm", "webassembly", "wasi", "sandbox", "ABI"] From 3b31d4dcb66cb1d66f786610c1311ac2fb72c526 Mon Sep 17 00:00:00 2001 From: Christoph Herzog Date: Thu, 25 Apr 2024 15:59:12 +0200 Subject: [PATCH 3/3] deps: Lift wasmer-wasix to be a workspace dependency --- Cargo.lock | 2 +- Cargo.toml | 6 +++++- lib/c-api/Cargo.toml | 2 +- lib/cli/Cargo.toml | 2 +- lib/sys-utils/Cargo.toml | 2 +- tests/lib/wast/Cargo.toml | 5 +++-- 6 files changed, 12 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4dc7c00bc81..9d6e327bed3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6852,7 +6852,7 @@ dependencies = [ [[package]] name = "wasmer-wasix" -version = "0.18.4" +version = "0.19.0" dependencies = [ "anyhow", "async-trait", diff --git a/Cargo.toml b/Cargo.toml index fa699fc4537..961363f30e9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -12,6 +12,7 @@ rust-version.workspace = true version.workspace = true [dependencies] +# Repo-local crates wasmer-config = { path = "./lib/config" } wasmer = { version = "=4.3.0-alpha.1", path = "lib/api", default-features = false } wasmer-compiler = { version = "=4.3.0-alpha.1", path = "lib/compiler", features = [ @@ -21,12 +22,14 @@ wasmer-compiler-cranelift = { version = "=4.3.0-alpha.1", path = "lib/compiler-c wasmer-compiler-singlepass = { version = "=4.3.0-alpha.1", path = "lib/compiler-singlepass", optional = true } wasmer-compiler-llvm = { version = "=4.3.0-alpha.1", path = "lib/compiler-llvm", optional = true } wasmer-emscripten = { version = "=4.3.0-alpha.1", path = "lib/emscripten", optional = true } -wasmer-wasix = { version = "0.18.4", path = "lib/wasix", optional = true } +wasmer-wasix = { path = "lib/wasix", optional = true } wasmer-wast = { version = "=4.3.0-alpha.1", path = "tests/lib/wast", optional = true } wasi-test-generator = { version = "=4.3.0-alpha.1", path = "tests/wasi-wast", optional = true } wasmer-cache = { version = "=4.3.0-alpha.1", path = "lib/cache", optional = true } wasmer-types = { version = "=4.3.0-alpha.1", path = "lib/types" } wasmer-middlewares = { version = "=4.3.0-alpha.1", path = "lib/middlewares", optional = true } + +# Third party dependencies cfg-if = "1.0" tokio = { version = "1", features = [ "rt", @@ -88,6 +91,7 @@ version = "4.3.0-alpha.1" [workspace.dependencies] # Repo-local crates wasmer-config = { path = "./lib/config" } +wasmer-wasix = { path = "./lib/wasix" } # Wasmer-owned crates webc = { version = "6.0.0-alpha3", default-features = false, features = ["package"] } diff --git a/lib/c-api/Cargo.toml b/lib/c-api/Cargo.toml index f1b0c9ba0a6..64d7a425358 100644 --- a/lib/c-api/Cargo.toml +++ b/lib/c-api/Cargo.toml @@ -32,7 +32,7 @@ wasmer-compiler-singlepass = { version = "=4.3.0-alpha.1", path = "../compiler-s wasmer-emscripten = { version = "=4.3.0-alpha.1", path = "../emscripten", optional = true } wasmer-middlewares = { version = "=4.3.0-alpha.1", path = "../middlewares", optional = true } wasmer-types = { version = "=4.3.0-alpha.1", path = "../types" } -wasmer-wasix = { version = "0.18.4", path = "../wasix", features = ["host-fs", "host-vnet"], optional = true } +wasmer-wasix = { workspace = true, features = ["host-fs", "host-vnet"], optional = true } webc = { workspace = true, optional = true } virtual-fs = { version = "0.11.3", path = "../virtual-fs", optional = true, default-features = false, features = ["static-fs"] } enumset.workspace = true diff --git a/lib/cli/Cargo.toml b/lib/cli/Cargo.toml index c2fc0979c09..3f5488eb811 100644 --- a/lib/cli/Cargo.toml +++ b/lib/cli/Cargo.toml @@ -76,7 +76,7 @@ wasmer-compiler-singlepass = { version = "=4.3.0-alpha.1", path = "../compiler-s wasmer-compiler-llvm = { version = "=4.3.0-alpha.1", path = "../compiler-llvm", optional = true } wasmer-emscripten = { version = "=4.3.0-alpha.1", path = "../emscripten" } wasmer-vm = { version = "=4.3.0-alpha.1", path = "../vm", optional = true } -wasmer-wasix = { version = "0.18.4", path = "../wasix", features = [ +wasmer-wasix = { workspace = true, features = [ "logging", "webc_runner_rt_wcgi", "webc_runner_rt_dcgi", diff --git a/lib/sys-utils/Cargo.toml b/lib/sys-utils/Cargo.toml index 7f889343d8b..46f5c079cd5 100644 --- a/lib/sys-utils/Cargo.toml +++ b/lib/sys-utils/Cargo.toml @@ -22,7 +22,7 @@ tracing = "0.1.37" libc = { version = "^0.2", default-features = false } [dev-dependencies] -wasmer-wasix = { path = "../wasix", version = "0.18.4" } +wasmer-wasix.workspace = true wasmer = { path = "../api", version = "=4.3.0-alpha.1", default-features = false, features = ["sys", "compiler", "cranelift"] } tracing-subscriber = { version = "0.3.16", features = ["fmt"] } tracing = "0.1.37" diff --git a/tests/lib/wast/Cargo.toml b/tests/lib/wast/Cargo.toml index 05888a01c3f..d3f5d6274a4 100644 --- a/tests/lib/wast/Cargo.toml +++ b/tests/lib/wast/Cargo.toml @@ -11,10 +11,11 @@ readme = "README.md" edition = "2018" [dependencies] -anyhow = "1.0" +wasmer-wasix.workspace = true wasmer = { path = "../../../lib/api", version = "=4.3.0-alpha.1", default-features = false } -wasmer-wasix = { path = "../../../lib/wasix", version = "0.18.4" } virtual-fs = { path = "../../../lib/virtual-fs", version = "0.11.3" } + +anyhow = "1.0" wast = "38.0" serde = "1" tempfile = "3.6.0"