From 915252eab9d97b7e20283d85d8bae93a7f19b03e Mon Sep 17 00:00:00 2001 From: Edoardo Marangoni Date: Wed, 27 Nov 2024 15:14:10 +0100 Subject: [PATCH 01/19] feat(api): Add function to get the runtime kind from an engine --- lib/api/src/entities/engine/inner.rs | 18 ++++++++++++++++++ lib/api/src/entities/engine/mod.rs | 5 +++++ lib/api/src/rt/js/entities/engine.rs | 5 +++++ lib/api/src/rt/jsc/entities/engine.rs | 5 +++++ lib/cli/src/commands/run/mod.rs | 6 ++++-- 5 files changed, 37 insertions(+), 2 deletions(-) diff --git a/lib/api/src/entities/engine/inner.rs b/lib/api/src/entities/engine/inner.rs index d4f960d7e23..cd06e86c009 100644 --- a/lib/api/src/entities/engine/inner.rs +++ b/lib/api/src/entities/engine/inner.rs @@ -13,6 +13,24 @@ use crate::{ gen_rt_ty!(Engine @derives Debug, Clone); impl RuntimeEngine { + /// Returns the [`crate::Runtime`] kind this engine belongs to. + pub fn get_rt_kind(&self) -> crate::Runtime { + match self { + #[cfg(feature = "sys")] + RuntimeEngine::Sys(_) => crate::Runtime::Sys, + #[cfg(feature = "v8")] + RuntimeEngine::V8(_) => crate::Runtime::V8, + #[cfg(feature = "wamr")] + RuntimeEngine::Wamr(_) => crate::Runtime::Wamr, + #[cfg(feature = "wasmi")] + RuntimeEngine::Wasmi(_) => crate::Runtime::Wasmi, + #[cfg(feature = "js")] + RuntimeEngine::Js(_) => crate::Runtime::Js, + #[cfg(feature = "jsc")] + RuntimeEngine::Jsc(_) => crate::Runtime::Jsc, + } + } + /// Returns the deterministic id of this engine. pub fn deterministic_id(&self) -> &str { match_rt!(on self => s { diff --git a/lib/api/src/entities/engine/mod.rs b/lib/api/src/entities/engine/mod.rs index f402e99698e..6befbaa1a08 100644 --- a/lib/api/src/entities/engine/mod.rs +++ b/lib/api/src/entities/engine/mod.rs @@ -48,6 +48,11 @@ impl Engine { ENGINE_ID_COUNTER.fetch_add(1, std::sync::atomic::Ordering::SeqCst) } + /// Returns the [`crate::Runtime`] kind this engine belongs to. + pub fn get_rt_kind(&self) -> crate::Runtime { + self.rt.get_rt_kind() + } + /// Returns the deterministic id of this engine. pub fn deterministic_id(&self) -> &str { self.rt.deterministic_id() diff --git a/lib/api/src/rt/js/entities/engine.rs b/lib/api/src/rt/js/entities/engine.rs index 3c1de461e11..5fa5f124e27 100644 --- a/lib/api/src/rt/js/entities/engine.rs +++ b/lib/api/src/rt/js/entities/engine.rs @@ -44,6 +44,11 @@ impl crate::Engine { _ => panic!("Not a `js` engine!"), } } + + /// Return true if [`self`] is an engine from the `js` runtime. + pub fn is_js(&self) -> bool { + matches!(self.rt, RuntimeEngine::Js(_)) + } } impl Into for Engine { diff --git a/lib/api/src/rt/jsc/entities/engine.rs b/lib/api/src/rt/jsc/entities/engine.rs index 834c6a9b17b..d6576fb5c24 100644 --- a/lib/api/src/rt/jsc/entities/engine.rs +++ b/lib/api/src/rt/jsc/entities/engine.rs @@ -240,6 +240,11 @@ impl crate::Engine { _ => panic!("Not a `jsc` engine!"), } } + + /// Return true if [`self`] is an engine from the `jsc` runtime. + pub fn is_jsc(&self) -> bool { + matches!(self.rt, RuntimeEngine::Jsc(_)) + } } impl Into for Engine { diff --git a/lib/cli/src/commands/run/mod.rs b/lib/cli/src/commands/run/mod.rs index 25b40257588..07628e0f135 100644 --- a/lib/cli/src/commands/run/mod.rs +++ b/lib/cli/src/commands/run/mod.rs @@ -25,8 +25,8 @@ use url::Url; #[cfg(feature = "sys")] use wasmer::sys::NativeEngineExt; use wasmer::{ - DeserializeError, Engine, Function, Imports, Instance, Module, Store, Type, TypedFunction, - Value, + AsStoreMut, DeserializeError, Engine, Function, Imports, Instance, Module, Store, Type, + TypedFunction, Value, }; #[cfg(feature = "compiler")] @@ -136,6 +136,8 @@ impl Run { } let engine = store.engine_mut(); + let rt_kind = engine.get_rt_kind(); + tracing::info!("Executing on runtime {rt_kind:?}"); #[cfg(feature = "sys")] if engine.is_sys() { From bf76d61d1691b746ff7fb33f7fa7ac211f5858a1 Mon Sep 17 00:00:00 2001 From: Edoardo Marangoni Date: Mon, 2 Dec 2024 15:22:34 +0100 Subject: [PATCH 02/19] fix(api/v8): Don't allow creation and instantiation on two different threads --- lib/api/src/rt/v8/entities/function/env.rs | 4 + lib/api/src/rt/v8/entities/global.rs | 48 ++++++++++-- lib/api/src/rt/v8/entities/instance.rs | 18 ++++- lib/api/src/rt/v8/entities/memory/mod.rs | 85 +++++++++++++++++++--- lib/api/src/rt/v8/entities/module.rs | 17 +++-- lib/api/src/rt/v8/entities/store/mod.rs | 12 ++- lib/api/src/rt/v8/entities/table.rs | 66 ++++++++++++++--- 7 files changed, 214 insertions(+), 36 deletions(-) diff --git a/lib/api/src/rt/v8/entities/function/env.rs b/lib/api/src/rt/v8/entities/function/env.rs index 5e44eb6ea99..23f11e65828 100644 --- a/lib/api/src/rt/v8/entities/function/env.rs +++ b/lib/api/src/rt/v8/entities/function/env.rs @@ -160,6 +160,10 @@ impl AsStoreRef for FunctionEnvMut<'_, T> { inner: self.store_mut.inner, } } + + fn get_rt_kind(&self) -> crate::Runtime { + crate::Runtime::V8 + } } impl AsStoreMut for FunctionEnvMut<'_, T> { diff --git a/lib/api/src/rt/v8/entities/global.rs b/lib/api/src/rt/v8/entities/global.rs index f446d77063f..5bb53e62e53 100644 --- a/lib/api/src/rt/v8/entities/global.rs +++ b/lib/api/src/rt/v8/entities/global.rs @@ -45,6 +45,11 @@ impl Global { mutability: Mutability, ) -> Result { let store = store.as_store_mut(); + let v8_store = store.inner.store.as_v8(); + + if v8_store.thread_id != std::thread::current().id() { + return Err(RuntimeError::new("Cannot create a new global from value: current thread is different from the thread the store was created in!")); + } let v8_type = val.ty().into_ct(); let v8_value = val.into_cv(); @@ -58,13 +63,18 @@ impl Global { unsafe { wasm_globaltype_new(wasm_valtype_new(v8_type), v8_mutability) }; Ok(Self { - handle: unsafe { - wasm_global_new(store.inner.store.as_v8().inner, v8_global_type, &v8_value) - }, + handle: unsafe { wasm_global_new(v8_store.inner, v8_global_type, &v8_value) }, }) } - pub fn ty(&self, _store: &impl AsStoreRef) -> GlobalType { + pub fn ty(&self, store: &impl AsStoreRef) -> GlobalType { + let store = store.as_store_ref(); + let v8_store = store.inner.store.as_v8(); + + if v8_store.thread_id != std::thread::current().id() { + panic!("Cannot get the global's type: current thread is different from the thread the store was created in!"); + } + let r#type = unsafe { wasm_global_type(self.handle) }; let mutability = unsafe { wasm_globaltype_mutability(&*r#type) }; let valtype = unsafe { wasm_globaltype_content(r#type) }; @@ -81,12 +91,26 @@ impl Global { } pub fn get(&self, store: &mut impl AsStoreMut) -> Value { + let store = store.as_store_ref(); + let v8_store = store.inner.store.as_v8(); + + if v8_store.thread_id != std::thread::current().id() { + panic!("Cannot get the global: current thread is different from the thread the store was created in!"); + } + let mut out = unsafe { std::mem::zeroed() }; unsafe { wasm_global_get(self.handle, &mut out) }; out.into_wv() } pub fn set(&self, store: &mut impl AsStoreMut, val: Value) -> Result<(), RuntimeError> { + let _store = store.as_store_ref(); + let v8_store = _store.inner.store.as_v8(); + + if v8_store.thread_id != std::thread::current().id() { + return Err(RuntimeError::new("Cannot set the global: current thread is different from the thread the store was created in!".to_owned())); + } + if val.ty() != self.ty(store).ty { return Err(RuntimeError::new(format!( "Incompatible types: {} != {}", @@ -107,12 +131,26 @@ impl Global { } pub(crate) fn from_vm_extern(store: &mut impl AsStoreMut, vm_global: VMExternGlobal) -> Self { + let _store = store.as_store_ref(); + let v8_store = _store.inner.store.as_v8(); + + if v8_store.thread_id != std::thread::current().id() { + panic!("Cannot create a new global from vm extern: current thread is different from the thread the store was created in!"); + } + Self { handle: vm_global.into_v8(), } } - pub fn is_from_store(&self, _store: &impl AsStoreRef) -> bool { + pub fn is_from_store(&self, store: &impl AsStoreRef) -> bool { + let store = store.as_store_ref(); + let v8_store = store.inner.store.as_v8(); + + if v8_store.thread_id != std::thread::current().id() { + panic!("Cannot check if global is from store: current thread is different from the thread the store was created in!"); + } + true } } diff --git a/lib/api/src/rt/v8/entities/instance.rs b/lib/api/src/rt/v8/entities/instance.rs index d25ebb79d5a..d02d11f14be 100644 --- a/lib/api/src/rt/v8/entities/instance.rs +++ b/lib/api/src/rt/v8/entities/instance.rs @@ -15,7 +15,7 @@ unsafe impl Sync for InstanceHandle {} impl InstanceHandle { fn new( store: *mut wasm_store_t, - module: *mut wasm_module_t, + module: *mut wasm_shared_module_t, mut externs: Vec, ) -> Result { let mut trap: *mut wasm_trap_t = std::ptr::null_mut() as _; @@ -26,7 +26,12 @@ impl InstanceHandle { let ptr = externs.as_ptr(); std::mem::forget(externs); - wasm_instance_new(store, module, ptr as *const *const _, &mut trap) + wasm_instance_new( + store, + wasm_module_obtain(store, module), + ptr as *const *const _, + &mut trap, + ) }; if instance.is_null() { @@ -82,6 +87,13 @@ impl Instance { module: &Module, imports: &Imports, ) -> Result<(Self, Exports), InstantiationError> { + let mut store = store.as_store_mut(); + let v8_store = store.inner.store.as_v8(); + + if v8_store.thread_id != std::thread::current().id() { + panic!("Cannot create new instance: current thread is different from the thread the store was created in!"); + } + let externs = module .imports() .map(|import_ty| { @@ -91,7 +103,7 @@ impl Instance { }) .collect::>(); - return Self::new_by_index(store, module, &externs); + return Self::new_by_index(&mut store, module, &externs); } pub(crate) fn new_by_index( diff --git a/lib/api/src/rt/v8/entities/memory/mod.rs b/lib/api/src/rt/v8/entities/memory/mod.rs index 4643d2b5054..3307a1ea415 100644 --- a/lib/api/src/rt/v8/entities/memory/mod.rs +++ b/lib/api/src/rt/v8/entities/memory/mod.rs @@ -26,6 +26,13 @@ unsafe impl Sync for Memory {} impl Memory { pub fn new(store: &mut impl AsStoreMut, ty: MemoryType) -> Result { + let mut store_mut = store.as_store_mut(); + let v8_store = store_mut.inner.store.as_v8(); + + if v8_store.thread_id != std::thread::current().id() { + panic!("Cannot create memory: current thread is different from the thread the store was created in!"); + } + let limits = Box::into_raw(Box::new(wasm_limits_t { min: ty.minimum.0, max: match ty.maximum { @@ -35,15 +42,18 @@ impl Memory { })); let memorytype = unsafe { wasm_memorytype_new(limits) }; - - let mut store = store.as_store_mut(); - let inner = store.inner.store.as_v8().inner; - let c_memory = unsafe { wasm_memory_new(inner, memorytype) }; + let c_memory = unsafe { wasm_memory_new(v8_store.inner, memorytype) }; Ok(Self { handle: c_memory }) } pub fn new_from_existing(new_store: &mut impl AsStoreMut, memory: VMMemory) -> Self { + let store_mut = new_store.as_store_mut(); + let v8_store = store_mut.inner.store.as_v8(); + + if v8_store.thread_id != std::thread::current().id() { + panic!("Cannot create memory from existing: current thread is different from the thread the store was created in!"); + } Self { handle: memory } } @@ -51,20 +61,33 @@ impl Memory { VMExtern::V8(unsafe { wasm_memory_as_extern(self.handle) }) } - pub fn ty(&self, _store: &impl AsStoreRef) -> MemoryType { + pub fn ty(&self, store: &impl AsStoreRef) -> MemoryType { + let store = store.as_store_ref(); + let v8_store = store.inner.store.as_v8(); + + if v8_store.thread_id != std::thread::current().id() { + panic!("Cannot get the memory's size: current thread is different from the thread the store was created in!"); + } + let memory_type: *mut wasm_memorytype_t = unsafe { wasm_memory_type(self.handle) }; let limits: *const wasm_limits_t = unsafe { wasm_memorytype_limits(memory_type) }; MemoryType { // [TODO]: Find a way to extract this from the inner memory type instead // of hardcoding. - shared: false, + shared: true, minimum: unsafe { wasmer_types::Pages((*limits).min) }, maximum: unsafe { Some(wasmer_types::Pages((*limits).max)) }, } } pub fn view<'a>(&self, store: &'a impl AsStoreRef) -> MemoryView<'a> { + let store_ref = store.as_store_ref(); + let v8_store = store_ref.inner.store.as_v8(); + + if v8_store.thread_id != std::thread::current().id() { + panic!("Cannot create memory view: current thread is different from the thread the store was created in!"); + } MemoryView::new(self, store) } @@ -77,6 +100,13 @@ impl Memory { where IntoPages: Into, { + let store_mut = store.as_store_mut(); + let v8_store = store_mut.inner.store.as_v8(); + + if v8_store.thread_id != std::thread::current().id() { + panic!("Cannot grow memory: current thread is different from the thread the store was created in!"); + } + unsafe { let delta: Pages = delta.into(); let current = Pages(wasm_memory_size(self.handle)); @@ -97,6 +127,13 @@ impl Memory { store: &mut impl AsStoreMut, min_size: u64, ) -> Result<(), MemoryError> { + let store_mut = store.as_store_mut(); + let v8_store = store_mut.inner.store.as_v8(); + + if v8_store.thread_id != std::thread::current().id() { + panic!("Cannot grow memory: current thread is different from the thread the store was created in!"); + } + unsafe { let current = wasm_memory_size(self.handle); let delta = (min_size as u32) - current; @@ -108,7 +145,13 @@ impl Memory { Ok(()) } - pub fn reset(&self, _store: &mut impl AsStoreMut) -> Result<(), MemoryError> { + pub fn reset(&self, store: &mut impl AsStoreMut) -> Result<(), MemoryError> { + let store_mut = store.as_store_mut(); + let v8_store = store_mut.inner.store.as_v8(); + + if v8_store.thread_id != std::thread::current().id() { + panic!("Cannot reset memory: current thread is different from the thread the store was created in!"); + } Ok(()) } @@ -140,6 +183,12 @@ impl Memory { } pub(crate) fn from_vm_extern(store: &mut impl AsStoreMut, internal: VMExternMemory) -> Self { + let store_ref = store.as_store_ref(); + let v8_store = store_ref.inner.store.as_v8(); + + if v8_store.thread_id != std::thread::current().id() { + panic!("Cannot create memory from vm extern: current thread is different from the thread the store was created in!"); + } Self { handle: internal.into_v8(), } @@ -147,13 +196,25 @@ impl Memory { /// Cloning memory will create another reference to the same memory that /// can be put into a new store - pub fn try_clone(&self, _store: &impl AsStoreRef) -> Result { + pub fn try_clone(&self, store: &impl AsStoreRef) -> Result { + let store_ref = store.as_store_ref(); + let v8_store = store_ref.inner.store.as_v8(); + + if v8_store.thread_id != std::thread::current().id() { + panic!("Cannot clone memory: current thread is different from the thread the store was created in!"); + } Ok(self.handle.clone()) } /// Copying the memory will actually copy all the bytes in the memory to /// a identical byte copy of the original that can be put into a new store pub fn try_copy(&self, store: &impl AsStoreRef) -> Result { + let store_ref = store.as_store_ref(); + let v8_store = store_ref.inner.store.as_v8(); + + if v8_store.thread_id != std::thread::current().id() { + panic!("Cannot copy memory: current thread is different from the thread the store was created in!"); + } let res = unsafe { wasm_memory_copy(self.handle) }; if res.is_null() { Err(MemoryError::Generic("memory copy failed".to_owned())) @@ -162,7 +223,13 @@ impl Memory { } } - pub fn is_from_store(&self, _store: &impl AsStoreRef) -> bool { + pub fn is_from_store(&self, store: &impl AsStoreRef) -> bool { + let store_ref = store.as_store_ref(); + let v8_store = store_ref.inner.store.as_v8(); + + if v8_store.thread_id != std::thread::current().id() { + panic!("Cannot check if memory is from store: current thread is different from the thread the store was created in!"); + } true } diff --git a/lib/api/src/rt/v8/entities/module.rs b/lib/api/src/rt/v8/entities/module.rs index a0fc06aeb15..b780afbbfbe 100644 --- a/lib/api/src/rt/v8/entities/module.rs +++ b/lib/api/src/rt/v8/entities/module.rs @@ -1,10 +1,7 @@ //! Data types, functions and traits for `v8` runtime's `Module` implementation. use std::{path::Path, sync::Arc}; -use crate::{ - rt::v8::bindings::{wasm_byte_vec_t, wasm_module_delete, wasm_module_new, wasm_module_t}, - AsEngineRef, IntoBytes, RuntimeModule, -}; +use crate::{rt::v8::bindings::*, AsEngineRef, IntoBytes, RuntimeModule}; use bytes::Bytes; use wasmer_types::{ @@ -12,8 +9,9 @@ use wasmer_types::{ GlobalType, ImportType, ImportsIterator, MemoryType, ModuleInfo, Mutability, Pages, SerializeError, TableType, Type, }; + pub(crate) struct ModuleHandle { - pub(crate) inner: *mut wasm_module_t, + pub(crate) inner: *mut wasm_shared_module_t, pub(crate) store: std::sync::Mutex, } @@ -35,7 +33,12 @@ impl ModuleHandle { let store = crate::store::Store::new(engine.as_engine_ref().engine().clone()); - let inner = unsafe { wasm_module_new(store.inner.store.as_v8().inner, &bytes as *const _) }; + let inner = unsafe { + wasm_module_share(wasm_module_new( + store.inner.store.as_v8().inner, + &bytes as *const _, + )) + }; let store = std::sync::Mutex::new(store); if inner.is_null() { @@ -47,7 +50,7 @@ impl ModuleHandle { } impl Drop for ModuleHandle { fn drop(&mut self) { - unsafe { wasm_module_delete(self.inner) } + unsafe { wasm_shared_module_delete(self.inner) } } } diff --git a/lib/api/src/rt/v8/entities/store/mod.rs b/lib/api/src/rt/v8/entities/store/mod.rs index 90937284282..2abe0eab415 100644 --- a/lib/api/src/rt/v8/entities/store/mod.rs +++ b/lib/api/src/rt/v8/entities/store/mod.rs @@ -1,4 +1,6 @@ //! Data types, functions and traits for `v8` runtime's `Store` implementation. +use std::thread::ThreadId; + use crate::{ engine::{AsEngineRef, Engine, EngineRef}, rt::v8::bindings::{wasm_store_delete, wasm_store_new, wasm_store_t}, @@ -12,6 +14,7 @@ pub use obj::*; pub struct Store { pub(crate) engine: Engine, pub(crate) inner: *mut wasm_store_t, + pub(crate) thread_id: ThreadId, } impl std::fmt::Debug for Store { @@ -25,7 +28,12 @@ impl std::fmt::Debug for Store { impl Store { pub(crate) fn new(engine: crate::engine::Engine) -> Self { let inner: *mut wasm_store_t = unsafe { wasm_store_new(engine.as_v8().inner.engine) }; - Store { inner, engine } + let thread_id = std::thread::current().id(); + Store { + inner, + engine, + thread_id, + } } pub(crate) fn engine(&self) -> &Engine { @@ -39,7 +47,7 @@ impl Store { impl Drop for Store { fn drop(&mut self) { - unsafe { wasm_store_delete(self.inner) } + // unsafe { wasm_store_delete(self.inner) } } } diff --git a/lib/api/src/rt/v8/entities/table.rs b/lib/api/src/rt/v8/entities/table.rs index 59ae0c78d98..0470cd58000 100644 --- a/lib/api/src/rt/v8/entities/table.rs +++ b/lib/api/src/rt/v8/entities/table.rs @@ -41,19 +41,19 @@ impl Table { init: Value, ) -> Result { let store_mut = store.as_store_mut(); + let v8_store = store_mut.inner.store.as_v8(); + + if v8_store.thread_id != std::thread::current().id() { + panic!("Cannot create new table: current thread is different from the thread the store was created in!"); + } + let engine = store_mut.engine(); let wasm_tablety = Self::type_to_v8(ty); let init: wasm_val_t = init.into_cv(); Ok(Self { - handle: unsafe { - wasm_table_new( - store_mut.inner.store.as_v8().inner, - wasm_tablety, - init.of.ref_, - ) - }, + handle: unsafe { wasm_table_new(v8_store.inner, wasm_tablety, init.of.ref_) }, }) } @@ -61,7 +61,14 @@ impl Table { VMExtern::V8(unsafe { wasm_table_as_extern(self.handle) }) } - pub fn ty(&self, _store: &impl AsStoreRef) -> TableType { + pub fn ty(&self, store: &impl AsStoreRef) -> TableType { + let store = store.as_store_ref(); + let v8_store = store.inner.store.as_v8(); + + if v8_store.thread_id != std::thread::current().id() { + panic!("Cannot get the table's type: current thread is different from the thread the store was created in!"); + } + let table_type: *mut wasm_tabletype_t = unsafe { wasm_table_type(self.handle) }; let table_limits = unsafe { wasm_tabletype_limits(table_type) }; let table_type = unsafe { wasm_tabletype_element(table_type) }; @@ -80,6 +87,13 @@ impl Table { } pub fn get(&self, store: &mut impl AsStoreMut, index: u32) -> Option { + let store_mut = store.as_store_mut(); + let v8_store = store_mut.inner.store.as_v8(); + + if v8_store.thread_id != std::thread::current().id() { + panic!("Cannot get table's element: current thread is different from the thread the store was created in!"); + } + unsafe { let ref_ = wasm_table_get(self.handle, index); @@ -108,6 +122,13 @@ impl Table { index: u32, val: Value, ) -> Result<(), RuntimeError> { + let store_mut = store.as_store_mut(); + let v8_store = store_mut.inner.store.as_v8(); + + if v8_store.thread_id != std::thread::current().id() { + panic!("Cannot create new table: current thread is different from the thread the store was created in!"); + } + unsafe { let init = match val { Value::ExternRef(None) | Value::FuncRef(None) => std::ptr::null_mut(), @@ -130,6 +151,12 @@ impl Table { } pub fn size(&self, store: &impl AsStoreRef) -> u32 { + let store = store.as_store_ref(); + let v8_store = store.inner.store.as_v8(); + + if v8_store.thread_id != std::thread::current().id() { + panic!("Cannot get the table's size: current thread is different from the thread the store was created in!"); + } unsafe { wasm_table_size(self.handle) } } @@ -139,6 +166,13 @@ impl Table { delta: u32, init: Value, ) -> Result { + let store_mut = store.as_store_mut(); + let v8_store = store_mut.inner.store.as_v8(); + + if v8_store.thread_id != std::thread::current().id() { + panic!("Cannot grow table: current thread is different from the thread the store was created in!"); + } + unsafe { let size = wasm_table_size(self.handle); let init = match init { @@ -169,13 +203,25 @@ impl Table { unimplemented!("Copying tables is currently not implemented!") } - pub(crate) fn from_vm_extern(_store: &mut impl AsStoreMut, vm_extern: VMExternTable) -> Self { + pub(crate) fn from_vm_extern(store: &mut impl AsStoreMut, vm_extern: VMExternTable) -> Self { + let store_mut = store.as_store_mut(); + let v8_store = store_mut.inner.store.as_v8(); + + if v8_store.thread_id != std::thread::current().id() { + panic!("Cannot create table from vm extern: current thread is different from the thread the store was created in!"); + } Self { handle: vm_extern.into_v8(), } } - pub fn is_from_store(&self, _store: &impl AsStoreRef) -> bool { + pub fn is_from_store(&self, store: &impl AsStoreRef) -> bool { + let store = store.as_store_ref(); + let v8_store = store.inner.store.as_v8(); + + if v8_store.thread_id != std::thread::current().id() { + panic!("Cannot check if table is from store: current thread is different from the thread the store was created in!"); + } true } } From f4cd60b5840937b7158960de7bd64f04c58bddde Mon Sep 17 00:00:00 2001 From: Edoardo Marangoni Date: Mon, 2 Dec 2024 15:23:43 +0100 Subject: [PATCH 03/19] fix(api/c_api): Take into account callbacks during function calls --- lib/api/src/rt/v8/entities/function/mod.rs | 72 ++++++++++++++++--- lib/api/src/rt/v8/entities/function/typed.rs | 20 ++++-- lib/api/src/rt/wamr/entities/function/mod.rs | 22 +++++- .../src/rt/wamr/entities/function/typed.rs | 24 +++++-- lib/api/src/rt/wasmi/entities/function/mod.rs | 23 +++++- .../src/rt/wasmi/entities/function/typed.rs | 24 +++++-- 6 files changed, 160 insertions(+), 25 deletions(-) diff --git a/lib/api/src/rt/v8/entities/function/mod.rs b/lib/api/src/rt/v8/entities/function/mod.rs index 35743965da2..17c3d7c99c2 100644 --- a/lib/api/src/rt/v8/entities/function/mod.rs +++ b/lib/api/src/rt/v8/entities/function/mod.rs @@ -82,6 +82,13 @@ impl Function { + Send + Sync, { + let mut store = store.as_store_mut(); + let v8_store = store.inner.store.as_v8(); + + if v8_store.thread_id != std::thread::current().id() { + panic!("Cannot create new function: current thread is different from the thread the store was created in!"); + } + let fn_ty: FunctionType = ty.into(); let params = fn_ty.params(); @@ -121,8 +128,7 @@ impl Function { ) }; - let mut store = store.as_store_mut(); - let inner = store.inner.store.as_v8().inner; + let inner = v8_store.inner; let callback: CCallback = make_fn_callback(&func, param_types.len()); @@ -159,6 +165,13 @@ impl Function { Args: WasmTypeList, Rets: WasmTypeList, { + let mut store = store.as_store_mut(); + let v8_store = store.inner.store.as_v8(); + + if v8_store.thread_id != std::thread::current().id() { + panic!("Cannot create new typed function: current thread is different from the thread the store was created in!"); + } + let mut param_types = Args::wasm_types() .into_iter() .map(|param| { @@ -190,8 +203,7 @@ impl Function { let wasm_functype = unsafe { wasm_functype_new(&mut wasm_param_types, &mut wasm_result_types) }; - let mut store = store.as_store_mut(); - let inner = store.inner.store.as_v8().inner; + let inner = v8_store.inner; let callback: CCallback = unsafe { std::mem::transmute(func.function_callback(crate::Runtime::V8).into_v8()) }; @@ -233,6 +245,13 @@ impl Function { Rets: WasmTypeList, T: Send + 'static, { + let mut store = store.as_store_mut(); + let v8_store = store.inner.store.as_v8(); + + if v8_store.thread_id != std::thread::current().id() { + panic!("Cannot create new typed function with env: current thread is different from the thread the store was created in!"); + } + let mut param_types = Args::wasm_types() .into_iter() .map(|param| { @@ -268,8 +287,7 @@ impl Function { ) }; - let mut store = store.as_store_mut(); - let inner = store.inner.store.as_v8().inner; + let inner = v8_store.inner; let callback: CCallback = unsafe { std::mem::transmute(func.function_callback(crate::Runtime::V8).into_v8()) }; @@ -300,7 +318,13 @@ impl Function { } } - pub fn ty(&self, _store: &impl AsStoreRef) -> FunctionType { + pub fn ty(&self, store: &impl AsStoreRef) -> FunctionType { + let store_ref = store.as_store_ref(); + let v8_store = store_ref.inner.store.as_v8(); + + if v8_store.thread_id != std::thread::current().id() { + panic!("Cannot get the function's type: current thread is different from the thread the store was created in!"); + } let type_ = unsafe { wasm_func_type(self.handle) }; let params: *const wasm_valtype_vec_t = unsafe { wasm_functype_params(type_) }; let returns: *const wasm_valtype_vec_t = unsafe { wasm_functype_results(type_) }; @@ -341,6 +365,11 @@ impl Function { params: &[Value], ) -> Result, RuntimeError> { let store_mut = store.as_store_mut(); + let v8_store = store_mut.inner.store.as_v8(); + + if v8_store.thread_id != std::thread::current().id() { + return Err(RuntimeError::new("Cannot call function: current thread is different from the thread the store was created in!")); + } let mut args = { let params = params @@ -366,7 +395,34 @@ impl Function { ptr }; - let trap = unsafe { wasm_func_call(self.handle, args as *const _, results as *mut _) }; + let mut trap; + + loop { + trap = unsafe { wasm_func_call(self.handle, args as *const _, results) }; + let store_mut = store.as_store_mut(); + if let Some(callback) = store_mut.inner.on_called.take() { + match callback(store_mut) { + Ok(wasmer_types::OnCalledAction::InvokeAgain) => { + continue; + } + Ok(wasmer_types::OnCalledAction::Finish) => { + break; + } + Ok(wasmer_types::OnCalledAction::Trap(trap)) => { + return Err(RuntimeError::user(trap)) + } + Err(trap) => return Err(RuntimeError::user(trap)), + } + } + break; + } + + if !trap.is_null() { + unsafe { + let trap: Trap = trap.into(); + return Err(RuntimeError::from(trap)); + } + } if !trap.is_null() { return Err(Into::::into(trap).into()); diff --git a/lib/api/src/rt/v8/entities/function/typed.rs b/lib/api/src/rt/v8/entities/function/typed.rs index 0164aadac2b..759dfd01539 100644 --- a/lib/api/src/rt/v8/entities/function/typed.rs +++ b/lib/api/src/rt/v8/entities/function/typed.rs @@ -51,9 +51,21 @@ macro_rules! impl_native_traits { let func = unsafe { wasm_extern_as_func(self.func.to_vm_extern().into_v8()) }; - let trap = unsafe { - wasm_func_call(func, params_list.as_ptr() as *const _, results) - }; + let mut trap; + + loop { + trap = unsafe {wasm_func_call(func, params_list.as_ptr() as *const _, results)}; + let store_mut = store.as_store_mut(); + if let Some(callback) = store_mut.inner.on_called.take() { + match callback(store_mut) { + Ok(wasmer_types::OnCalledAction::InvokeAgain) => { continue; } + Ok(wasmer_types::OnCalledAction::Finish) => { break; } + Ok(wasmer_types::OnCalledAction::Trap(trap)) => { return Err(RuntimeError::user(trap)) }, + Err(trap) => { return Err(RuntimeError::user(trap)) }, + } + } + break; + } if !trap.is_null() { unsafe { @@ -62,7 +74,7 @@ macro_rules! impl_native_traits { } } - unsafe { + unsafe { let rets_len = Rets::wasm_types().len(); let mut results: *const [crate::rt::v8::bindings::wasm_val_t] = std::ptr::slice_from_raw_parts(results, rets_len); diff --git a/lib/api/src/rt/wamr/entities/function/mod.rs b/lib/api/src/rt/wamr/entities/function/mod.rs index 2f0f0215b69..42ed8c1f60e 100644 --- a/lib/api/src/rt/wamr/entities/function/mod.rs +++ b/lib/api/src/rt/wamr/entities/function/mod.rs @@ -373,7 +373,27 @@ impl Function { } }; - let trap = unsafe { wasm_func_call(self.handle, &mut args as _, &mut results as *mut _) }; + let mut trap; + + loop { + trap = unsafe { wasm_func_call(self.handle, &mut args as _, &mut results as *mut _) }; + let store_mut = store.as_store_mut(); + if let Some(callback) = store_mut.inner.on_called.take() { + match callback(store_mut) { + Ok(wasmer_types::OnCalledAction::InvokeAgain) => { + continue; + } + Ok(wasmer_types::OnCalledAction::Finish) => { + break; + } + Ok(wasmer_types::OnCalledAction::Trap(trap)) => { + return Err(RuntimeError::user(trap)) + } + Err(trap) => return Err(RuntimeError::user(trap)), + } + } + break; + } if !trap.is_null() { return Err(Into::::into(trap).into()); diff --git a/lib/api/src/rt/wamr/entities/function/typed.rs b/lib/api/src/rt/wamr/entities/function/typed.rs index 720a1d08791..841e58b16e9 100644 --- a/lib/api/src/rt/wamr/entities/function/typed.rs +++ b/lib/api/src/rt/wamr/entities/function/typed.rs @@ -52,14 +52,28 @@ macro_rules! impl_native_traits { let func = unsafe { wasm_extern_as_func(self.func.to_vm_extern().into_wamr()) }; - let trap = { - unsafe { - let mut params = std::mem::zeroed(); - wasm_val_vec_new(&mut params, params_list.len(), params_list.as_ptr()); - wasm_func_call(func, ¶ms, &mut results) + let mut params = unsafe {std::mem::zeroed()}; + unsafe {wasm_val_vec_new(&mut params, params_list.len(), params_list.as_ptr())}; + + let mut trap; + + loop { + trap = unsafe { wasm_func_call(func, ¶ms, &mut results) }; + + let store_mut = store.as_store_mut(); + if let Some(callback) = store_mut.inner.on_called.take() { + match callback(store_mut) { + Ok(wasmer_types::OnCalledAction::InvokeAgain) => { continue; } + Ok(wasmer_types::OnCalledAction::Finish) => { break; } + Ok(wasmer_types::OnCalledAction::Trap(trap)) => { return Err(RuntimeError::user(trap)) }, + Err(trap) => { return Err(RuntimeError::user(trap)) }, + } } + break; }; + + if !trap.is_null() { unsafe { let trap: Trap = trap.into(); diff --git a/lib/api/src/rt/wasmi/entities/function/mod.rs b/lib/api/src/rt/wasmi/entities/function/mod.rs index 82443a92607..7ea52ee3c0f 100644 --- a/lib/api/src/rt/wasmi/entities/function/mod.rs +++ b/lib/api/src/rt/wasmi/entities/function/mod.rs @@ -373,7 +373,28 @@ impl Function { } }; - let trap = unsafe { wasm_func_call(self.handle, &mut args as _, &mut results as *mut _) }; + let mut trap; + + loop { + trap = unsafe { wasm_func_call(self.handle, &mut args as _, &mut results as *mut _) }; + + let store_mut = store.as_store_mut(); + if let Some(callback) = store_mut.inner.on_called.take() { + match callback(store_mut) { + Ok(wasmer_types::OnCalledAction::InvokeAgain) => { + continue; + } + Ok(wasmer_types::OnCalledAction::Finish) => { + break; + } + Ok(wasmer_types::OnCalledAction::Trap(trap)) => { + return Err(RuntimeError::user(trap)) + } + Err(trap) => return Err(RuntimeError::user(trap)), + } + } + break; + } if !trap.is_null() { return Err(Into::::into(trap).into()); diff --git a/lib/api/src/rt/wasmi/entities/function/typed.rs b/lib/api/src/rt/wasmi/entities/function/typed.rs index 32c02a8f01c..a575df83704 100644 --- a/lib/api/src/rt/wasmi/entities/function/typed.rs +++ b/lib/api/src/rt/wasmi/entities/function/typed.rs @@ -50,14 +50,26 @@ macro_rules! impl_native_traits { }; - let func = unsafe { wasm_extern_as_func(self.func.to_vm_extern().into_wasmi()) }; + let func = unsafe { wasm_extern_as_func(self.func.to_vm_extern().into_wasmi()) }; - let trap = { - unsafe { - let mut params = std::mem::zeroed(); - wasm_val_vec_new(&mut params, params_list.len(), params_list.as_ptr()); - wasm_func_call(func, ¶ms, &mut results) + let mut params = unsafe {std::mem::zeroed()}; + unsafe {wasm_val_vec_new(&mut params, params_list.len(), params_list.as_ptr())}; + + let mut trap; + + loop { + trap = unsafe { wasm_func_call(func, ¶ms, &mut results) }; + + let store_mut = store.as_store_mut(); + if let Some(callback) = store_mut.inner.on_called.take() { + match callback(store_mut) { + Ok(wasmer_types::OnCalledAction::InvokeAgain) => { continue; } + Ok(wasmer_types::OnCalledAction::Finish) => { break; } + Ok(wasmer_types::OnCalledAction::Trap(trap)) => { return Err(RuntimeError::user(trap)) }, + Err(trap) => { return Err(RuntimeError::user(trap)) }, + } } + break; }; if !trap.is_null() { From 4a564b140e0ad2dc1311ed45331df5f82be47d24 Mon Sep 17 00:00:00 2001 From: Edoardo Marangoni Date: Wed, 4 Dec 2024 16:13:08 +0100 Subject: [PATCH 04/19] feat(api): Rename `Runtime` to `RuntimeKind` --- lib/api/src/entities/engine/inner.rs | 14 ++++---- lib/api/src/entities/engine/mod.rs | 4 +-- lib/api/src/entities/function/host/imp.rs | 36 +++++++++---------- lib/api/src/entities/function/host/mod.rs | 6 ++-- lib/api/src/rt/js/entities/function/mod.rs | 2 +- lib/api/src/rt/jsc/entities/function/mod.rs | 2 +- lib/api/src/rt/mod.rs | 2 +- lib/api/src/rt/sys/entities/function/mod.rs | 8 ++--- lib/api/src/rt/v8/entities/function/env.rs | 4 --- lib/api/src/rt/v8/entities/function/mod.rs | 4 +-- lib/api/src/rt/wamr/entities/function/mod.rs | 4 +-- lib/api/src/rt/wamr/entities/table.rs | 2 +- lib/api/src/rt/wasmi/entities/function/mod.rs | 4 +-- lib/api/src/rt/wasmi/entities/table.rs | 2 +- 14 files changed, 45 insertions(+), 49 deletions(-) diff --git a/lib/api/src/entities/engine/inner.rs b/lib/api/src/entities/engine/inner.rs index cd06e86c009..e5d642fcd54 100644 --- a/lib/api/src/entities/engine/inner.rs +++ b/lib/api/src/entities/engine/inner.rs @@ -14,20 +14,20 @@ gen_rt_ty!(Engine @derives Debug, Clone); impl RuntimeEngine { /// Returns the [`crate::Runtime`] kind this engine belongs to. - pub fn get_rt_kind(&self) -> crate::Runtime { + pub fn get_rt_kind(&self) -> crate::RuntimeKind { match self { #[cfg(feature = "sys")] - RuntimeEngine::Sys(_) => crate::Runtime::Sys, + RuntimeEngine::Sys(_) => crate::RuntimeKind::Sys, #[cfg(feature = "v8")] - RuntimeEngine::V8(_) => crate::Runtime::V8, + RuntimeEngine::V8(_) => crate::RuntimeKind::V8, #[cfg(feature = "wamr")] - RuntimeEngine::Wamr(_) => crate::Runtime::Wamr, + RuntimeEngine::Wamr(_) => crate::RuntimeKind::Wamr, #[cfg(feature = "wasmi")] - RuntimeEngine::Wasmi(_) => crate::Runtime::Wasmi, + RuntimeEngine::Wasmi(_) => crate::RuntimeKind::Wasmi, #[cfg(feature = "js")] - RuntimeEngine::Js(_) => crate::Runtime::Js, + RuntimeEngine::Js(_) => crate::RuntimeKind::Js, #[cfg(feature = "jsc")] - RuntimeEngine::Jsc(_) => crate::Runtime::Jsc, + RuntimeEngine::Jsc(_) => crate::RuntimeKind::Jsc, } } diff --git a/lib/api/src/entities/engine/mod.rs b/lib/api/src/entities/engine/mod.rs index 6befbaa1a08..3639e7c919b 100644 --- a/lib/api/src/entities/engine/mod.rs +++ b/lib/api/src/entities/engine/mod.rs @@ -48,8 +48,8 @@ impl Engine { ENGINE_ID_COUNTER.fetch_add(1, std::sync::atomic::Ordering::SeqCst) } - /// Returns the [`crate::Runtime`] kind this engine belongs to. - pub fn get_rt_kind(&self) -> crate::Runtime { + /// Returns the [`crate::RuntimeKind`] kind this engine belongs to. + pub fn get_rt_kind(&self) -> crate::RuntimeKind { self.rt.get_rt_kind() } diff --git a/lib/api/src/entities/function/host/imp.rs b/lib/api/src/entities/function/host/imp.rs index 8546af561e3..546416af905 100644 --- a/lib/api/src/entities/function/host/imp.rs +++ b/lib/api/src/entities/function/host/imp.rs @@ -21,31 +21,31 @@ impl< $( $x, )* Rets, RetsAsResult, T, Func> crate::HostFunction crate::vm::VMFunctionCallback { + fn function_callback(&self, rt: crate::rt::RuntimeKind) -> crate::vm::VMFunctionCallback { paste::paste!{ match rt { #[cfg(feature = "sys")] - crate::rt::Runtime::Sys => crate::vm::VMFunctionCallback::Sys(crate::rt::sys::function::[](self)), + crate::rt::RuntimeKind::Sys => crate::vm::VMFunctionCallback::Sys(crate::rt::sys::function::[](self)), #[cfg(feature = "js")] - crate::rt::Runtime::Js => crate::vm::VMFunctionCallback::Js(crate::rt::js::function::[](self)), + crate::rt::RuntimeKind::Js => crate::vm::VMFunctionCallback::Js(crate::rt::js::function::[](self)), #[cfg(feature = "jsc")] - crate::rt::Runtime::Jsc => crate::vm::VMFunctionCallback::Jsc(crate::rt::jsc::function::[](self)), + crate::rt::RuntimeKind::Jsc => crate::vm::VMFunctionCallback::Jsc(crate::rt::jsc::function::[](self)), #[cfg(feature = "wamr")] - crate::rt::Runtime::Wamr => crate::vm::VMFunctionCallback::Wamr(crate::rt::wamr::function::[](self)), + crate::rt::RuntimeKind::Wamr => crate::vm::VMFunctionCallback::Wamr(crate::rt::wamr::function::[](self)), #[cfg(feature = "wasmi")] - crate::rt::Runtime::Wasmi => crate::vm::VMFunctionCallback::Wasmi(crate::rt::wasmi::function::[](self)), + crate::rt::RuntimeKind::Wasmi => crate::vm::VMFunctionCallback::Wasmi(crate::rt::wasmi::function::[](self)), #[cfg(feature = "v8")] - crate::rt::Runtime::V8 => crate::vm::VMFunctionCallback::V8(crate::rt::v8::function::[](self)) + crate::rt::RuntimeKind::V8 => crate::vm::VMFunctionCallback::V8(crate::rt::v8::function::[](self)) } } } #[allow(non_snake_case)] - fn call_trampoline_address(rt: crate::rt::Runtime) -> crate::vm::VMTrampoline { + fn call_trampoline_address(rt: crate::rt::RuntimeKind) -> crate::vm::VMTrampoline { paste::paste!{ match rt { #[cfg(feature = "sys")] - crate::rt::Runtime::Sys => crate::vm::VMTrampoline::Sys(crate::rt::sys::function::[]::<$($x,)* Rets>()), + crate::rt::RuntimeKind::Sys => crate::vm::VMTrampoline::Sys(crate::rt::sys::function::[]::<$($x,)* Rets>()), _ => unimplemented!() } } @@ -68,31 +68,31 @@ where #[allow(non_snake_case)] - fn function_callback(&self, rt: crate::rt::Runtime) -> crate::vm::VMFunctionCallback { + fn function_callback(&self, rt: crate::rt::RuntimeKind) -> crate::vm::VMFunctionCallback { paste::paste!{ match rt { #[cfg(feature = "sys")] - crate::rt::Runtime::Sys => crate::vm::VMFunctionCallback::Sys(crate::rt::sys::function::[](self)), + crate::rt::RuntimeKind::Sys => crate::vm::VMFunctionCallback::Sys(crate::rt::sys::function::[](self)), #[cfg(feature = "js")] - crate::rt::Runtime::Js => crate::vm::VMFunctionCallback::Js(crate::rt::js::function::[](self)), + crate::rt::RuntimeKind::Js => crate::vm::VMFunctionCallback::Js(crate::rt::js::function::[](self)), #[cfg(feature = "jsc")] - crate::rt::Runtime::Jsc => crate::vm::VMFunctionCallback::Jsc(crate::rt::jsc::function::[](self)), + crate::rt::RuntimeKind::Jsc => crate::vm::VMFunctionCallback::Jsc(crate::rt::jsc::function::[](self)), #[cfg(feature = "wamr")] - crate::rt::Runtime::Wamr => crate::vm::VMFunctionCallback::Wamr(crate::rt::wamr::function::[](self)), + crate::rt::RuntimeKind::Wamr => crate::vm::VMFunctionCallback::Wamr(crate::rt::wamr::function::[](self)), #[cfg(feature = "wasmi")] - crate::rt::Runtime::Wasmi => crate::vm::VMFunctionCallback::Wasmi(crate::rt::wasmi::function::[](self)), + crate::rt::RuntimeKind::Wasmi => crate::vm::VMFunctionCallback::Wasmi(crate::rt::wasmi::function::[](self)), #[cfg(feature = "v8")] - crate::rt::Runtime::V8 => crate::vm::VMFunctionCallback::V8(crate::rt::v8::function::[](self)) + crate::rt::RuntimeKind::V8 => crate::vm::VMFunctionCallback::V8(crate::rt::v8::function::[](self)) } } } #[allow(non_snake_case)] - fn call_trampoline_address(rt: crate::rt::Runtime) -> crate::vm::VMTrampoline { + fn call_trampoline_address(rt: crate::rt::RuntimeKind) -> crate::vm::VMTrampoline { paste::paste!{ match rt { #[cfg(feature = "sys")] - crate::rt::Runtime::Sys => crate::vm::VMTrampoline::Sys(crate::rt::sys::function::[]::<$($x,)* Rets>()), + crate::rt::RuntimeKind::Sys => crate::vm::VMTrampoline::Sys(crate::rt::sys::function::[]::<$($x,)* Rets>()), _ => unimplemented!() } } diff --git a/lib/api/src/entities/function/host/mod.rs b/lib/api/src/entities/function/host/mod.rs index 6fc70f4bddb..d8a8f4a0de3 100644 --- a/lib/api/src/entities/function/host/mod.rs +++ b/lib/api/src/entities/function/host/mod.rs @@ -2,7 +2,7 @@ mod imp; use crate::{ vm::{VMFunctionCallback, VMTrampoline}, - Runtime, WasmTypeList, + RuntimeKind, WasmTypeList, }; /// The `HostFunction` trait represents the set of functions that @@ -16,10 +16,10 @@ where Kind: HostFunctionKind, { /// Get the pointer to the function body for a given runtime. - fn function_callback(&self, rt: Runtime) -> crate::vm::VMFunctionCallback; + fn function_callback(&self, rt: RuntimeKind) -> crate::vm::VMFunctionCallback; /// Get the pointer to the function call trampoline for a given runtime. - fn call_trampoline_address(rt: Runtime) -> crate::vm::VMTrampoline; + fn call_trampoline_address(rt: RuntimeKind) -> crate::vm::VMTrampoline; } /// Empty trait to specify the kind of `HostFunction`: With or diff --git a/lib/api/src/rt/js/entities/function/mod.rs b/lib/api/src/rt/js/entities/function/mod.rs index 01a9e99491b..42fce5429c1 100644 --- a/lib/api/src/rt/js/entities/function/mod.rs +++ b/lib/api/src/rt/js/entities/function/mod.rs @@ -320,7 +320,7 @@ where T: Sized, { Self { - address: function.function_callback(crate::Runtime::Js).into_js(), + address: function.function_callback(crate::RuntimeKind::Js).into_js(), _phantom: PhantomData, } } diff --git a/lib/api/src/rt/jsc/entities/function/mod.rs b/lib/api/src/rt/jsc/entities/function/mod.rs index 908bf2d25a1..e3a5b6e790c 100644 --- a/lib/api/src/rt/jsc/entities/function/mod.rs +++ b/lib/api/src/rt/jsc/entities/function/mod.rs @@ -313,7 +313,7 @@ where T: Sized, { Self { - callback: function.function_callback(crate::Runtime::Jsc).into_jsc(), + callback: function.function_callback(crate::RuntimeKind::Jsc).into_jsc(), _phantom: PhantomData, } } diff --git a/lib/api/src/rt/mod.rs b/lib/api/src/rt/mod.rs index 5fdd396cf53..25e137c8b36 100644 --- a/lib/api/src/rt/mod.rs +++ b/lib/api/src/rt/mod.rs @@ -21,7 +21,7 @@ pub mod jsc; #[derive(Debug, Clone, Copy)] /// An enumeration over all the supported runtimes. -pub enum Runtime { +pub enum RuntimeKind { #[cfg(feature = "sys")] /// The `sys` runtime. Sys, diff --git a/lib/api/src/rt/sys/entities/function/mod.rs b/lib/api/src/rt/sys/entities/function/mod.rs index 17624c828a2..2c34437c989 100644 --- a/lib/api/src/rt/sys/entities/function/mod.rs +++ b/lib/api/src/rt/sys/entities/function/mod.rs @@ -132,7 +132,7 @@ impl Function { Rets: WasmTypeList, { let env = FunctionEnv::new(store, ()); - let func_ptr = func.function_callback(crate::Runtime::Sys).into_sys(); + let func_ptr = func.function_callback(crate::RuntimeKind::Sys).into_sys(); let host_data = Box::new(StaticFunction { raw_store: store.as_store_mut().as_raw() as *mut u8, env, @@ -150,7 +150,7 @@ impl Function { }; let call_trampoline = >::call_trampoline_address( - crate::Runtime::Sys, + crate::RuntimeKind::Sys, ) .into_sys(); let anyfunc = VMCallerCheckedAnyfunc { @@ -181,7 +181,7 @@ impl Function { Args: WasmTypeList, Rets: WasmTypeList, { - let func_ptr = func.function_callback(crate::Runtime::Sys).into_sys(); + let func_ptr = func.function_callback(crate::RuntimeKind::Sys).into_sys(); let host_data = Box::new(StaticFunction { raw_store: store.as_store_mut().as_raw() as *mut u8, env: env.as_sys().clone().into(), @@ -198,7 +198,7 @@ impl Function { host_env: host_data.as_ref() as *const _ as *mut c_void, }; let call_trampoline = >::call_trampoline_address( - crate::Runtime::Sys, + crate::RuntimeKind::Sys, ) .into_sys(); let anyfunc = VMCallerCheckedAnyfunc { diff --git a/lib/api/src/rt/v8/entities/function/env.rs b/lib/api/src/rt/v8/entities/function/env.rs index 23f11e65828..5e44eb6ea99 100644 --- a/lib/api/src/rt/v8/entities/function/env.rs +++ b/lib/api/src/rt/v8/entities/function/env.rs @@ -160,10 +160,6 @@ impl AsStoreRef for FunctionEnvMut<'_, T> { inner: self.store_mut.inner, } } - - fn get_rt_kind(&self) -> crate::Runtime { - crate::Runtime::V8 - } } impl AsStoreMut for FunctionEnvMut<'_, T> { diff --git a/lib/api/src/rt/v8/entities/function/mod.rs b/lib/api/src/rt/v8/entities/function/mod.rs index 17c3d7c99c2..66130207850 100644 --- a/lib/api/src/rt/v8/entities/function/mod.rs +++ b/lib/api/src/rt/v8/entities/function/mod.rs @@ -206,7 +206,7 @@ impl Function { let inner = v8_store.inner; let callback: CCallback = - unsafe { std::mem::transmute(func.function_callback(crate::Runtime::V8).into_v8()) }; + unsafe { std::mem::transmute(func.function_callback(crate::RuntimeKind::V8).into_v8()) }; let mut callback_env: *mut FunctionCallbackEnv<'_, F> = Box::into_raw(Box::new(FunctionCallbackEnv { @@ -290,7 +290,7 @@ impl Function { let inner = v8_store.inner; let callback: CCallback = - unsafe { std::mem::transmute(func.function_callback(crate::Runtime::V8).into_v8()) }; + unsafe { std::mem::transmute(func.function_callback(crate::RuntimeKind::V8).into_v8()) }; let mut callback_env: *mut FunctionCallbackEnv<'_, F> = Box::into_raw(Box::new(FunctionCallbackEnv { diff --git a/lib/api/src/rt/wamr/entities/function/mod.rs b/lib/api/src/rt/wamr/entities/function/mod.rs index 42ed8c1f60e..a3c060b8ce9 100644 --- a/lib/api/src/rt/wamr/entities/function/mod.rs +++ b/lib/api/src/rt/wamr/entities/function/mod.rs @@ -198,7 +198,7 @@ impl Function { let inner = store.inner.store.as_wamr().inner; let callback: CCallback = unsafe { - std::mem::transmute(func.function_callback(crate::Runtime::Wamr).into_wamr()) + std::mem::transmute(func.function_callback(crate::RuntimeKind::Wamr).into_wamr()) }; let mut callback_env: *mut FunctionCallbackEnv<'_, F> = @@ -277,7 +277,7 @@ impl Function { let inner = store.inner.store.as_wamr().inner; let callback: CCallback = unsafe { - std::mem::transmute(func.function_callback(crate::Runtime::Wamr).into_wamr()) + std::mem::transmute(func.function_callback(crate::RuntimeKind::Wamr).into_wamr()) }; let mut callback_env: *mut FunctionCallbackEnv<'_, F> = diff --git a/lib/api/src/rt/wamr/entities/table.rs b/lib/api/src/rt/wamr/entities/table.rs index 98233dadd91..3470a5070a1 100644 --- a/lib/api/src/rt/wamr/entities/table.rs +++ b/lib/api/src/rt/wamr/entities/table.rs @@ -8,7 +8,7 @@ use crate::{ utils::convert::{IntoCApiType, IntoCApiValue, IntoWasmerType, IntoWasmerValue}, vm::VMTable, }, - AsStoreMut, AsStoreRef, Runtime, RuntimeError, RuntimeTable, Value, + AsStoreMut, AsStoreRef, RuntimeKind, RuntimeError, RuntimeTable, Value, }; #[derive(Debug, Clone, PartialEq, Eq)] diff --git a/lib/api/src/rt/wasmi/entities/function/mod.rs b/lib/api/src/rt/wasmi/entities/function/mod.rs index 7ea52ee3c0f..42042d5ed31 100644 --- a/lib/api/src/rt/wasmi/entities/function/mod.rs +++ b/lib/api/src/rt/wasmi/entities/function/mod.rs @@ -198,7 +198,7 @@ impl Function { let inner = store.inner.store.as_wasmi().inner; let callback: CCallback = unsafe { - std::mem::transmute(func.function_callback(crate::Runtime::Wasmi).into_wasmi()) + std::mem::transmute(func.function_callback(crate::RuntimeKind::Wasmi).into_wasmi()) }; let mut callback_env: *mut FunctionCallbackEnv<'_, F> = @@ -277,7 +277,7 @@ impl Function { let inner = store.inner.store.as_wasmi().inner; let callback: CCallback = unsafe { - std::mem::transmute(func.function_callback(crate::Runtime::Wasmi).into_wasmi()) + std::mem::transmute(func.function_callback(crate::RuntimeKind::Wasmi).into_wasmi()) }; let mut callback_env: *mut FunctionCallbackEnv<'_, F> = diff --git a/lib/api/src/rt/wasmi/entities/table.rs b/lib/api/src/rt/wasmi/entities/table.rs index dbb3286574a..64a0b1e136f 100644 --- a/lib/api/src/rt/wasmi/entities/table.rs +++ b/lib/api/src/rt/wasmi/entities/table.rs @@ -8,7 +8,7 @@ use crate::{ utils::convert::{IntoCApiType, IntoCApiValue, IntoWasmerType, IntoWasmerValue}, vm::VMTable, }, - AsStoreMut, AsStoreRef, Runtime, RuntimeError, RuntimeTable, Value, + AsStoreMut, AsStoreRef, RuntimeKind, RuntimeError, RuntimeTable, Value, }; #[derive(Debug, Clone, PartialEq, Eq)] From 86833eb688dc0ba2c5bfbf1dd5a4d81102bfe6fd Mon Sep 17 00:00:00 2001 From: Edoardo Marangoni Date: Wed, 4 Dec 2024 16:13:28 +0100 Subject: [PATCH 05/19] fix(api/v8): Better message when an export is not found --- lib/api/src/rt/v8/entities/instance.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/api/src/rt/v8/entities/instance.rs b/lib/api/src/rt/v8/entities/instance.rs index d02d11f14be..6fb4e4710f0 100644 --- a/lib/api/src/rt/v8/entities/instance.rs +++ b/lib/api/src/rt/v8/entities/instance.rs @@ -99,7 +99,7 @@ impl Instance { .map(|import_ty| { imports .get_export(import_ty.module(), import_ty.name()) - .expect("Extern not found") + .expect(format!("Export {import_ty:?} not found").as_str()) }) .collect::>(); From fe17f5781ec56b553b8db1b9a585a58772246709 Mon Sep 17 00:00:00 2001 From: Edoardo Marangoni Date: Thu, 5 Dec 2024 12:31:40 +0100 Subject: [PATCH 06/19] fix(api): Make `StoreInner` sync --- lib/api/src/entities/store/inner.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/api/src/entities/store/inner.rs b/lib/api/src/entities/store/inner.rs index 77948c5c580..3dfd42ba9f2 100644 --- a/lib/api/src/entities/store/inner.rs +++ b/lib/api/src/entities/store/inner.rs @@ -19,6 +19,9 @@ pub(crate) struct StoreInner { pub(crate) on_called: Option, } +unsafe impl Send for StoreInner {} +unsafe impl Sync for StoreInner {} + impl std::fmt::Debug for StoreInner { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { f.debug_struct("StoreInner") From 221837375e1697649b4364d730ce45a32d170caa Mon Sep 17 00:00:00 2001 From: Edoardo Marangoni Date: Thu, 5 Dec 2024 12:33:55 +0100 Subject: [PATCH 07/19] fix(cli): Move `StoreOptions` to `RuntimeOptions` ...since it's not necessarily creating a store anymore. --- lib/cli-compiler/src/commands/validate.rs | 2 +- lib/cli/src/commands/compile.rs | 7 +- lib/cli/src/commands/create_exe.rs | 2 +- lib/cli/src/commands/create_obj.rs | 6 +- lib/cli/src/commands/gen_c_header.rs | 2 +- lib/cli/src/commands/inspect.rs | 7 +- lib/cli/src/commands/run/mod.rs | 37 ++++---- lib/cli/src/commands/validate.rs | 7 +- lib/cli/src/commands/wast.rs | 2 +- lib/cli/src/lib.rs | 2 +- lib/cli/src/{store.rs => rt.rs} | 101 +++++++--------------- 11 files changed, 64 insertions(+), 111 deletions(-) rename lib/cli/src/{store.rs => rt.rs} (85%) diff --git a/lib/cli-compiler/src/commands/validate.rs b/lib/cli-compiler/src/commands/validate.rs index 9641fd82d70..8959cc81a57 100644 --- a/lib/cli-compiler/src/commands/validate.rs +++ b/lib/cli-compiler/src/commands/validate.rs @@ -13,7 +13,7 @@ pub struct Validate { path: PathBuf, #[clap(flatten)] - store: StoreOptions, + rt: RuntimeOptions, } impl Validate { diff --git a/lib/cli/src/commands/compile.rs b/lib/cli/src/commands/compile.rs index a70ffa56a38..2dc61a3ba3a 100644 --- a/lib/cli/src/commands/compile.rs +++ b/lib/cli/src/commands/compile.rs @@ -7,7 +7,7 @@ use wasmer::{ *, }; -use crate::{common::HashAlgorithm, store::StoreOptions, warning}; +use crate::{common::HashAlgorithm, rt::RuntimeOptions, warning}; #[derive(Debug, Parser)] /// The options for the `wasmer compile` subcommand @@ -25,7 +25,7 @@ pub struct Compile { target_triple: Option, #[clap(flatten)] - store: StoreOptions, + rt: RuntimeOptions, #[clap(short = 'm')] cpu_features: Vec, @@ -60,7 +60,8 @@ impl Compile { Target::new(target_triple.clone(), features) }) .unwrap_or_default(); - let (mut store, compiler_type) = self.store.get_store_for_target(target.clone())?; + let compiler_type = self.rt.get_rt()?; + let mut store = self.rt.get_store_for_target(target.clone())?; let engine = store.engine_mut(); let hash_algorithm = self.hash_algorithm.unwrap_or_default().into(); diff --git a/lib/cli/src/commands/create_exe.rs b/lib/cli/src/commands/create_exe.rs index c72287ae961..c26767ad0f3 100644 --- a/lib/cli/src/commands/create_exe.rs +++ b/lib/cli/src/commands/create_exe.rs @@ -5,7 +5,7 @@ use super::CliCommand; use crate::{ common::{normalize_path, HashAlgorithm}, config::WasmerEnv, - store::RuntimeOptions, + rt::RuntimeOptions, }; use anyhow::{anyhow, bail, Context, Result}; use clap::Parser; diff --git a/lib/cli/src/commands/create_obj.rs b/lib/cli/src/commands/create_obj.rs index 515e320c029..47defa929d4 100644 --- a/lib/cli/src/commands/create_obj.rs +++ b/lib/cli/src/commands/create_obj.rs @@ -8,7 +8,7 @@ use clap::Parser; use wasmer::sys::*; use wasmer_package::utils::from_disk; -use crate::store::RuntimeOptions; +use crate::rt::RuntimeOptions; #[derive(Debug, Parser)] /// The options for the `wasmer create-exe` subcommand @@ -82,10 +82,10 @@ impl CreateObj { ); let (_, compiler_type) = self.rt.get_store_for_target(target.clone())?; match compiler_type { - crate::store::RuntimeType::V8 | crate::store::RuntimeType::Wamr => { + crate::rt::RuntimeType::V8 | crate::rt::RuntimeType::Wamr => { anyhow::bail!("Cannot produce objects with {compiler_type}!") } - crate::store::RuntimeType::Headless => todo!(), + crate::rt::RuntimeType::Headless => todo!(), _ => {} } println!("Compiler: {}", compiler_type); diff --git a/lib/cli/src/commands/gen_c_header.rs b/lib/cli/src/commands/gen_c_header.rs index 895f3be376a..be40428eb59 100644 --- a/lib/cli/src/commands/gen_c_header.rs +++ b/lib/cli/src/commands/gen_c_header.rs @@ -14,7 +14,7 @@ use wasmer_package::{package::WasmerPackageError, utils::from_bytes}; use wasmer_types::MetadataHeader; use webc::{compat::SharedBytes, Container, ContainerError, DetectError}; -use crate::store::RuntimeOptions; +use crate::rt::RuntimeOptions; #[derive(Debug, Parser)] /// The options for the `wasmer gen-c-header` subcommand diff --git a/lib/cli/src/commands/inspect.rs b/lib/cli/src/commands/inspect.rs index 80b4d05abd9..a3b78e6b8a8 100644 --- a/lib/cli/src/commands/inspect.rs +++ b/lib/cli/src/commands/inspect.rs @@ -1,12 +1,11 @@ use std::path::PathBuf; +use crate::rt::RuntimeOptions; use anyhow::{Context, Result}; use bytesize::ByteSize; use clap::Parser; use wasmer::*; -use crate::store::StoreOptions; - #[derive(Debug, Parser)] /// The options for the `wasmer validate` subcommand pub struct Inspect { @@ -15,7 +14,7 @@ pub struct Inspect { path: PathBuf, #[clap(flatten)] - store: StoreOptions, + rt: RuntimeOptions, } impl Inspect { @@ -26,7 +25,7 @@ impl Inspect { } fn inner_execute(&self) -> Result<()> { - let (store, _compiler_type) = self.store.get_store()?; + let store = self.rt.get_store()?; let module_contents = std::fs::read(&self.path)?; let iswasm = is_wasm(&module_contents); diff --git a/lib/cli/src/commands/run/mod.rs b/lib/cli/src/commands/run/mod.rs index 07628e0f135..83640b825f7 100644 --- a/lib/cli/src/commands/run/mod.rs +++ b/lib/cli/src/commands/run/mod.rs @@ -58,7 +58,7 @@ use webc::Container; use crate::{ commands::run::wasi::Wasi, common::HashAlgorithm, config::WasmerEnv, error::PrettyError, - logging::Output, store::StoreOptions, + logging::Output, rt::RuntimeOptions, }; const TICK: Duration = Duration::from_millis(250); @@ -69,7 +69,7 @@ pub struct Run { #[clap(flatten)] env: WasmerEnv, #[clap(flatten)] - store: StoreOptions, + rt: RuntimeOptions, #[clap(flatten)] wasi: crate::commands::run::Wasi, #[clap(flatten)] @@ -128,19 +128,15 @@ impl Run { let _guard = handle.enter(); - let (mut store, _) = self.store.get_store()?; - - #[cfg(feature = "sys")] - if self.stack_size.is_some() { - wasmer_vm::set_stack_size(self.stack_size.unwrap()); - } - - let engine = store.engine_mut(); + let mut engine = self.rt.get_engine()?; let rt_kind = engine.get_rt_kind(); tracing::info!("Executing on runtime {rt_kind:?}"); #[cfg(feature = "sys")] if engine.is_sys() { + if self.stack_size.is_some() { + wasmer_vm::set_stack_size(self.stack_size.unwrap()); + } let hash_algorithm = self.hash_algorithm.unwrap_or_default().into(); engine.set_hash_algorithm(Some(hash_algorithm)); } @@ -180,7 +176,7 @@ impl Run { module, module_hash, path, - } => self.execute_wasm(&path, &module, module_hash, store, runtime.clone()), + } => self.execute_wasm(&path, &module, module_hash, runtime.clone()), ExecutableTarget::Package(pkg) => self.execute_webc(&pkg, runtime.clone()), } }; @@ -205,13 +201,12 @@ impl Run { path: &Path, module: &Module, module_hash: ModuleHash, - mut store: Store, runtime: Arc, ) -> Result<(), Error> { if wasmer_wasix::is_wasi_module(module) || wasmer_wasix::is_wasix_module(module) { - self.execute_wasi_module(path, module, module_hash, runtime, store) + self.execute_wasi_module(path, module, module_hash, runtime) } else { - self.execute_pure_wasm_module(module, &mut store) + self.execute_pure_wasm_module(module) } } @@ -362,9 +357,12 @@ impl Run { } #[tracing::instrument(skip_all)] - fn execute_pure_wasm_module(&self, module: &Module, store: &mut Store) -> Result<(), Error> { + fn execute_pure_wasm_module(&self, module: &Module) -> Result<(), Error> { + /// The rest of the execution happens in the main thread, so we can create the + /// store here. + let mut store = self.rt.get_store()?; let imports = Imports::default(); - let instance = Instance::new(store, module, &imports) + let instance = Instance::new(&mut store, module, &imports) .context("Unable to instantiate the WebAssembly module")?; let entry_function = match &self.invoke { @@ -379,7 +377,7 @@ impl Run { } }; - let return_values = invoke_function(&instance, store, entry_function, &self.args)?; + let return_values = invoke_function(&instance, &mut store, entry_function, &self.args)?; println!( "{}", @@ -450,7 +448,6 @@ impl Run { module: &Module, module_hash: ModuleHash, runtime: Arc, - mut store: Store, ) -> Result<(), Error> { let program_name = wasm_path.display().to_string(); @@ -500,10 +497,10 @@ impl Run { bail!("Wasmer binfmt interpreter needs at least three arguments (including $0) - must be registered as binfmt interpreter with the CFP flags. (Got arguments: {:?})", argv); } }; - let store = StoreOptions::default(); + let rt = RuntimeOptions::default(); Ok(Run { env: WasmerEnv::default(), - store, + rt, wasi: Wasi::for_binfmt_interpreter()?, wcgi: WcgiOptions::default(), stack_size: None, diff --git a/lib/cli/src/commands/validate.rs b/lib/cli/src/commands/validate.rs index 8806ca49f17..a9a3837d9f4 100644 --- a/lib/cli/src/commands/validate.rs +++ b/lib/cli/src/commands/validate.rs @@ -4,8 +4,7 @@ use anyhow::{bail, Context, Result}; use clap::Parser; use wasmer::*; -use crate::store::StoreOptions; - +use crate::rt::RuntimeOptions; #[derive(Debug, Parser)] /// The options for the `wasmer validate` subcommand pub struct Validate { @@ -14,7 +13,7 @@ pub struct Validate { path: PathBuf, #[clap(flatten)] - store: StoreOptions, + rt: RuntimeOptions, } impl Validate { @@ -24,7 +23,7 @@ impl Validate { .context(format!("failed to validate `{}`", self.path.display())) } fn inner_execute(&self) -> Result<()> { - let (store, _compiler_type) = self.store.get_store()?; + let store = self.rt.get_store()?; let module_contents = std::fs::read(&self.path)?; if !is_wasm(&module_contents) { diff --git a/lib/cli/src/commands/wast.rs b/lib/cli/src/commands/wast.rs index 63a78d60572..ad0287c4f6f 100644 --- a/lib/cli/src/commands/wast.rs +++ b/lib/cli/src/commands/wast.rs @@ -6,7 +6,7 @@ use clap::Parser; use wasmer::sys::engine::NativeEngineExt; use wasmer_wast::Wast as WastSpectest; -use crate::{common::HashAlgorithm, store::StoreOptions}; +use crate::{common::HashAlgorithm, rt::StoreOptions}; #[derive(Debug, Parser)] /// The options for the `wasmer wast` subcommand diff --git a/lib/cli/src/lib.rs b/lib/cli/src/lib.rs index a7bae62da71..3d67705468c 100644 --- a/lib/cli/src/lib.rs +++ b/lib/cli/src/lib.rs @@ -27,7 +27,7 @@ mod error; mod c_gen; mod logging; mod opts; -mod store; +mod rt; mod types; mod utils; diff --git a/lib/cli/src/store.rs b/lib/cli/src/rt.rs similarity index 85% rename from lib/cli/src/store.rs rename to lib/cli/src/rt.rs index 4ee1adc5458..bf89c64848e 100644 --- a/lib/cli/src/store.rs +++ b/lib/cli/src/rt.rs @@ -2,7 +2,7 @@ //! commands. // NOTE: A lot of this code depends on feature flags. -// To not go crazy with annotations, some lints are disablefor the whole +// To not go crazy with annotations, some lints are disabled for the whole // module. #![allow(dead_code, unused_imports, unused_variables)] @@ -20,13 +20,6 @@ use wasmer_compiler::CompilerConfig; use wasmer::Engine; -#[derive(Debug, Clone, clap::Parser, Default)] -/// The compiler options -pub struct StoreOptions { - #[clap(flatten)] - rt: RuntimeOptions, -} - #[derive(Debug, clap::Parser, Clone, Default)] /// The WebAssembly features that can be passed through the /// Command Line args. @@ -65,32 +58,32 @@ pub struct WasmFeatures { pub struct RuntimeOptions { /// Use Singlepass compiler. #[cfg(feature = "singlepass")] - #[clap(long, conflicts_with_all = &["cranelift", "llvm", "v8", "wamr", "wasmi"])] + #[clap(long, conflicts_with_all = &["cranelift", "v8", "wamr", "wasmi"])] singlepass: bool, /// Use Cranelift compiler. #[cfg(feature = "cranelift")] - #[clap(long, conflicts_with_all = &["singlepass", "llvm", "v8", "wamr", "wasmi"])] + #[clap(long, conflicts_with_all = &["v8", "wamr", "wasmi"])] cranelift: bool, /// Use LLVM compiler. #[cfg(feature = "llvm")] - #[clap(long, conflicts_with_all = &["singlepass", "cranelift", "v8", "wamr", "wasmi"])] + #[clap(long, conflicts_with_all = &["cranelift", "v8", "wamr", "wasmi"])] llvm: bool, /// Use the V8 runtime. #[cfg(feature = "v8")] - #[clap(long, conflicts_with_all = &["singlepass", "cranelift", "llvm", "wamr", "wasmi"])] + #[clap(long, conflicts_with_all = &["cranelift", "wamr", "wasmi"])] v8: bool, /// Use WAMR. #[cfg(feature = "wamr")] - #[clap(long, conflicts_with_all = &["singlepass", "cranelift", "llvm", "v8", "wasmi"])] + #[clap(long, conflicts_with_all = &[ "cranelift", "v8", "wasmi"])] wamr: bool, /// Use the wasmi runtime. #[cfg(feature = "wasmi")] - #[clap(long, conflicts_with_all = &["singlepass", "cranelift", "llvm", "v8", "wamr"])] + #[clap(long, conflicts_with_all = &[ "cranelift", "v8", "wamr"])] wasmi: bool, /// Enable compiler internal verification. @@ -110,7 +103,7 @@ pub struct RuntimeOptions { } impl RuntimeOptions { - fn get_rt(&self) -> Result { + pub fn get_rt(&self) -> Result { #[cfg(feature = "cranelift")] { if self.cranelift { @@ -168,11 +161,21 @@ impl RuntimeOptions { } else if #[cfg(feature = "wasmi")] { Ok(RuntimeType::Wasmi) } else { - bail!("There are no available compilers for your architecture"); + bail!("There are no available runtimes for your architecture"); } } } + pub fn get_store(&self) -> Result { + let target = Target::default(); + self.get_store_for_target(target) + } + + pub fn get_engine(&self) -> Result { + let target = Target::default(); + self.get_engine_for_target(target) + } + #[cfg(feature = "compiler")] /// Get the enaled Wasm features. pub fn get_features(&self, mut features: Features) -> Result { @@ -198,16 +201,21 @@ impl RuntimeOptions { } /// Gets the Store for a given target. - #[cfg(feature = "compiler")] - pub fn get_store_for_target(&self, target: Target) -> Result<(Store, RuntimeType)> { + pub fn get_store_for_target(&self, target: Target) -> Result { let rt = self.get_rt()?; - let engine = self.get_engine(target, &rt)?; + let engine = self.get_engine_for_target_and_rt(target, &rt)?; let store = Store::new(engine); - Ok((store, rt)) + Ok(store) } #[cfg(feature = "compiler")] - fn get_engine(&self, target: Target, rt: &RuntimeType) -> Result { + pub fn get_engine_for_target(&self, target: Target) -> Result { + let rt = self.get_rt()?; + self.get_engine_for_target_and_rt(target, &rt) + } + + #[cfg(feature = "compiler")] + fn get_engine_for_target_and_rt(&self, target: Target, rt: &RuntimeType) -> Result { match rt { RuntimeType::V8 => { #[cfg(feature = "v8")] @@ -457,54 +465,3 @@ impl std::fmt::Display for RuntimeType { ) } } - -#[cfg(feature = "compiler")] -impl StoreOptions { - /// Gets the store for the host target, with the compiler name selected - pub fn get_store(&self) -> Result<(Store, RuntimeType)> { - let target = Target::default(); - self.get_store_for_target(target) - } - - /// Gets the store for a given target, with the compiler name selected. - pub fn get_store_for_target(&self, target: Target) -> Result<(Store, RuntimeType)> { - let rt = self.rt.get_rt()?; - let engine = self.rt.get_engine(target, &rt)?; - let store = Store::new(engine); - Ok((store, rt)) - } -} - -// If we don't have a compiler, but we have an engine -#[cfg(not(any( - feature = "compiler", - feature = "jsc", - feature = "wamr", - feature = "v8", - feature = "wasmi" -)))] -impl StoreOptions { - fn get_engine_headless(&self) -> Result { - let engine: wasmer_compiler::Engine = wasmer_compiler::EngineBuilder::headless().engine(); - Ok(engine) - } - - /// Get the store (headless engine) - pub fn get_store(&self) -> Result<(Store, RuntimeType)> { - let engine = self.get_engine_headless()?; - let store = Store::new(engine); - Ok((store, RuntimeType::Headless)) - } -} - -#[cfg(all( - not(feature = "compiler"), - any(feature = "jsc", feature = "wamr", feature = "wasmi", feature = "v8") -))] -impl StoreOptions { - /// Get the store (headless engine) - pub fn get_store(&self) -> Result<(Store, RuntimeType)> { - let store = Store::default(); - Ok((store, RuntimeType::Headless)) - } -} From 9221246415585a939966ee1297f12293141dec5a Mon Sep 17 00:00:00 2001 From: Edoardo Marangoni Date: Thu, 5 Dec 2024 12:34:39 +0100 Subject: [PATCH 08/19] fix(wasix): Add better message when a non-errno error happens --- lib/wasix/src/bin_factory/exec.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/wasix/src/bin_factory/exec.rs b/lib/wasix/src/bin_factory/exec.rs index 2f14afe4c78..cb05d3504eb 100644 --- a/lib/wasix/src/bin_factory/exec.rs +++ b/lib/wasix/src/bin_factory/exec.rs @@ -25,7 +25,6 @@ use crate::{Runtime, WasiEnv, WasiFunctionEnv}; pub async fn spawn_exec( binary: BinaryPackage, name: &str, - _store: Store, env: WasiEnv, runtime: &Arc, ) -> Result { @@ -324,7 +323,14 @@ fn call_module( }; let code = if let Err(err) = &ret { - err.as_exit_code().unwrap_or_else(|| Errno::Noexec.into()) + match err.as_exit_code() { + Some(s) => s, + None => { + error!("{err}"); + eprintln!("{err}"); + Errno::Noexec.into() + } + } } else { Errno::Success.into() }; From c04fffcb479400a9cd5e3084df43c3392076f730 Mon Sep 17 00:00:00 2001 From: Edoardo Marangoni Date: Thu, 5 Dec 2024 12:41:59 +0100 Subject: [PATCH 09/19] fix: Remove top level constructions of `stores` --- lib/wasix/src/bin_factory/mod.rs | 6 +-- .../src/os/command/builtins/cmd_wasmer.rs | 11 ++---- lib/wasix/src/os/command/mod.rs | 6 +-- lib/wasix/src/os/console/mod.rs | 3 +- lib/wasix/src/runners/wasi.rs | 4 +- lib/wasix/src/syscalls/wasix/proc_exec2.rs | 19 ++-------- lib/wasix/src/syscalls/wasix/proc_spawn.rs | 38 ++++++++----------- 7 files changed, 30 insertions(+), 57 deletions(-) diff --git a/lib/wasix/src/bin_factory/mod.rs b/lib/wasix/src/bin_factory/mod.rs index 88ab7fb0d14..808296cb690 100644 --- a/lib/wasix/src/bin_factory/mod.rs +++ b/lib/wasix/src/bin_factory/mod.rs @@ -70,7 +70,6 @@ impl BinFactory { pub fn spawn<'a>( &'a self, name: String, - store: wasmer::Store, env: WasiEnv, ) -> Pin> + 'a>> { Box::pin(async move { @@ -111,7 +110,7 @@ impl BinFactory { env.prepare_spawn(cmd); - spawn_exec(pkg, name.as_str(), store, env, &self.runtime).await + spawn_exec(pkg, name.as_str(), env, &self.runtime).await } } }) @@ -121,7 +120,6 @@ impl BinFactory { &self, name: String, parent_ctx: Option<&FunctionEnvMut<'_, WasiEnv>>, - store: &mut Option, builder: &mut Option, ) -> Result { // We check for built in commands @@ -129,7 +127,7 @@ impl BinFactory { if self.commands.exists(name.as_str()) { return self .commands - .exec(parent_ctx, name.as_str(), store, builder); + .exec(parent_ctx, name.as_str(), builder); } } else if self.commands.exists(name.as_str()) { tracing::warn!("builtin command without a parent ctx - {}", name); diff --git a/lib/wasix/src/os/command/builtins/cmd_wasmer.rs b/lib/wasix/src/os/command/builtins/cmd_wasmer.rs index ad861477911..b5ea2b78444 100644 --- a/lib/wasix/src/os/command/builtins/cmd_wasmer.rs +++ b/lib/wasix/src/os/command/builtins/cmd_wasmer.rs @@ -7,7 +7,7 @@ use crate::{ SpawnError, }; use virtual_fs::{AsyncReadExt, FileSystem}; -use wasmer::{FunctionEnvMut, Store}; +use wasmer::FunctionEnvMut; use wasmer_package::utils::from_bytes; use wasmer_wasix_types::wasi::Errno; @@ -55,7 +55,6 @@ impl CmdWasmer { &self, parent_ctx: &FunctionEnvMut<'a, WasiEnv>, name: &str, - store: &mut Option, config: &mut Option, what: Option, mut args: Vec, @@ -71,7 +70,6 @@ impl CmdWasmer { } if let Some(what) = what { - let store = store.take().ok_or(SpawnError::UnknownError)?; let mut env = config.take().ok_or(SpawnError::UnknownError)?; // Set the arguments of the environment by replacing the state @@ -136,7 +134,7 @@ impl CmdWasmer { env.use_package_async(&binary).await.unwrap(); // Now run the module - spawn_exec(binary, name, store, env, &self.runtime).await + spawn_exec(binary, name, env, &self.runtime).await } Executable::Wasm(bytes) => spawn_exec_wasm(&bytes, name, env, &self.runtime).await, } @@ -177,7 +175,6 @@ impl VirtualCommand for CmdWasmer { &self, parent_ctx: &FunctionEnvMut<'_, WasiEnv>, name: &str, - store: &mut Option, env: &mut Option, ) -> Result { // Read the command we want to run @@ -193,7 +190,7 @@ impl VirtualCommand for CmdWasmer { Some("run") => { let what = args.next().map(|a| a.to_string()); let args = args.map(|a| a.to_string()).collect(); - self.run(parent_ctx, name, store, env, what, args).await + self.run(parent_ctx, name, env, what, args).await } Some("--help") | None => { unsafe { stderr_write(parent_ctx, HELP.as_bytes()) } @@ -206,7 +203,7 @@ impl VirtualCommand for CmdWasmer { Some(what) => { let what = Some(what.to_string()); let args = args.map(|a| a.to_string()).collect(); - self.run(parent_ctx, name, store, env, what, args).await + self.run(parent_ctx, name, env, what, args).await } } }; diff --git a/lib/wasix/src/os/command/mod.rs b/lib/wasix/src/os/command/mod.rs index d5ecc590b95..af7b981c2e9 100644 --- a/lib/wasix/src/os/command/mod.rs +++ b/lib/wasix/src/os/command/mod.rs @@ -2,7 +2,7 @@ pub mod builtins; use std::{collections::HashMap, sync::Arc}; -use wasmer::{FunctionEnvMut, Store}; +use wasmer::FunctionEnvMut; use wasmer_wasix_types::wasi::Errno; use crate::{ @@ -27,7 +27,6 @@ where &self, parent_ctx: &FunctionEnvMut<'_, WasiEnv>, path: &str, - store: &mut Option, config: &mut Option, ) -> Result; } @@ -86,12 +85,11 @@ impl Commands { &self, parent_ctx: &FunctionEnvMut<'_, WasiEnv>, path: &str, - store: &mut Option, builder: &mut Option, ) -> Result { let path = path.to_string(); if let Some(cmd) = self.commands.get(&path) { - cmd.exec(parent_ctx, path.as_str(), store, builder) + cmd.exec(parent_ctx, path.as_str(), builder) } else { unsafe { InlineWaker::block_on(stderr_write( diff --git a/lib/wasix/src/os/console/mod.rs b/lib/wasix/src/os/console/mod.rs index fb91145e375..cd454eb1cb4 100644 --- a/lib/wasix/src/os/console/mod.rs +++ b/lib/wasix/src/os/console/mod.rs @@ -278,8 +278,7 @@ impl Console { // Build the config // Run the binary - let store = self.runtime.new_store(); - let process = InlineWaker::block_on(spawn_exec(pkg, prog, store, env, &self.runtime))?; + let process = InlineWaker::block_on(spawn_exec(pkg, prog, env, &self.runtime))?; // Return the process Ok((process, wasi_process)) diff --git a/lib/wasix/src/runners/wasi.rs b/lib/wasix/src/runners/wasi.rs index 445dea45818..bee52ce5fe0 100644 --- a/lib/wasix/src/runners/wasi.rs +++ b/lib/wasix/src/runners/wasi.rs @@ -383,8 +383,6 @@ impl crate::runners::Runner for WasiRunner { } let env = env.build()?; - let store = runtime.new_store(); - let command_name = command_name.to_string(); let tasks = runtime.task_manager().clone(); let pkg = pkg.clone(); @@ -392,7 +390,7 @@ impl crate::runners::Runner for WasiRunner { let exit_code = tasks.spawn_and_block_on( async move { let mut task_handle = - crate::bin_factory::spawn_exec(pkg, &command_name, store, env, &runtime) + crate::bin_factory::spawn_exec(pkg, &command_name, env, &runtime) .await .context("Spawn failed")?; diff --git a/lib/wasix/src/syscalls/wasix/proc_exec2.rs b/lib/wasix/src/syscalls/wasix/proc_exec2.rs index 66fda51e1a2..f5ca13e3ebc 100644 --- a/lib/wasix/src/syscalls/wasix/proc_exec2.rs +++ b/lib/wasix/src/syscalls/wasix/proc_exec2.rs @@ -100,8 +100,6 @@ pub fn proc_exec2( } }; - let new_store = ctx.data().runtime.new_store(); - // If we are in a vfork we need to first spawn a subprocess of this type // with the forked WasiEnv, then do a longjmp back to the vfork point. if let Some(mut vfork) = ctx.data_mut().vfork.take() { @@ -128,22 +126,20 @@ pub fn proc_exec2( let bin_factory = Box::new(ctx.data().bin_factory.clone()); let tasks = wasi_env.tasks().clone(); - let mut new_store = Some(new_store); let mut config = Some(wasi_env); - match bin_factory.try_built_in(name.clone(), Some(&ctx), &mut new_store, &mut config) { + match bin_factory.try_built_in(name.clone(), Some(&ctx), &mut config) { Ok(a) => {} Err(err) => { if !err.is_not_found() { error!("builtin failed - {}", err); } - let new_store = new_store.take().unwrap(); let env = config.take().unwrap(); let name_inner = name.clone(); __asyncify_light(ctx.data(), None, async { - let ret = bin_factory.spawn(name_inner, new_store, env).await; + let ret = bin_factory.spawn(name_inner, env).await; match ret { Ok(ret) => { trace!(%child_pid, "spawned sub-process"); @@ -220,26 +216,19 @@ pub fn proc_exec2( // Create the process and drop the context let bin_factory = Box::new(ctx.data().bin_factory.clone()); - let mut new_store = Some(new_store); let mut builder = Some(wasi_env); - let process = match bin_factory.try_built_in( - name.clone(), - Some(&ctx), - &mut new_store, - &mut builder, - ) { + let process = match bin_factory.try_built_in(name.clone(), Some(&ctx), &mut builder) { Ok(a) => Ok(a), Err(err) => { if !err.is_not_found() { error!("builtin failed - {}", err); } - let new_store = new_store.take().unwrap(); let env = builder.take().unwrap(); // Spawn a new process with this current execution environment - InlineWaker::block_on(bin_factory.spawn(name, new_store, env)) + InlineWaker::block_on(bin_factory.spawn(name, env)) } }; diff --git a/lib/wasix/src/syscalls/wasix/proc_spawn.rs b/lib/wasix/src/syscalls/wasix/proc_spawn.rs index 1c45c75d2ee..4342c3f9056 100644 --- a/lib/wasix/src/syscalls/wasix/proc_spawn.rs +++ b/lib/wasix/src/syscalls/wasix/proc_spawn.rs @@ -103,9 +103,6 @@ pub fn proc_spawn_internal( ) -> WasiResult<(ProcessHandles, FunctionEnvMut<'_, WasiEnv>)> { let env = ctx.data(); - // Build a new store that will be passed to the thread - let new_store = ctx.data().runtime.new_store(); - // Fork the current environment and set the new arguments let (mut child_env, handle) = match ctx.data().fork() { Ok(x) => x, @@ -219,30 +216,27 @@ pub fn proc_spawn_internal( let bin_factory = Box::new(ctx.data().bin_factory.clone()); let child_pid = child_env.pid(); - let mut new_store = Some(new_store); let mut builder = Some(child_env); // First we try the built in commands - let mut process = - match bin_factory.try_built_in(name.clone(), Some(&ctx), &mut new_store, &mut builder) { - Ok(a) => a, - Err(err) => { - if !err.is_not_found() { - error!("builtin failed - {}", err); - } - // Now we actually spawn the process - let child_work = - bin_factory.spawn(name, new_store.take().unwrap(), builder.take().unwrap()); + let mut process = match bin_factory.try_built_in(name.clone(), Some(&ctx), &mut builder) { + Ok(a) => a, + Err(err) => { + if !err.is_not_found() { + error!("builtin failed - {}", err); + } + // Now we actually spawn the process + let child_work = bin_factory.spawn(name, builder.take().unwrap()); - match __asyncify(&mut ctx, None, async move { Ok(child_work.await) })? - .map_err(|err| Errno::Unknown) - { - Ok(Ok(a)) => a, - Ok(Err(err)) => return Ok(Err(conv_spawn_err_to_errno(&err))), - Err(err) => return Ok(Err(err)), - } + match __asyncify(&mut ctx, None, async move { Ok(child_work.await) })? + .map_err(|err| Errno::Unknown) + { + Ok(Ok(a)) => a, + Ok(Err(err)) => return Ok(Err(conv_spawn_err_to_errno(&err))), + Err(err) => return Ok(Err(err)), } - }; + } + }; // Add the process to the environment state { From a352602c8291019d762058c90f405f2d306a867d Mon Sep 17 00:00:00 2001 From: Edoardo Marangoni Date: Thu, 5 Dec 2024 12:43:29 +0100 Subject: [PATCH 10/19] fix(wasix): Create store and instances in the executor thread Note: the removal of the reference for `StoreSnapshots` in `TaskWasm` in favour of an owned value is due to an initial work for making `TaskWasm` `Send` or not leaking references that outlive spawned threads. --- lib/wasix/src/runtime/task_manager/mod.rs | 10 +-- lib/wasix/src/runtime/task_manager/tokio.rs | 86 +++++++++++++++++--- lib/wasix/src/state/func_env.rs | 37 +++++++-- lib/wasix/src/syscalls/wasix/proc_fork.rs | 2 +- lib/wasix/src/syscalls/wasix/thread_spawn.rs | 2 +- 5 files changed, 110 insertions(+), 27 deletions(-) diff --git a/lib/wasix/src/runtime/task_manager/mod.rs b/lib/wasix/src/runtime/task_manager/mod.rs index a5d86f234a0..7a9c4f6a97f 100644 --- a/lib/wasix/src/runtime/task_manager/mod.rs +++ b/lib/wasix/src/runtime/task_manager/mod.rs @@ -69,18 +69,18 @@ pub struct TaskWasmRecycleProperties { pub type TaskWasmRecycle = dyn FnOnce(TaskWasmRecycleProperties) + Send + 'static; /// Represents a WASM task that will be executed on a dedicated thread -pub struct TaskWasm<'a, 'b> { +pub struct TaskWasm<'a> { pub run: Box, pub recycle: Option>, pub env: WasiEnv, pub module: Module, - pub globals: Option<&'b StoreSnapshot>, + pub globals: Option, pub spawn_type: SpawnMemoryType<'a>, pub trigger: Option>, pub update_layout: bool, } -impl<'a, 'b> TaskWasm<'a, 'b> { +impl<'a> TaskWasm<'a> { pub fn new(run: Box, env: WasiEnv, module: Module, update_layout: bool) -> Self { let shared_memory = module.imports().memories().next().map(|a| *a.ty()); Self { @@ -110,7 +110,7 @@ impl<'a, 'b> TaskWasm<'a, 'b> { self } - pub fn with_globals(mut self, snapshot: &'b StoreSnapshot) -> Self { + pub fn with_globals(mut self, snapshot: StoreSnapshot) -> Self { self.globals.replace(snapshot); self } @@ -372,7 +372,7 @@ impl dyn VirtualTaskManager { false, ) .with_memory(SpawnMemoryType::ShareMemory(memory, store.as_store_ref())) - .with_globals(&snapshot) + .with_globals(snapshot) .with_trigger(Box::new(move || { Box::pin(async move { let mut poller = AsyncifyPollerOwned { diff --git a/lib/wasix/src/runtime/task_manager/tokio.rs b/lib/wasix/src/runtime/task_manager/tokio.rs index c524134c6e0..884b3ccdea9 100644 --- a/lib/wasix/src/runtime/task_manager/tokio.rs +++ b/lib/wasix/src/runtime/task_manager/tokio.rs @@ -1,10 +1,11 @@ use std::sync::Mutex; use std::{num::NonZeroUsize, pin::Pin, sync::Arc, time::Duration}; +use crate::runtime::SpawnMemoryType; +use crate::{os::task::thread::WasiThreadError, WasiFunctionEnv}; use futures::{future::BoxFuture, Future}; use tokio::runtime::{Handle, Runtime}; - -use crate::{os::task::thread::WasiThreadError, WasiFunctionEnv}; +use wasmer::AsStoreMut; use super::{TaskWasm, TaskWasmRunProperties, VirtualTaskManager}; @@ -119,6 +120,19 @@ impl<'g> Drop for TokioRuntimeGuard<'g> { fn drop(&mut self) {} } +/// Describes whether a new memory should be created (and, in case, its type) or if it was already +/// created and the store it belongs to. +/// +/// # Note +/// +/// This type is necessary for now because we can't pass a [`wasmer::StoreRef`] between threads, so this +/// conceptually is a Send-able [`SpawnMemoryType`]. +pub enum SpawnMemoryTypeOrStore { + New, + Type(wasmer::MemoryType), + StoreAndMemory(wasmer::Store, Option), +} + impl VirtualTaskManager for TokioTaskManager { /// See [`VirtualTaskManager::sleep_now`]. fn sleep_now(&self, time: Duration) -> Pin + Send + Sync>> { @@ -149,18 +163,59 @@ impl VirtualTaskManager for TokioTaskManager { // Create the context on a new store let run = task.run; let recycle = task.recycle; - let (ctx, mut store) = WasiFunctionEnv::new_with_store( - task.module, - task.env, - task.globals, - task.spawn_type, - task.update_layout, - )?; + //let module = task.module; + let env = task.env; + //let globals = task.globals; + //let update_layout = task.update_layout; + + let make_memory: SpawnMemoryTypeOrStore = match task.spawn_type { + SpawnMemoryType::CreateMemory => SpawnMemoryTypeOrStore::New, + SpawnMemoryType::CreateMemoryOfType(t) => SpawnMemoryTypeOrStore::Type(t), + SpawnMemoryType::ShareMemory(_, _) | SpawnMemoryType::CopyMemory(_, _) => { + let mut store = env.runtime().new_store(); + let memory = self.build_memory(&mut store.as_store_mut(), task.spawn_type)?; + SpawnMemoryTypeOrStore::StoreAndMemory(store, memory) + } + }; - // If we have a trigger then we first need to run - // the poller to completion if let Some(trigger) = task.trigger { tracing::trace!("spawning task_wasm trigger in async pool"); + // In principle, we'd need to create this in the `pool.execute` function below, that is + // + // ``` + // 227: pool.execute(move || { + // ...: let (ctx, mut store) = WasiFunctionEnv::new_with_store( + // ...: ... + // ``` + // + // However, in the loop spawned below we need to have a `FunctionEnvMut`, which + // must be created with a mutable reference to the store. We can't, however since + // ``` + // pool.execute(move || { + // let (ctx, mut store) = WasiFunctionEnv::new_with_store( + // ... + // tx.send(store.as_store_mut()) + // ``` + // or + // ``` + // pool.execute(move || { + // let (ctx, mut store) = WasiFunctionEnv::new_with_store( + // ... + // tx.send(ctx.env.clone().into_mut(&mut store.as_store_mut())) + // ``` + // Since the reference would outlive the owned value. + // + // So, we create the store (and memory, and instance) outside the execution thread (the + // pool's one), and let it fail for runtimes that don't support entities created in a + // thread that's not the one in which execution happens in; this until we can clone + // stores. + let (ctx, mut store) = WasiFunctionEnv::new_with_store( + task.module, + env, + task.globals, + make_memory, + task.update_layout, + )?; let mut trigger = trigger(); let pool = self.pool.clone(); @@ -211,7 +266,14 @@ impl VirtualTaskManager for TokioTaskManager { // Run the callback on a dedicated thread self.pool.execute(move || { tracing::trace!("task_wasm started in blocking thread"); - + let (ctx, store) = WasiFunctionEnv::new_with_store( + task.module, + env, + task.globals, + make_memory, + task.update_layout, + ) + .unwrap(); // Invoke the callback run(TaskWasmRunProperties { ctx, diff --git a/lib/wasix/src/state/func_env.rs b/lib/wasix/src/state/func_env.rs index dfd50449723..eb9a4467097 100644 --- a/lib/wasix/src/state/func_env.rs +++ b/lib/wasix/src/state/func_env.rs @@ -12,7 +12,7 @@ use crate::os::task::thread::RewindResultType; use crate::syscalls::restore_snapshot; use crate::{ import_object_for_all_wasi_versions, - runtime::SpawnMemoryType, + runtime::task_manager::tokio::SpawnMemoryTypeOrStore, state::WasiInstanceHandles, utils::{get_wasi_version, get_wasi_versions, store::restore_store_snapshot}, RewindStateOption, StoreSnapshot, WasiEnv, WasiError, WasiRuntimeError, WasiThreadError, @@ -41,16 +41,37 @@ impl WasiFunctionEnv { pub fn new_with_store( module: Module, env: WasiEnv, - store_snapshot: Option<&StoreSnapshot>, - spawn_type: SpawnMemoryType, + store_snapshot: Option, + spawn_type: SpawnMemoryTypeOrStore, update_layout: bool, ) -> Result<(Self, Store), WasiThreadError> { // Create a new store and put the memory object in it // (but only if it has imported memory) - let mut store = env.runtime.new_store(); - let memory = env - .tasks() - .build_memory(&mut store.as_store_mut(), spawn_type)?; + let (memory, store): (Option, Option) = match spawn_type { + SpawnMemoryTypeOrStore::New => (None, None), + SpawnMemoryTypeOrStore::Type(mut ty) => { + ty.shared = true; + + let mut store = env.runtime.new_store(); + + // Note: If memory is shared, maximum needs to be set in the + // browser otherwise creation will fail. + let _ = ty.maximum.get_or_insert(wasmer_types::Pages::max_value()); + + let mem = Memory::new(&mut store, ty).map_err(|err| { + tracing::error!( + error = &err as &dyn std::error::Error, + memory_type=?ty, + "could not create memory", + ); + WasiThreadError::MemoryCreateFailed(err) + })?; + (Some(mem), Some(store)) + } + SpawnMemoryTypeOrStore::StoreAndMemory(s, m) => (m, Some(s)), + }; + + let mut store = store.unwrap_or_else(|| env.runtime().new_store()); // Build the context object and import the memory let mut ctx = WasiFunctionEnv::new(&mut store, env); @@ -79,7 +100,7 @@ impl WasiFunctionEnv { // Set all the globals if let Some(snapshot) = store_snapshot { - restore_store_snapshot(&mut store, snapshot); + restore_store_snapshot(&mut store, &snapshot); } Ok((ctx, store)) diff --git a/lib/wasix/src/syscalls/wasix/proc_fork.rs b/lib/wasix/src/syscalls/wasix/proc_fork.rs index 0761a2cf701..c1793f83678 100644 --- a/lib/wasix/src/syscalls/wasix/proc_fork.rs +++ b/lib/wasix/src/syscalls/wasix/proc_fork.rs @@ -200,7 +200,7 @@ pub fn proc_fork( tasks_outer .task_wasm( TaskWasm::new(Box::new(run), child_env, module, false) - .with_globals(&snapshot) + .with_globals(snapshot) .with_memory(spawn_type), ) .map_err(|err| { diff --git a/lib/wasix/src/syscalls/wasix/thread_spawn.rs b/lib/wasix/src/syscalls/wasix/thread_spawn.rs index 415e2be7949..56b99814c1c 100644 --- a/lib/wasix/src/syscalls/wasix/thread_spawn.rs +++ b/lib/wasix/src/syscalls/wasix/thread_spawn.rs @@ -166,7 +166,7 @@ pub fn thread_spawn_internal_using_layout( tasks .task_wasm( TaskWasm::new(Box::new(run), thread_env, thread_module, false) - .with_globals(&globals) + .with_globals(globals) .with_memory(spawn_type), ) .map_err(Into::::into)?; From 20c161a5e0306ceb9f10a4343f0d69e940615e6f Mon Sep 17 00:00:00 2001 From: Edoardo Marangoni Date: Thu, 5 Dec 2024 12:44:02 +0100 Subject: [PATCH 11/19] chore: Make linter happy --- lib/api/src/rt/jsc/entities/function/mod.rs | 4 +++- lib/api/src/rt/v8/entities/function/mod.rs | 10 ++++++---- lib/api/src/rt/v8/entities/store/mod.rs | 2 +- lib/api/src/rt/wamr/entities/table.rs | 2 +- lib/api/src/rt/wasmi/entities/function/mod.rs | 10 ++++++++-- lib/api/src/rt/wasmi/entities/table.rs | 2 +- lib/wasix/src/bin_factory/mod.rs | 4 +--- 7 files changed, 21 insertions(+), 13 deletions(-) diff --git a/lib/api/src/rt/jsc/entities/function/mod.rs b/lib/api/src/rt/jsc/entities/function/mod.rs index e3a5b6e790c..6ef80ad90a8 100644 --- a/lib/api/src/rt/jsc/entities/function/mod.rs +++ b/lib/api/src/rt/jsc/entities/function/mod.rs @@ -313,7 +313,9 @@ where T: Sized, { Self { - callback: function.function_callback(crate::RuntimeKind::Jsc).into_jsc(), + callback: function + .function_callback(crate::RuntimeKind::Jsc) + .into_jsc(), _phantom: PhantomData, } } diff --git a/lib/api/src/rt/v8/entities/function/mod.rs b/lib/api/src/rt/v8/entities/function/mod.rs index 66130207850..26bc3b024a6 100644 --- a/lib/api/src/rt/v8/entities/function/mod.rs +++ b/lib/api/src/rt/v8/entities/function/mod.rs @@ -205,8 +205,9 @@ impl Function { let inner = v8_store.inner; - let callback: CCallback = - unsafe { std::mem::transmute(func.function_callback(crate::RuntimeKind::V8).into_v8()) }; + let callback: CCallback = unsafe { + std::mem::transmute(func.function_callback(crate::RuntimeKind::V8).into_v8()) + }; let mut callback_env: *mut FunctionCallbackEnv<'_, F> = Box::into_raw(Box::new(FunctionCallbackEnv { @@ -289,8 +290,9 @@ impl Function { let inner = v8_store.inner; - let callback: CCallback = - unsafe { std::mem::transmute(func.function_callback(crate::RuntimeKind::V8).into_v8()) }; + let callback: CCallback = unsafe { + std::mem::transmute(func.function_callback(crate::RuntimeKind::V8).into_v8()) + }; let mut callback_env: *mut FunctionCallbackEnv<'_, F> = Box::into_raw(Box::new(FunctionCallbackEnv { diff --git a/lib/api/src/rt/v8/entities/store/mod.rs b/lib/api/src/rt/v8/entities/store/mod.rs index 2abe0eab415..fb391673fb9 100644 --- a/lib/api/src/rt/v8/entities/store/mod.rs +++ b/lib/api/src/rt/v8/entities/store/mod.rs @@ -47,7 +47,7 @@ impl Store { impl Drop for Store { fn drop(&mut self) { - // unsafe { wasm_store_delete(self.inner) } + // unsafe { wasm_store_delete(self.inner) } } } diff --git a/lib/api/src/rt/wamr/entities/table.rs b/lib/api/src/rt/wamr/entities/table.rs index 3470a5070a1..316a5304806 100644 --- a/lib/api/src/rt/wamr/entities/table.rs +++ b/lib/api/src/rt/wamr/entities/table.rs @@ -8,7 +8,7 @@ use crate::{ utils::convert::{IntoCApiType, IntoCApiValue, IntoWasmerType, IntoWasmerValue}, vm::VMTable, }, - AsStoreMut, AsStoreRef, RuntimeKind, RuntimeError, RuntimeTable, Value, + AsStoreMut, AsStoreRef, RuntimeError, RuntimeKind, RuntimeTable, Value, }; #[derive(Debug, Clone, PartialEq, Eq)] diff --git a/lib/api/src/rt/wasmi/entities/function/mod.rs b/lib/api/src/rt/wasmi/entities/function/mod.rs index 42042d5ed31..5584332ec50 100644 --- a/lib/api/src/rt/wasmi/entities/function/mod.rs +++ b/lib/api/src/rt/wasmi/entities/function/mod.rs @@ -198,7 +198,10 @@ impl Function { let inner = store.inner.store.as_wasmi().inner; let callback: CCallback = unsafe { - std::mem::transmute(func.function_callback(crate::RuntimeKind::Wasmi).into_wasmi()) + std::mem::transmute( + func.function_callback(crate::RuntimeKind::Wasmi) + .into_wasmi(), + ) }; let mut callback_env: *mut FunctionCallbackEnv<'_, F> = @@ -277,7 +280,10 @@ impl Function { let inner = store.inner.store.as_wasmi().inner; let callback: CCallback = unsafe { - std::mem::transmute(func.function_callback(crate::RuntimeKind::Wasmi).into_wasmi()) + std::mem::transmute( + func.function_callback(crate::RuntimeKind::Wasmi) + .into_wasmi(), + ) }; let mut callback_env: *mut FunctionCallbackEnv<'_, F> = diff --git a/lib/api/src/rt/wasmi/entities/table.rs b/lib/api/src/rt/wasmi/entities/table.rs index 64a0b1e136f..5dff49f5a26 100644 --- a/lib/api/src/rt/wasmi/entities/table.rs +++ b/lib/api/src/rt/wasmi/entities/table.rs @@ -8,7 +8,7 @@ use crate::{ utils::convert::{IntoCApiType, IntoCApiValue, IntoWasmerType, IntoWasmerValue}, vm::VMTable, }, - AsStoreMut, AsStoreRef, RuntimeKind, RuntimeError, RuntimeTable, Value, + AsStoreMut, AsStoreRef, RuntimeError, RuntimeKind, RuntimeTable, Value, }; #[derive(Debug, Clone, PartialEq, Eq)] diff --git a/lib/wasix/src/bin_factory/mod.rs b/lib/wasix/src/bin_factory/mod.rs index 808296cb690..21a6d2ba45a 100644 --- a/lib/wasix/src/bin_factory/mod.rs +++ b/lib/wasix/src/bin_factory/mod.rs @@ -125,9 +125,7 @@ impl BinFactory { // 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(), builder); + return self.commands.exec(parent_ctx, name.as_str(), builder); } } else if self.commands.exists(name.as_str()) { tracing::warn!("builtin command without a parent ctx - {}", name); From cff7a39dd6afc6ae09fd6e5a65fa7c2e19ec8b2e Mon Sep 17 00:00:00 2001 From: Edoardo Marangoni Date: Thu, 5 Dec 2024 12:46:25 +0100 Subject: [PATCH 12/19] fix(cli-compiler): Revert to `StoreOptions` in cli-compiler --- lib/cli-compiler/src/commands/validate.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cli-compiler/src/commands/validate.rs b/lib/cli-compiler/src/commands/validate.rs index 8959cc81a57..9641fd82d70 100644 --- a/lib/cli-compiler/src/commands/validate.rs +++ b/lib/cli-compiler/src/commands/validate.rs @@ -13,7 +13,7 @@ pub struct Validate { path: PathBuf, #[clap(flatten)] - rt: RuntimeOptions, + store: StoreOptions, } impl Validate { From 5277d33549fdd429d5f6b4e0ee39d35afb874faf Mon Sep 17 00:00:00 2001 From: Edoardo Marangoni Date: Thu, 5 Dec 2024 12:58:36 +0100 Subject: [PATCH 13/19] chore: Make linter happy --- lib/api/src/entities/engine/inner.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/api/src/entities/engine/inner.rs b/lib/api/src/entities/engine/inner.rs index e5d642fcd54..10dc131f611 100644 --- a/lib/api/src/entities/engine/inner.rs +++ b/lib/api/src/entities/engine/inner.rs @@ -17,17 +17,17 @@ impl RuntimeEngine { pub fn get_rt_kind(&self) -> crate::RuntimeKind { match self { #[cfg(feature = "sys")] - RuntimeEngine::Sys(_) => crate::RuntimeKind::Sys, + Self::Sys(_) => crate::RuntimeKind::Sys, #[cfg(feature = "v8")] - RuntimeEngine::V8(_) => crate::RuntimeKind::V8, + Self::V8(_) => crate::RuntimeKind::V8, #[cfg(feature = "wamr")] - RuntimeEngine::Wamr(_) => crate::RuntimeKind::Wamr, + Self::Wamr(_) => crate::RuntimeKind::Wamr, #[cfg(feature = "wasmi")] - RuntimeEngine::Wasmi(_) => crate::RuntimeKind::Wasmi, + Self::Wasmi(_) => crate::RuntimeKind::Wasmi, #[cfg(feature = "js")] - RuntimeEngine::Js(_) => crate::RuntimeKind::Js, + Self::Js(_) => crate::RuntimeKind::Js, #[cfg(feature = "jsc")] - RuntimeEngine::Jsc(_) => crate::RuntimeKind::Jsc, + Self::Jsc(_) => crate::RuntimeKind::Jsc, } } From c64f4215e64e52b18453e1a576f39063907074aa Mon Sep 17 00:00:00 2001 From: Edoardo Marangoni Date: Thu, 5 Dec 2024 12:58:53 +0100 Subject: [PATCH 14/19] fix(cli): Use appropriate calls to `RuntimeOptions` --- lib/cli/src/commands/create_exe.rs | 6 ++++-- lib/cli/src/commands/create_obj.rs | 2 +- lib/cli/src/commands/wast.rs | 9 +++++---- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/lib/cli/src/commands/create_exe.rs b/lib/cli/src/commands/create_exe.rs index c26767ad0f3..96e32c0ffd7 100644 --- a/lib/cli/src/commands/create_exe.rs +++ b/lib/cli/src/commands/create_exe.rs @@ -234,9 +234,9 @@ impl CliCommand for CreateExe { return Err(anyhow::anyhow!("input path cannot be a directory")); } - let (store, compiler_type) = self.compiler.get_store_for_target(target.clone())?; + let compiler_type = self.compiler.get_rt()?; + let mut engine = self.compiler.get_engine()?; - let mut engine = store.engine().clone(); let hash_algorithm = self.hash_algorithm.unwrap_or_default().into(); engine.set_hash_algorithm(Some(hash_algorithm)); @@ -283,6 +283,8 @@ impl CliCommand for CreateExe { ) }?; + let store = self.compiler.get_store()?; + get_module_infos(&store, &tempdir, &atoms)?; let mut entrypoint = get_entrypoint(&tempdir)?; create_header_files_in_dir(&tempdir, &mut entrypoint, &atoms, &self.precompiled_atom)?; diff --git a/lib/cli/src/commands/create_obj.rs b/lib/cli/src/commands/create_obj.rs index 47defa929d4..577491225dd 100644 --- a/lib/cli/src/commands/create_obj.rs +++ b/lib/cli/src/commands/create_obj.rs @@ -80,7 +80,7 @@ impl CreateObj { &target_triple, &self.cpu_features, ); - let (_, compiler_type) = self.rt.get_store_for_target(target.clone())?; + let compiler_type = self.rt.get_rt()?; match compiler_type { crate::rt::RuntimeType::V8 | crate::rt::RuntimeType::Wamr => { anyhow::bail!("Cannot produce objects with {compiler_type}!") diff --git a/lib/cli/src/commands/wast.rs b/lib/cli/src/commands/wast.rs index ad0287c4f6f..90269e1e488 100644 --- a/lib/cli/src/commands/wast.rs +++ b/lib/cli/src/commands/wast.rs @@ -6,7 +6,7 @@ use clap::Parser; use wasmer::sys::engine::NativeEngineExt; use wasmer_wast::Wast as WastSpectest; -use crate::{common::HashAlgorithm, rt::StoreOptions}; +use crate::{common::HashAlgorithm, rt::RuntimeOptions}; #[derive(Debug, Parser)] /// The options for the `wasmer wast` subcommand @@ -16,7 +16,7 @@ pub struct Wast { path: PathBuf, #[clap(flatten)] - store: StoreOptions, + rt: RuntimeOptions, #[clap(short, long)] /// A flag to indicate wast stop at the first error or continue. @@ -34,9 +34,10 @@ impl Wast { .context(format!("failed to test the wast `{}`", self.path.display())) } fn inner_execute(&self) -> Result<()> { - let (store, _compiler_name) = self.store.get_store()?; + let mut store = self.rt.get_store()?; + + let engine = store.engine_mut(); - let mut engine = store.engine().clone(); let hash_algorithm = self.hash_algorithm.unwrap_or_default().into(); engine.set_hash_algorithm(Some(hash_algorithm)); From c463031c981baf70994b9d856845934f93fe5d1e Mon Sep 17 00:00:00 2001 From: Edoardo Marangoni Date: Fri, 6 Dec 2024 08:42:52 +0100 Subject: [PATCH 15/19] fix(api): Remove unnecessary `Send` and `Sync` for `StoreInner` --- lib/api/src/entities/store/inner.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/api/src/entities/store/inner.rs b/lib/api/src/entities/store/inner.rs index 3dfd42ba9f2..77948c5c580 100644 --- a/lib/api/src/entities/store/inner.rs +++ b/lib/api/src/entities/store/inner.rs @@ -19,9 +19,6 @@ pub(crate) struct StoreInner { pub(crate) on_called: Option, } -unsafe impl Send for StoreInner {} -unsafe impl Sync for StoreInner {} - impl std::fmt::Debug for StoreInner { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { f.debug_struct("StoreInner") From 1aaf081e1803cd58712bfeae03e073484f506da4 Mon Sep 17 00:00:00 2001 From: Edoardo Marangoni Date: Fri, 6 Dec 2024 08:47:02 +0100 Subject: [PATCH 16/19] fix(api/js): Fix missing qualification --- lib/api/src/rt/js/entities/engine.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/api/src/rt/js/entities/engine.rs b/lib/api/src/rt/js/entities/engine.rs index 5fa5f124e27..285858ebfe6 100644 --- a/lib/api/src/rt/js/entities/engine.rs +++ b/lib/api/src/rt/js/entities/engine.rs @@ -47,7 +47,7 @@ impl crate::Engine { /// Return true if [`self`] is an engine from the `js` runtime. pub fn is_js(&self) -> bool { - matches!(self.rt, RuntimeEngine::Js(_)) + matches!(self.rt, crate::RuntimeEngine::Js(_)) } } From 89c4a4cd35a2c04dd6462a07e275293f89c1ade8 Mon Sep 17 00:00:00 2001 From: Edoardo Marangoni Date: Fri, 6 Dec 2024 08:58:21 +0100 Subject: [PATCH 17/19] fix(api/jsc): Fix missing qualification --- lib/api/src/rt/jsc/entities/engine.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/api/src/rt/jsc/entities/engine.rs b/lib/api/src/rt/jsc/entities/engine.rs index d6576fb5c24..ad59026f090 100644 --- a/lib/api/src/rt/jsc/entities/engine.rs +++ b/lib/api/src/rt/jsc/entities/engine.rs @@ -243,7 +243,7 @@ impl crate::Engine { /// Return true if [`self`] is an engine from the `jsc` runtime. pub fn is_jsc(&self) -> bool { - matches!(self.rt, RuntimeEngine::Jsc(_)) + matches!(self.rt, crate::RuntimeEngine::Jsc(_)) } } From 9038db95f25983426bb96446ae2bfd3e77168ae5 Mon Sep 17 00:00:00 2001 From: Edoardo Marangoni Date: Fri, 6 Dec 2024 08:58:38 +0100 Subject: [PATCH 18/19] fix(api): Add `non_exhaustive` to public `RuntimeKind` enum --- lib/api/src/rt/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/api/src/rt/mod.rs b/lib/api/src/rt/mod.rs index 25e137c8b36..4db3ff47d3b 100644 --- a/lib/api/src/rt/mod.rs +++ b/lib/api/src/rt/mod.rs @@ -19,6 +19,7 @@ pub mod js; #[cfg(feature = "jsc")] pub mod jsc; +#[non_exhaustive] #[derive(Debug, Clone, Copy)] /// An enumeration over all the supported runtimes. pub enum RuntimeKind { From c2815b3824134875c58c381f268be95a3258cc25 Mon Sep 17 00:00:00 2001 From: Edoardo Marangoni Date: Fri, 6 Dec 2024 08:59:29 +0100 Subject: [PATCH 19/19] fix(wasix): Remove comments and move `SpawnMemoryTypeOrStore` to visible path (mod.rs instead of tokio.rs) --- lib/wasix/src/runtime/task_manager/mod.rs | 13 +++++++++++++ lib/wasix/src/runtime/task_manager/tokio.rs | 19 +------------------ lib/wasix/src/state/func_env.rs | 2 +- 3 files changed, 15 insertions(+), 19 deletions(-) diff --git a/lib/wasix/src/runtime/task_manager/mod.rs b/lib/wasix/src/runtime/task_manager/mod.rs index 7a9c4f6a97f..ffa2cc1ba98 100644 --- a/lib/wasix/src/runtime/task_manager/mod.rs +++ b/lib/wasix/src/runtime/task_manager/mod.rs @@ -31,6 +31,19 @@ pub enum SpawnMemoryType<'a> { CopyMemory(Memory, StoreRef<'a>), } +/// Describes whether a new memory should be created (and, in case, its type) or if it was already +/// created and the store it belongs to. +/// +/// # Note +/// +/// This type is necessary for now because we can't pass a [`wasmer::StoreRef`] between threads, so this +/// conceptually is a Send-able [`SpawnMemoryType`]. +pub enum SpawnMemoryTypeOrStore { + New, + Type(wasmer::MemoryType), + StoreAndMemory(wasmer::Store, Option), +} + pub type WasmResumeTask = dyn FnOnce(WasiFunctionEnv, Store, Bytes) + Send + 'static; pub type WasmResumeTrigger = dyn FnOnce() -> Pin> + Send + 'static>> diff --git a/lib/wasix/src/runtime/task_manager/tokio.rs b/lib/wasix/src/runtime/task_manager/tokio.rs index 884b3ccdea9..f65926a64b6 100644 --- a/lib/wasix/src/runtime/task_manager/tokio.rs +++ b/lib/wasix/src/runtime/task_manager/tokio.rs @@ -7,7 +7,7 @@ use futures::{future::BoxFuture, Future}; use tokio::runtime::{Handle, Runtime}; use wasmer::AsStoreMut; -use super::{TaskWasm, TaskWasmRunProperties, VirtualTaskManager}; +use super::{SpawnMemoryTypeOrStore, TaskWasm, TaskWasmRunProperties, VirtualTaskManager}; #[derive(Debug, Clone)] pub enum RuntimeOrHandle { @@ -120,19 +120,6 @@ impl<'g> Drop for TokioRuntimeGuard<'g> { fn drop(&mut self) {} } -/// Describes whether a new memory should be created (and, in case, its type) or if it was already -/// created and the store it belongs to. -/// -/// # Note -/// -/// This type is necessary for now because we can't pass a [`wasmer::StoreRef`] between threads, so this -/// conceptually is a Send-able [`SpawnMemoryType`]. -pub enum SpawnMemoryTypeOrStore { - New, - Type(wasmer::MemoryType), - StoreAndMemory(wasmer::Store, Option), -} - impl VirtualTaskManager for TokioTaskManager { /// See [`VirtualTaskManager::sleep_now`]. fn sleep_now(&self, time: Duration) -> Pin + Send + Sync>> { @@ -160,13 +147,9 @@ impl VirtualTaskManager for TokioTaskManager { /// See [`VirtualTaskManager::task_wasm`]. fn task_wasm(&self, task: TaskWasm) -> Result<(), WasiThreadError> { - // Create the context on a new store let run = task.run; let recycle = task.recycle; - //let module = task.module; let env = task.env; - //let globals = task.globals; - //let update_layout = task.update_layout; let make_memory: SpawnMemoryTypeOrStore = match task.spawn_type { SpawnMemoryType::CreateMemory => SpawnMemoryTypeOrStore::New, diff --git a/lib/wasix/src/state/func_env.rs b/lib/wasix/src/state/func_env.rs index eb9a4467097..fba2703ef8f 100644 --- a/lib/wasix/src/state/func_env.rs +++ b/lib/wasix/src/state/func_env.rs @@ -12,7 +12,7 @@ use crate::os::task::thread::RewindResultType; use crate::syscalls::restore_snapshot; use crate::{ import_object_for_all_wasi_versions, - runtime::task_manager::tokio::SpawnMemoryTypeOrStore, + runtime::task_manager::SpawnMemoryTypeOrStore, state::WasiInstanceHandles, utils::{get_wasi_version, get_wasi_versions, store::restore_store_snapshot}, RewindStateOption, StoreSnapshot, WasiEnv, WasiError, WasiRuntimeError, WasiThreadError,