From 97f81b1150165e6b20f7d46d3ed4dd15a1af1563 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20Gonz=C3=A1lez?= Date: Wed, 29 Mar 2023 22:45:14 +0200 Subject: [PATCH 1/3] fix: Fixes thread issues described in #40 by refactoring `execution_ctx.rs` --- CHANGELOG.md | 8 ++ mod_wasm/modules/wasm/mod_wasm.h | 2 +- wasm_runtime/Cargo.lock | 139 +++++++++++++++++++----------- wasm_runtime/Cargo.toml | 2 +- wasm_runtime/include/version.h | 4 +- wasm_runtime/src/execution_ctx.rs | 97 ++++++++++++++------- 6 files changed, 168 insertions(+), 84 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 73291054..e08b8d16 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,14 @@ - +## 0.11.1 (2023/03/30) + +- Fixes [#40](https://github.com/vmware-labs/mod_wasm/issues/40), where a new thread could not create a new Wasm execution context while another thread was runnig a Wasm module. This was only mesurable if the execution of the Wasm module was long enough in time or it took longer than expected (i.e.: I/O issues, infinite loop, etc.). Note that CPU-limited Wasm executions are not implemented yet (see [#9](https://github.com/vmware-labs/mod_wasm/issues/9)). + +### `libwasm_runtime.so` +- Internal refactoring of `execution_ctx.rs` to address [#40](https://github.com/vmware-labs/mod_wasm/issues/40). +- Updated `cargo.lock` dependencies via `cargo update`. + ## 0.11.0 (2023/03/27) - In this version, Wasm modules can now return any output type via stdout, including binaries with non UTF-8 bytes sequences or `\0` NULL terminators in the middle. diff --git a/mod_wasm/modules/wasm/mod_wasm.h b/mod_wasm/modules/wasm/mod_wasm.h index e193e05f..da77068f 100644 --- a/mod_wasm/modules/wasm/mod_wasm.h +++ b/mod_wasm/modules/wasm/mod_wasm.h @@ -15,4 +15,4 @@ */ #define MOD_WASM_VERSION_MAJOR 0 #define MOD_WASM_VERSION_MINOR 11 -#define MOD_WASM_VERSION_PATCH 0 +#define MOD_WASM_VERSION_PATCH 1 diff --git a/wasm_runtime/Cargo.lock b/wasm_runtime/Cargo.lock index 902c4a90..1bc8af7a 100644 --- a/wasm_runtime/Cargo.lock +++ b/wasm_runtime/Cargo.lock @@ -51,7 +51,7 @@ checksum = "b9ccdd8f2a161be9bd5c023df56f1b2a0bd1d83872ae53b71a84a12c9bf6e842" dependencies = [ "proc-macro2", "quote", - "syn 2.0.10", + "syn 2.0.11", ] [[package]] @@ -104,9 +104,9 @@ checksum = "14c189c53d098945499cdfa7ecc63567cf3886b3332b312a5b4585d8d3a6a610" [[package]] name = "cap-fs-ext" -version = "1.0.7" +version = "1.0.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "07d9cd7dc1d714d59974a6a68bed489c914b7b2620d1d4334d88d5ec9f29ebbd" +checksum = "2767fc3595843a8cfb4b95ac507d1535fb6df994cd3566092786591b770542fb" dependencies = [ "cap-primitives", "cap-std", @@ -116,26 +116,26 @@ dependencies = [ [[package]] name = "cap-primitives" -version = "1.0.7" +version = "1.0.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8e41334d53bab60f94878253f8a950c231596c8bbb99b4f71b13223dd48e18c6" +checksum = "68c5812f1b3818f5132a8ea1ddcc09c32bc4374874616c6093f2d352e93f1d30" dependencies = [ "ambient-authority", - "fs-set-times", + "fs-set-times 0.19.0", "io-extras", "io-lifetimes", "ipnet", "maybe-owned", - "rustix", + "rustix 0.37.4", "windows-sys", "winx", ] [[package]] name = "cap-rand" -version = "1.0.7" +version = "1.0.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6b5ddc7e3565e7cc4bf20d0c386b328f9e0f1b83fe0bcc0e055a1f08245e2aca" +checksum = "b494c41f7d3ce72095f5fe9f61d732313ac959d91e1c863518073d234b69720e" dependencies = [ "ambient-authority", "rand", @@ -143,26 +143,25 @@ dependencies = [ [[package]] name = "cap-std" -version = "1.0.7" +version = "1.0.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e9dd840c16dee1df417f3985d173a2bb6ef55d48ea3d4deddcef46f31c9e7028" +checksum = "a83342c1f448b1d092cc55c6127ffe88841e9c98dd9f652a283a89279b12376c" dependencies = [ "cap-primitives", "io-extras", "io-lifetimes", - "ipnet", - "rustix", + "rustix 0.37.4", ] [[package]] name = "cap-time-ext" -version = "1.0.7" +version = "1.0.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "77c39790e8e7455a92993bea5a2e947721c395cfbc344b74f092746c55441d76" +checksum = "1c36aba6f39b14951cc10cc331441ffbdb471960d27e2f0813eb05f33d786e56" dependencies = [ "cap-primitives", "once_cell", - "rustix", + "rustix 0.37.4", "winx", ] @@ -440,6 +439,17 @@ dependencies = [ "winapi", ] +[[package]] +name = "errno" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "50d6a0976c999d473fe89ad888d5a284e55366d9dc9038b1ba2aa15128c4afa0" +dependencies = [ + "errno-dragonfly", + "libc", + "windows-sys", +] + [[package]] name = "errno-dragonfly" version = "0.1.2" @@ -458,12 +468,12 @@ checksum = "4443176a9f2c162692bd3d352d745ef9413eec5782a80d8fd6f8a1ac692a07f7" [[package]] name = "fd-lock" -version = "3.0.10" +version = "3.0.11" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8ef1a30ae415c3a691a4f41afddc2dbcd6d70baf338368d85ebc1e8ed92cedb9" +checksum = "9799aefb4a2e4a01cc47610b1dd47c18ab13d991f27bbcaed9296f5a53d5cbad" dependencies = [ "cfg-if", - "rustix", + "rustix 0.37.4", "windows-sys", ] @@ -493,7 +503,18 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "857cf27edcb26c2a36d84b2954019573d335bb289876113aceacacdca47a4fd4" dependencies = [ "io-lifetimes", - "rustix", + "rustix 0.36.11", + "windows-sys", +] + +[[package]] +name = "fs-set-times" +version = "0.19.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bba733060df596995a5b3945c5d4c7362cfe9ab006baaddac633733908ec2814" +dependencies = [ + "io-lifetimes", + "rustix 0.37.4", "windows-sys", ] @@ -508,9 +529,9 @@ dependencies = [ [[package]] name = "generic-array" -version = "0.14.6" +version = "0.14.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bff49e947297f3312447abdca79f45f4738097cc82b06e72054d2223f601f1b9" +checksum = "85649ca51fd72272d7821adaf274ad91c288277713d9c18820d8499a7ff69e9a" dependencies = [ "typenum", "version_check", @@ -636,13 +657,13 @@ checksum = "12b6ee2129af8d4fb011108c73d99a1b83a85977f23b82460c0ae2e25bb4b57f" [[package]] name = "is-terminal" -version = "0.4.5" +version = "0.4.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8687c819457e979cc940d09cb16e42a1bf70aa6b60a549de6d3a62a0ee90c69e" +checksum = "256017f749ab3117e93acb91063009e1f1bb56d03965b14c2c8df4eb02c524d8" dependencies = [ "hermit-abi 0.3.1", "io-lifetimes", - "rustix", + "rustix 0.37.4", "windows-sys", ] @@ -708,6 +729,12 @@ version = "0.1.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f051f77a7c8e6957c0696eac88f26b0117e54f52d3fc682ab19397a8812846a4" +[[package]] +name = "linux-raw-sys" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cd550e73688e6d578f0ac2119e32b797a327631a42f9433e59d02e139c8df60d" + [[package]] name = "log" version = "0.4.17" @@ -740,11 +767,11 @@ checksum = "2dffe52ecf27772e601905b7522cb4ef790d2cc203488bbd0e2fe85fcb74566d" [[package]] name = "memfd" -version = "0.6.2" +version = "0.6.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b20a59d985586e4a5aef64564ac77299f8586d8be6cf9106a5a40207e8908efb" +checksum = "ffc89ccdc6e10d6907450f753537ebc5c5d3460d2e4e62ea74bd571db62c0f9e" dependencies = [ - "rustix", + "rustix 0.37.4", ] [[package]] @@ -966,11 +993,25 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "db4165c9963ab29e422d6c26fbc1d37f15bace6b2810221f9d925023480fcf0e" dependencies = [ "bitflags", - "errno", + "errno 0.2.8", + "io-lifetimes", + "libc", + "linux-raw-sys 0.1.4", + "windows-sys", +] + +[[package]] +name = "rustix" +version = "0.37.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c348b5dc624ecee40108aa2922fed8bad89d7fcc2b9f8cb18f632898ac4a37f9" +dependencies = [ + "bitflags", + "errno 0.3.0", "io-lifetimes", "itoa", "libc", - "linux-raw-sys", + "linux-raw-sys 0.3.0", "once_cell", "windows-sys", ] @@ -983,22 +1024,22 @@ checksum = "d29ab0c6d3fc0ee92fe66e2d99f700eab17a8d57d1c1d3b748380fb20baa78cd" [[package]] name = "serde" -version = "1.0.158" +version = "1.0.159" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "771d4d9c4163ee138805e12c710dd365e4f44be8be0503cb1bb9eb989425d9c9" +checksum = "3c04e8343c3daeec41f58990b9d77068df31209f2af111e059e9fe9646693065" dependencies = [ "serde_derive", ] [[package]] name = "serde_derive" -version = "1.0.158" +version = "1.0.159" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e801c1712f48475582b7696ac71e0ca34ebb30e09338425384269d9717c62cad" +checksum = "4c614d17805b093df4b147b51339e7e44bf05ef59fba1e45d83500bcfb4d8585" dependencies = [ "proc-macro2", "quote", - "syn 2.0.10", + "syn 2.0.11", ] [[package]] @@ -1052,9 +1093,9 @@ dependencies = [ [[package]] name = "syn" -version = "2.0.10" +version = "2.0.11" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5aad1363ed6d37b84299588d62d3a7d95b5a5c2d9aad5c85609fda12afaa1f40" +checksum = "21e3787bb71465627110e7d87ed4faaa36c1f61042ee67badb9e2ef173accc40" dependencies = [ "proc-macro2", "quote", @@ -1063,16 +1104,16 @@ dependencies = [ [[package]] name = "system-interface" -version = "0.25.4" +version = "0.25.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f355df185d945435f24c51fda9bf01bea6acb6c0b753e1241e5cc05413a659d4" +checksum = "12a638b21790634294d82697a110052af16bf88d88bec7c8f8e57989e2f922b7" dependencies = [ "bitflags", "cap-fs-ext", "cap-std", "fd-lock", "io-lifetimes", - "rustix", + "rustix 0.37.4", "windows-sys", "winx", ] @@ -1109,7 +1150,7 @@ checksum = "f9456a42c5b0d803c8cd86e73dd7cc9edd429499f37a3550d286d5e86720569f" dependencies = [ "proc-macro2", "quote", - "syn 2.0.10", + "syn 2.0.11", ] [[package]] @@ -1252,12 +1293,12 @@ dependencies = [ "cap-rand", "cap-std", "cap-time-ext", - "fs-set-times", + "fs-set-times 0.18.1", "io-extras", "io-lifetimes", "is-terminal", "once_cell", - "rustix", + "rustix 0.36.11", "system-interface", "tracing", "wasi-common", @@ -1276,7 +1317,7 @@ dependencies = [ "cap-std", "io-extras", "log", - "rustix", + "rustix 0.36.11", "thiserror", "tracing", "wasmtime", @@ -1295,7 +1336,7 @@ dependencies = [ [[package]] name = "wasm_runtime" -version = "0.11.0" +version = "0.11.1" dependencies = [ "anyhow", "once_cell", @@ -1369,7 +1410,7 @@ dependencies = [ "directories-next", "file-per-thread-logger", "log", - "rustix", + "rustix 0.36.11", "serde", "sha2", "toml", @@ -1446,7 +1487,7 @@ checksum = "01b1192624694399f601de28db78975ed20fa859da8e048bf8250bd3b38d302b" dependencies = [ "cc", "cfg-if", - "rustix", + "rustix 0.36.11", "wasmtime-asm-macros", "windows-sys", ] @@ -1484,7 +1525,7 @@ checksum = "17e35d335dd2461c631ba24d2326d993bd3a4bdb4b0217e5bda4f518ba0e29f3" dependencies = [ "object", "once_cell", - "rustix", + "rustix 0.36.11", ] [[package]] @@ -1515,7 +1556,7 @@ dependencies = [ "memoffset", "paste", "rand", - "rustix", + "rustix 0.36.11", "wasmtime-asm-macros", "wasmtime-environ", "wasmtime-fiber", diff --git a/wasm_runtime/Cargo.toml b/wasm_runtime/Cargo.toml index 18def119..15ff59da 100644 --- a/wasm_runtime/Cargo.toml +++ b/wasm_runtime/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "wasm_runtime" -version = "0.11.0" +version = "0.11.1" edition = "2021" authors = ["VMware's Wasm Labs"] description = "Wrapper for offering a simple C-API to manage WebAssembly modules via Wasmtime" diff --git a/wasm_runtime/include/version.h b/wasm_runtime/include/version.h index a3d60f98..d64d59dc 100644 --- a/wasm_runtime/include/version.h +++ b/wasm_runtime/include/version.h @@ -1,5 +1,5 @@ -#define WASM_RUNTIME_VERSION "0.11.0" +#define WASM_RUNTIME_VERSION "0.11.1" #define WASM_RUNTIME_VERSION_MAJOR 0 #define WASM_RUNTIME_VERSION_MINOR 11 -#define WASM_RUNTIME_VERSION_PATCH 0 +#define WASM_RUNTIME_VERSION_PATCH 1 diff --git a/wasm_runtime/src/execution_ctx.rs b/wasm_runtime/src/execution_ctx.rs index 6a2f2e80..550c713c 100644 --- a/wasm_runtime/src/execution_ctx.rs +++ b/wasm_runtime/src/execution_ctx.rs @@ -37,10 +37,6 @@ impl WasmExecutionCtx { /// Or in case of invalid `config_id`, it returns a String explaing the error. /// pub fn create_from_config(config_id: &str) -> Result { - - // get write access to the WasmExecutionCtx HashMap - let mut executionctxs = WASM_RUNTIME_EXECUTIONCTXS.write() - .expect("ERROR! Poisoned RwLock WASM_RUNTIME_EXECUTIONCTXS on write()"); // get read access to the WasmConfig HashMap let configs = WASM_RUNTIME_CONFIGS.read() @@ -71,10 +67,7 @@ impl WasmExecutionCtx { wasi_stdout: Arc::new(RwLock::new(Vec::new())), }; - // insert created WasmExecutionCtx object into the HashMap - executionctxs.insert(wasm_executionctx.id.clone(), wasm_executionctx); - - Ok(hex_id) + Self::try_insert(wasm_executionctx) } @@ -84,17 +77,10 @@ impl WasmExecutionCtx { /// Returns Result<(), String>, so that in case of error the String will contain the reason. /// pub fn deallocate(executionctx_id: &str) -> Result<(), String> { - - // get write access to the WasmExecutionCtx HashMap - let mut executionctxs = WASM_RUNTIME_EXECUTIONCTXS.write() - .expect("ERROR! Poisoned RwLock WASM_RUNTIME_EXECUTIONCTXS on write()"); - - match executionctxs.remove(executionctx_id) { - Some(_) => Ok(()), - None => { - let error_msg = format!("Wasm execution context \'{}\' to deallocate not found!", executionctx_id); - Err(error_msg) - } + // extracts the Execution Context and it is automatically dropped + match Self::extract(executionctx_id) { + Ok(_) => Ok(()), + Err(e) => Err(e) } } @@ -159,24 +145,28 @@ impl WasmExecutionCtx { /// pub fn run(executionctx_id: &str) -> Result, String> { - // get read access to the WasmExecutionCtx HashMap - let executionctxs = WASM_RUNTIME_EXECUTIONCTXS.read() - .expect("ERROR! Poisoned RwLock WASM_RUNTIME_EXECUTIONCTXS on read()"); - - // get the given WasmExecutionCtx object - let wasm_executionctx = match executionctxs.get(executionctx_id) { - Some(exectx) => exectx, - None => { - let error_msg = format!("Wasm execution context \'{}\' not created previously!", executionctx_id); + // extract Wasm execution context + let wasm_executionctx = match Self::extract(executionctx_id) { + Ok(exectx) => exectx, + Err(e) => { + let error_msg = format!("Can't run Wasm execution context \'{}\'! {}", executionctx_id, e); return Err(error_msg); } }; - // invoke "_start" function for the given Wasm execution context - wasm_engine::invoke_wasm_function(wasm_executionctx, "_start")?; + // invoke default "_start" function for the given Wasm execution context + wasm_engine::invoke_wasm_function(&wasm_executionctx, "_start")?; // read stdout from the Wasm execution context and return it - Ok(Self::read_stdout(wasm_executionctx)) + let wasm_module_stdout = Self::read_stdout(&wasm_executionctx); + + match Self::try_insert(wasm_executionctx) { + Ok(_) => Ok(wasm_module_stdout), + Err(e) => { + let error_msg = format!("Can't insert back Wasm execution context \'{}\' after execution! {}", executionctx_id, e); + return Err(error_msg); + } + } } // Helper function to generate random hex IDs for the given length @@ -207,6 +197,51 @@ impl WasmExecutionCtx { stdout_buf.clone() } + + // Helper function to insert a Wasm execution context from the HashMap + // + // It checks for duplicated `executionctx_id`. + // Returns Result, with the inserted Wasm execution context id, + // otherswise, in case of error the String will contain the reason. + // + fn try_insert(wasm_executionctx: WasmExecutionCtx) -> Result { + let executionctx_id = wasm_executionctx.id.clone(); + + // get write access to the WasmExecutionCtx HashMap + let mut executionctxs = WASM_RUNTIME_EXECUTIONCTXS.write() + .expect("ERROR! Poisoned RwLock WASM_RUNTIME_EXECUTIONCTXS on write()"); + + // check for existing `executionctx_id` + match executionctxs.get(&wasm_executionctx.id) { + Some(existing_ectx) => { + let error_msg = format!("Wasm execution context \'{}\' already exist!", existing_ectx.id); + Err(error_msg) + }, + None => { // insert for non-existing `executionctx_id` + executionctxs.insert(wasm_executionctx.id.clone(), wasm_executionctx); + Ok(executionctx_id) + } + } + } + + // Helper function to extract a Wasm execution context from the HashMap + // + // It checks for wrong `executionctx_id`. + // Returns Result<(), String>, so that in case of error the String will contain the reason. + // + fn extract(executionctx_id: &str) -> Result { + // get write access to the WasmExecutionCtx HashMap + let mut executionctxs = WASM_RUNTIME_EXECUTIONCTXS.write() + .expect("ERROR! Poisoned RwLock WASM_RUNTIME_EXECUTIONCTXS on write()"); + + match executionctxs.remove(executionctx_id) { + Some(ectx) => Ok(ectx), + None => { + let error_msg = format!("Wasm execution context \'{}\' not found!", executionctx_id); + Err(error_msg) + } + } + } } From 49121fb9aa2ea53494439bae45b92cc430a98d35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20Gonz=C3=A1lez?= Date: Wed, 29 Mar 2023 22:56:09 +0200 Subject: [PATCH 2/3] chore: `README.md` typos --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e08b8d16..00a962e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ ## 0.11.1 (2023/03/30) -- Fixes [#40](https://github.com/vmware-labs/mod_wasm/issues/40), where a new thread could not create a new Wasm execution context while another thread was runnig a Wasm module. This was only mesurable if the execution of the Wasm module was long enough in time or it took longer than expected (i.e.: I/O issues, infinite loop, etc.). Note that CPU-limited Wasm executions are not implemented yet (see [#9](https://github.com/vmware-labs/mod_wasm/issues/9)). +- Fixes [#40](https://github.com/vmware-labs/mod_wasm/issues/40), where a new thread could not create a new Wasm execution context while another thread was running a Wasm module. This was only measurable if the execution of the Wasm module was long enough in time or if it took longer than expected (i.e.: I/O issues, infinite loop, etc.). Note that CPU-limited Wasm executions are not implemented yet (see [#9](https://github.com/vmware-labs/mod_wasm/issues/9)). ### `libwasm_runtime.so` - Internal refactoring of `execution_ctx.rs` to address [#40](https://github.com/vmware-labs/mod_wasm/issues/40). From 1cedb9ab459c239adcad87319e956a4997daa2ce Mon Sep 17 00:00:00 2001 From: gzurl <31731754+gzurl@users.noreply.github.com> Date: Fri, 31 Mar 2023 17:18:11 +0200 Subject: [PATCH 3/3] Updating release date --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 00a962e8..051770f9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ - -## 0.11.1 (2023/03/30) +## 0.11.1 (2023/03/31) - Fixes [#40](https://github.com/vmware-labs/mod_wasm/issues/40), where a new thread could not create a new Wasm execution context while another thread was running a Wasm module. This was only measurable if the execution of the Wasm module was long enough in time or if it took longer than expected (i.e.: I/O issues, infinite loop, etc.). Note that CPU-limited Wasm executions are not implemented yet (see [#9](https://github.com/vmware-labs/mod_wasm/issues/9)).