diff --git a/CHANGELOG.md b/CHANGELOG.md index e9869eb041f..fa8d89225f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ ### Changed +- [#1865](https://github.com/wasmerio/wasmer/pull/1865) Require that implementors of `WasmerEnv` also implement `Send`, `Sync`, and `Clone`. - [#1851](https://github.com/wasmerio/wasmer/pull/1851) Improve test suite and documentation of the Wasmer C API - [#1874](https://github.com/wasmerio/wasmer/pull/1874) Set `CompilerConfig` to be owned (following wasm-c-api) - [#1880](https://github.com/wasmerio/wasmer/pull/1880) Remove cmake dependency for tests diff --git a/examples/imports_function_env.rs b/examples/imports_function_env.rs index d9dc1f1d5c0..3b46a9a7baf 100644 --- a/examples/imports_function_env.rs +++ b/examples/imports_function_env.rs @@ -19,8 +19,7 @@ //! //! Ready? -use std::cell::RefCell; -use std::sync::Arc; +use std::sync::{Arc, Mutex}; use wasmer::{imports, wat2wasm, Function, Instance, Module, Store, WasmerEnv}; use wasmer_compiler_cranelift::Cranelift; use wasmer_engine_jit::JIT; @@ -57,11 +56,11 @@ fn main() -> Result<(), Box> { let module = Module::new(&store, wasm_bytes)?; // We create some shared data here, `Arc` is required because we may - // move our WebAssembly instance to another thread to run it. RefCell + // move our WebAssembly instance to another thread to run it. Mutex // lets us get shared mutabilty which is fine because we know we won't // run host calls concurrently. If concurrency is a possibilty, we'd have // to use a `Mutex`. - let shared_counter: Arc> = Arc::new(RefCell::new(0)); + let shared_counter: Arc> = Arc::new(Mutex::new(0)); // Once we have our counter we'll wrap it inside en `Env` which we'll pass // to our imported functions. @@ -70,17 +69,17 @@ fn main() -> Result<(), Box> { // possible to know the size of the `Env` at compile time (i.e it has to // implement the `Sized` trait) and that it implement the `WasmerEnv` trait. // We derive a default implementation of `WasmerEnv` here. - #[derive(WasmerEnv)] + #[derive(WasmerEnv, Clone)] struct Env { - counter: Arc>, + counter: Arc>, } // Create the functions fn get_counter(env: &Env) -> i32 { - *env.counter.borrow() + *env.counter.lock().unwrap() } fn add_to_counter(env: &Env, add: i32) -> i32 { - let mut counter_ref = env.counter.borrow_mut(); + let mut counter_ref = env.counter.lock().unwrap(); *counter_ref += add; *counter_ref @@ -106,7 +105,7 @@ fn main() -> Result<(), Box> { .get_function("increment_counter_loop")? .native::()?; - let counter_value: i32 = *shared_counter.borrow(); + let counter_value: i32 = *shared_counter.lock().unwrap(); println!("Initial ounter value: {:?}", counter_value); println!("Calling `increment_counter_loop` function..."); @@ -115,7 +114,7 @@ fn main() -> Result<(), Box> { // It will loop five times thus incrementing our counter five times. let result = increment_counter_loop.call(5)?; - let counter_value: i32 = *shared_counter.borrow(); + let counter_value: i32 = *shared_counter.lock().unwrap(); println!("New counter value (host): {:?}", counter_value); assert_eq!(counter_value, 5); diff --git a/lib/api/src/env.rs b/lib/api/src/env.rs index 53c37d6d2ea..af403030e19 100644 --- a/lib/api/src/env.rs +++ b/lib/api/src/env.rs @@ -30,12 +30,12 @@ impl From for HostEnvInitError { /// ``` /// use wasmer::{WasmerEnv, LazyInit, Memory, NativeFunc}; /// -/// #[derive(WasmerEnv)] +/// #[derive(WasmerEnv, Clone)] /// pub struct MyEnvWithNoInstanceData { /// non_instance_data: u8, /// } /// -/// #[derive(WasmerEnv)] +/// #[derive(WasmerEnv, Clone)] /// pub struct MyEnvWithInstanceData { /// non_instance_data: u8, /// #[wasmer(export)] @@ -54,6 +54,7 @@ impl From for HostEnvInitError { /// This trait can also be implemented manually: /// ``` /// # use wasmer::{WasmerEnv, LazyInit, Memory, Instance, HostEnvInitError}; +/// #[derive(Clone)] /// pub struct MyEnv { /// memory: LazyInit, /// } @@ -66,7 +67,7 @@ impl From for HostEnvInitError { /// } /// } /// ``` -pub trait WasmerEnv { +pub trait WasmerEnv: Clone + Send + Sync { /// The function that Wasmer will call on your type to let it finish /// setting up the environment with data from the `Instance`. /// @@ -94,26 +95,26 @@ impl WasmerEnv for isize {} impl WasmerEnv for char {} impl WasmerEnv for bool {} impl WasmerEnv for String {} -impl WasmerEnv for ::std::sync::atomic::AtomicBool {} -impl WasmerEnv for ::std::sync::atomic::AtomicI8 {} -impl WasmerEnv for ::std::sync::atomic::AtomicU8 {} -impl WasmerEnv for ::std::sync::atomic::AtomicI16 {} -impl WasmerEnv for ::std::sync::atomic::AtomicU16 {} -impl WasmerEnv for ::std::sync::atomic::AtomicI32 {} -impl WasmerEnv for ::std::sync::atomic::AtomicU32 {} -impl WasmerEnv for ::std::sync::atomic::AtomicI64 {} -impl WasmerEnv for ::std::sync::atomic::AtomicUsize {} -impl WasmerEnv for ::std::sync::atomic::AtomicIsize {} -impl WasmerEnv for dyn ::std::any::Any {} +impl<'a> WasmerEnv for &'a ::std::sync::atomic::AtomicBool {} +impl<'a> WasmerEnv for &'a ::std::sync::atomic::AtomicI8 {} +impl<'a> WasmerEnv for &'a ::std::sync::atomic::AtomicU8 {} +impl<'a> WasmerEnv for &'a ::std::sync::atomic::AtomicI16 {} +impl<'a> WasmerEnv for &'a ::std::sync::atomic::AtomicU16 {} +impl<'a> WasmerEnv for &'a ::std::sync::atomic::AtomicI32 {} +impl<'a> WasmerEnv for &'a ::std::sync::atomic::AtomicU32 {} +impl<'a> WasmerEnv for &'a ::std::sync::atomic::AtomicI64 {} +impl<'a> WasmerEnv for &'a ::std::sync::atomic::AtomicUsize {} +impl<'a> WasmerEnv for &'a ::std::sync::atomic::AtomicIsize {} impl WasmerEnv for Box { fn init_with_instance(&mut self, instance: &Instance) -> Result<(), HostEnvInitError> { (&mut **self).init_with_instance(instance) } } -impl WasmerEnv for &'static mut T { +impl WasmerEnv for ::std::sync::Arc<::std::sync::Mutex> { fn init_with_instance(&mut self, instance: &Instance) -> Result<(), HostEnvInitError> { - (*self).init_with_instance(instance) + let mut guard = self.lock().unwrap(); + guard.init_with_instance(instance) } } diff --git a/lib/api/src/externals/function.rs b/lib/api/src/externals/function.rs index dc4aa0ef075..f961ceeda32 100644 --- a/lib/api/src/externals/function.rs +++ b/lib/api/src/externals/function.rs @@ -12,7 +12,8 @@ pub use inner::{UnsafeMutableEnv, WithUnsafeMutableEnv}; use std::cmp::max; use std::fmt; -use wasmer_engine::{Export, ExportFunction}; +use std::sync::Arc; +use wasmer_engine::{Export, ExportFunction, ExportFunctionMetadata}; use wasmer_vm::{ raise_user_trap, resume_panic, wasmer_call_trampoline, VMCallerCheckedAnyfunc, VMDynamicFunctionContext, VMExportFunction, VMFunctionBody, VMFunctionEnvironment, @@ -106,23 +107,48 @@ impl Function { F: Fn(&[Val]) -> Result, RuntimeError> + 'static, { let ty: FunctionType = ty.into(); - let dynamic_ctx = VMDynamicFunctionContext::from_context(VMDynamicFunctionWithoutEnv { - func: Box::new(func), - function_type: ty.clone(), - }); + let dynamic_ctx: VMDynamicFunctionContext = + VMDynamicFunctionContext::from_context(DynamicFunctionWithoutEnv { + func: Arc::new(func), + function_type: ty.clone(), + }); // We don't yet have the address with the Wasm ABI signature. // The engine linker will replace the address with one pointing to a // generated dynamic trampoline. let address = std::ptr::null() as *const VMFunctionBody; - let vmctx = VMFunctionEnvironment { - host_env: Box::into_raw(Box::new(dynamic_ctx)) as *mut _, + let host_env = Box::into_raw(Box::new(dynamic_ctx)) as *mut _; + let vmctx = VMFunctionEnvironment { host_env }; + let host_env_clone_fn: fn(*mut std::ffi::c_void) -> *mut std::ffi::c_void = |ptr| { + let duped_env: VMDynamicFunctionContext = unsafe { + let ptr: *mut VMDynamicFunctionContext = ptr as _; + let item: &VMDynamicFunctionContext = &*ptr; + item.clone() + }; + Box::into_raw(Box::new(duped_env)) as _ + }; + let host_env_drop_fn: fn(*mut std::ffi::c_void) = |ptr: *mut std::ffi::c_void| { + unsafe { + Box::from_raw(ptr as *mut VMDynamicFunctionContext) + }; }; Self { store: store.clone(), definition: FunctionDefinition::Host(HostFunctionDefinition { has_env: false }), exported: ExportFunction { - import_init_function_ptr: None, + metadata: Some(Arc::new( + // # Safety + // - All these functions work on all threads + // - The host env is `Send`. + unsafe { + ExportFunctionMetadata::new( + host_env, + None, + host_env_clone_fn, + host_env_drop_fn, + ) + }, + )), vm_function: VMExportFunction { address, kind: VMFunctionKind::Dynamic, @@ -147,7 +173,7 @@ impl Function { /// # use wasmer::{Function, FunctionType, Type, Store, Value, WasmerEnv}; /// # let store = Store::default(); /// # - /// #[derive(WasmerEnv)] + /// #[derive(WasmerEnv, Clone)] /// struct Env { /// multiplier: i32, /// }; @@ -168,7 +194,7 @@ impl Function { /// # let store = Store::default(); /// const I32_I32_TO_I32: ([Type; 2], [Type; 1]) = ([Type::I32, Type::I32], [Type::I32]); /// - /// #[derive(WasmerEnv)] + /// #[derive(WasmerEnv, Clone)] /// struct Env { /// multiplier: i32, /// }; @@ -187,30 +213,65 @@ impl Function { Env: Sized + WasmerEnv + 'static, { let ty: FunctionType = ty.into(); - let dynamic_ctx = VMDynamicFunctionContext::from_context(VMDynamicFunctionWithEnv { - env: Box::new(env), - func: Box::new(func), - function_type: ty.clone(), - }); + let dynamic_ctx: VMDynamicFunctionContext> = + VMDynamicFunctionContext::from_context(DynamicFunctionWithEnv { + env: Box::new(env), + func: Arc::new(func), + function_type: ty.clone(), + }); + // We don't yet have the address with the Wasm ABI signature. // The engine linker will replace the address with one pointing to a // generated dynamic trampoline. let address = std::ptr::null() as *const VMFunctionBody; - let vmctx = VMFunctionEnvironment { - host_env: Box::into_raw(Box::new(dynamic_ctx)) as *mut _, - }; - // TODO: look into removing transmute by changing API type signatures + let host_env = Box::into_raw(Box::new(dynamic_ctx)) as *mut _; + let vmctx = VMFunctionEnvironment { host_env }; + let import_init_function_ptr: fn(_, _) -> Result<(), _> = + |ptr: *mut std::ffi::c_void, instance: *const std::ffi::c_void| { + let ptr = ptr as *mut VMDynamicFunctionContext>; + unsafe { + let env = &mut *ptr; + let env: &mut Env = &mut *env.ctx.env; + let instance = &*(instance as *const crate::Instance); + Env::init_with_instance(env, instance) + } + }; let import_init_function_ptr = Some(unsafe { std::mem::transmute:: Result<(), _>, fn(_, _) -> Result<(), _>>( - Env::init_with_instance, + import_init_function_ptr, ) }); + let host_env_clone_fn: fn(*mut std::ffi::c_void) -> *mut std::ffi::c_void = |ptr| { + let duped_env: VMDynamicFunctionContext> = unsafe { + let ptr: *mut VMDynamicFunctionContext> = ptr as _; + let item: &VMDynamicFunctionContext> = &*ptr; + item.clone() + }; + Box::into_raw(Box::new(duped_env)) as _ + }; + let host_env_drop_fn: fn(*mut std::ffi::c_void) = |ptr: *mut std::ffi::c_void| { + unsafe { + Box::from_raw(ptr as *mut VMDynamicFunctionContext>) + }; + }; Self { store: store.clone(), definition: FunctionDefinition::Host(HostFunctionDefinition { has_env: true }), exported: ExportFunction { - import_init_function_ptr, + metadata: Some(Arc::new( + // # Safety + // - All these functions work on all threads + // - The host env is `Send`. + unsafe { + ExportFunctionMetadata::new( + host_env, + import_init_function_ptr, + host_env_clone_fn, + host_env_drop_fn, + ) + }, + )), vm_function: VMExportFunction { address, kind: VMFunctionKind::Dynamic, @@ -264,7 +325,7 @@ impl Function { exported: ExportFunction { // TODO: figure out what's going on in this function: it takes an `Env` // param but also marks itself as not having an env - import_init_function_ptr: None, + metadata: None, vm_function: VMExportFunction { address, vmctx, @@ -288,7 +349,7 @@ impl Function { /// # use wasmer::{Store, Function, WasmerEnv}; /// # let store = Store::default(); /// # - /// #[derive(WasmerEnv)] + /// #[derive(WasmerEnv, Clone)] /// struct Env { /// multiplier: i32, /// }; @@ -318,10 +379,20 @@ impl Function { // Wasm-defined functions have a `VMContext`. // In the case of Host-defined functions `VMContext` is whatever environment // the user want to attach to the function. - let box_env = Box::new(env); - let vmctx = VMFunctionEnvironment { - host_env: Box::into_raw(box_env) as *mut _, + let host_env = Box::into_raw(Box::new(env)) as *mut _; + let vmctx = VMFunctionEnvironment { host_env }; + let host_env_clone_fn: fn(*mut std::ffi::c_void) -> *mut std::ffi::c_void = |ptr| { + let duped_env = unsafe { + let ptr: *mut Env = ptr as _; + let item: &Env = &*ptr; + item.clone() + }; + Box::into_raw(Box::new(duped_env)) as _ }; + let host_env_drop_fn: fn(*mut std::ffi::c_void) = |ptr: *mut std::ffi::c_void| { + unsafe { Box::from_raw(ptr as *mut Env) }; + }; + // TODO: look into removing transmute by changing API type signatures let import_init_function_ptr = Some(unsafe { std::mem::transmute:: Result<(), _>, fn(_, _) -> Result<(), _>>( @@ -334,7 +405,19 @@ impl Function { store: store.clone(), definition: FunctionDefinition::Host(HostFunctionDefinition { has_env: true }), exported: ExportFunction { - import_init_function_ptr, + metadata: Some(Arc::new( + // # Safety + // - All these functions work on all threads + // - The host env is `Send`. + unsafe { + ExportFunctionMetadata::new( + host_env, + import_init_function_ptr, + host_env_clone_fn, + host_env_drop_fn, + ) + }, + )), vm_function: VMExportFunction { address, kind: VMFunctionKind::Static, @@ -373,9 +456,20 @@ impl Function { let address = function.address(); let box_env = Box::new(env); - let vmctx = VMFunctionEnvironment { - host_env: Box::into_raw(box_env) as *mut _, + let host_env = Box::into_raw(box_env) as *mut _; + let vmctx = VMFunctionEnvironment { host_env }; + let host_env_clone_fn: fn(*mut std::ffi::c_void) -> *mut std::ffi::c_void = |ptr| { + let duped_env: Env = { + let ptr: *mut Env = ptr as _; + let item: &Env = &*ptr; + item.clone() + }; + Box::into_raw(Box::new(duped_env)) as _ }; + let host_env_drop_fn: fn(*mut std::ffi::c_void) = |ptr: *mut std::ffi::c_void| { + Box::from_raw(ptr as *mut Env); + }; + let signature = function.ty(); // TODO: look into removing transmute by changing API type signatures let import_init_function_ptr = Some(std::mem::transmute::< @@ -387,7 +481,17 @@ impl Function { store: store.clone(), definition: FunctionDefinition::Host(HostFunctionDefinition { has_env: true }), exported: ExportFunction { - import_init_function_ptr, + metadata: Some(Arc::new( + // # Safety + // - All these functions work on all threads + // - The host env is `Send`. + ExportFunctionMetadata::new( + host_env, + import_init_function_ptr, + host_env_clone_fn, + host_env_drop_fn, + ), + )), vm_function: VMExportFunction { address, kind: VMFunctionKind::Static, @@ -756,13 +860,14 @@ pub(crate) trait VMDynamicFunction { fn function_type(&self) -> &FunctionType; } -pub(crate) struct VMDynamicFunctionWithoutEnv { +#[derive(Clone)] +pub(crate) struct DynamicFunctionWithoutEnv { #[allow(clippy::type_complexity)] - func: Box Result, RuntimeError> + 'static>, + func: Arc Result, RuntimeError> + 'static>, function_type: FunctionType, } -impl VMDynamicFunction for VMDynamicFunctionWithoutEnv { +impl VMDynamicFunction for DynamicFunctionWithoutEnv { fn call(&self, args: &[Val]) -> Result, RuntimeError> { (*self.func)(&args) } @@ -771,19 +876,27 @@ impl VMDynamicFunction for VMDynamicFunctionWithoutEnv { } } -#[repr(C)] -pub(crate) struct VMDynamicFunctionWithEnv +pub(crate) struct DynamicFunctionWithEnv where Env: Sized + 'static, { - // This field _must_ come first in this struct. - env: Box, function_type: FunctionType, #[allow(clippy::type_complexity)] - func: Box Result, RuntimeError> + 'static>, + func: Arc Result, RuntimeError> + 'static>, + env: Box, +} + +impl Clone for DynamicFunctionWithEnv { + fn clone(&self) -> Self { + Self { + env: self.env.clone(), + function_type: self.function_type.clone(), + func: self.func.clone(), + } + } } -impl VMDynamicFunction for VMDynamicFunctionWithEnv +impl VMDynamicFunction for DynamicFunctionWithEnv where Env: Sized + 'static, { diff --git a/lib/api/src/native.rs b/lib/api/src/native.rs index 7330b31e6fe..c2b89232362 100644 --- a/lib/api/src/native.rs +++ b/lib/api/src/native.rs @@ -10,8 +10,8 @@ use std::marker::PhantomData; use crate::externals::function::{ - FunctionDefinition, HostFunctionDefinition, VMDynamicFunction, VMDynamicFunctionWithEnv, - VMDynamicFunctionWithoutEnv, WasmFunctionDefinition, + DynamicFunctionWithEnv, DynamicFunctionWithoutEnv, FunctionDefinition, HostFunctionDefinition, + VMDynamicFunction, WasmFunctionDefinition, }; use crate::{FromToNativeWasmType, Function, RuntimeError, Store, WasmTypeList}; use std::panic::{catch_unwind, AssertUnwindSafe}; @@ -21,6 +21,7 @@ use wasmer_vm::{VMDynamicFunctionContext, VMFunctionBody, VMFunctionEnvironment, /// A WebAssembly function that can be called natively /// (using the Native ABI). +#[derive(Clone)] pub struct NativeFunc { definition: FunctionDefinition, store: Store, @@ -183,13 +184,13 @@ macro_rules! impl_native_traits { VMFunctionKind::Dynamic => { let params_list = [ $( $x.to_native().to_value() ),* ]; let results = if !has_env { - type VMContextWithoutEnv = VMDynamicFunctionContext; + type VMContextWithoutEnv = VMDynamicFunctionContext; unsafe { let ctx = self.vmctx().host_env as *mut VMContextWithoutEnv; (*ctx).ctx.call(¶ms_list)? } } else { - type VMContextWithEnv = VMDynamicFunctionContext>; + type VMContextWithEnv = VMDynamicFunctionContext>; unsafe { let ctx = self.vmctx().host_env as *mut VMContextWithEnv; (*ctx).ctx.call(¶ms_list)? diff --git a/lib/api/src/types.rs b/lib/api/src/types.rs index a0f8d89b4e4..eeebbc34e71 100644 --- a/lib/api/src/types.rs +++ b/lib/api/src/types.rs @@ -76,7 +76,7 @@ impl ValFuncRef for Val { let export = wasmer_engine::ExportFunction { // TODO: // figure out if we ever need a value here: need testing with complicated import patterns - import_init_function_ptr: None, + metadata: None, vm_function: wasmer_vm::VMExportFunction { address: item.func_ptr, signature, diff --git a/lib/api/tests/externals.rs b/lib/api/tests/externals.rs index 9d2125ccae1..5dec277bc83 100644 --- a/lib/api/tests/externals.rs +++ b/lib/api/tests/externals.rs @@ -403,7 +403,7 @@ fn function_outlives_instance() -> Result<()> { #[test] fn manually_generate_wasmer_env() -> Result<()> { let store = Store::default(); - #[derive(WasmerEnv)] + #[derive(WasmerEnv, Clone)] struct MyEnv { val: u32, memory: LazyInit, diff --git a/lib/c-api/src/deprecated/import/mod.rs b/lib/c-api/src/deprecated/import/mod.rs index c6e1cc58b7b..d972c7af82b 100644 --- a/lib/c-api/src/deprecated/import/mod.rs +++ b/lib/c-api/src/deprecated/import/mod.rs @@ -15,7 +15,7 @@ use std::ffi::{c_void, CStr}; use std::os::raw::c_char; use std::ptr::{self, NonNull}; use std::slice; -// use std::sync::Arc; +use std::sync::{Arc, Mutex}; // use std::convert::TryFrom, use std::collections::HashMap; use wasmer::{ @@ -48,7 +48,7 @@ pub(crate) struct CAPIImportObject { /// List of pointers to `LegacyEnv`s used to patch imported functions to be able to /// pass a the "vmctx" as the first argument. /// Needed here because of extending import objects. - pub(crate) instance_pointers_to_update: Vec>, + pub(crate) instance_pointers_to_update: Vec>>, } #[repr(C)] @@ -399,7 +399,6 @@ pub unsafe extern "C" fn wasmer_import_object_imports_destroy( let function_wrapper: Box = Box::from_raw(import.value.func as *mut _); let _: Box = Box::from_raw(function_wrapper.func.as_ptr()); - let _: Box = Box::from_raw(function_wrapper.legacy_env.as_ptr()); } wasmer_import_export_kind::WASM_GLOBAL => { let _: Box = Box::from_raw(import.value.global as *mut _); @@ -464,7 +463,7 @@ pub unsafe extern "C" fn wasmer_import_object_extend( let func_export = (*func_wrapper).func.as_ptr(); import_object .instance_pointers_to_update - .push((*func_wrapper).legacy_env); + .push((*func_wrapper).legacy_env.clone()); Extern::Function((&*func_export).clone()) } wasmer_import_export_kind::WASM_GLOBAL => { @@ -635,11 +634,16 @@ pub unsafe extern "C" fn wasmer_import_func_params_arity( /// struct used to pass in context to functions (which must be back-patched) -#[derive(Debug, Default, WasmerEnv)] +#[derive(Debug, Default, WasmerEnv, Clone)] pub(crate) struct LegacyEnv { pub(crate) instance_ptr: Option>, } +/// Because this type requires unsafe to do any meaningful operation, we +/// can implement `Send` and `Sync`. All operations must be synchronized. +unsafe impl Send for LegacyEnv {} +unsafe impl Sync for LegacyEnv {} + impl LegacyEnv { pub(crate) fn ctx_ptr(&self) -> *mut CAPIInstance { self.instance_ptr @@ -654,7 +658,7 @@ impl LegacyEnv { #[derive(Debug)] pub(crate) struct FunctionWrapper { pub(crate) func: NonNull, - pub(crate) legacy_env: NonNull, + pub(crate) legacy_env: Arc>, } /// Creates new host function, aka imported function. `func` is a @@ -691,13 +695,14 @@ pub unsafe extern "C" fn wasmer_import_func_new( let store = get_global_store(); - let env_ptr = Box::into_raw(Box::new(LegacyEnv::default())); + let env = Arc::new(Mutex::new(LegacyEnv::default())); - let func = Function::new_with_env(store, &func_type, &mut *env_ptr, move |env, args| { + let func = Function::new_with_env(store, &func_type, env.clone(), move |env, args| { use libffi::high::call::{call, Arg}; use libffi::low::CodePtr; + let env_guard = env.lock().unwrap(); - let ctx_ptr = env.ctx_ptr(); + let ctx_ptr = env_guard.ctx_ptr(); let ctx_ptr_val = ctx_ptr as *const _ as isize as i64; let ffi_args: Vec = { @@ -735,7 +740,7 @@ pub unsafe extern "C" fn wasmer_import_func_new( let function_wrapper = FunctionWrapper { func: NonNull::new_unchecked(Box::into_raw(Box::new(func))), - legacy_env: NonNull::new_unchecked(env_ptr), + legacy_env: env.clone(), }; Box::into_raw(Box::new(function_wrapper)) as *mut wasmer_import_func_t } @@ -865,7 +870,6 @@ pub unsafe extern "C" fn wasmer_import_func_returns_arity( pub unsafe extern "C" fn wasmer_import_func_destroy(func: Option>) { if let Some(func) = func { let function_wrapper = Box::from_raw(func.cast::().as_ptr()); - let _legacy_env = Box::from_raw(function_wrapper.legacy_env.as_ptr()); let _function = Box::from_raw(function_wrapper.func.as_ptr()); } } diff --git a/lib/c-api/src/deprecated/instance.rs b/lib/c-api/src/deprecated/instance.rs index 5ca24ee6ffa..b5d5cf39ddc 100644 --- a/lib/c-api/src/deprecated/instance.rs +++ b/lib/c-api/src/deprecated/instance.rs @@ -186,7 +186,7 @@ pub unsafe extern "C" fn wasmer_instantiate( wasmer_import_export_kind::WASM_FUNCTION => { let func_wrapper = import.value.func as *mut FunctionWrapper; let func_export = (*func_wrapper).func.as_ptr(); - instance_pointers_to_update.push((*func_wrapper).legacy_env); + instance_pointers_to_update.push((*func_wrapper).legacy_env.clone()); Extern::Function((&*func_export).clone()) } wasmer_import_export_kind::WASM_GLOBAL => { @@ -229,8 +229,9 @@ pub unsafe extern "C" fn wasmer_instantiate( ctx_data: None, }; let c_api_instance_pointer = Box::into_raw(Box::new(c_api_instance)); - for mut to_update in instance_pointers_to_update { - to_update.as_mut().instance_ptr = Some(NonNull::new_unchecked(c_api_instance_pointer)); + for to_update in instance_pointers_to_update { + let mut to_update_guard = to_update.lock().unwrap(); + to_update_guard.instance_ptr = Some(NonNull::new_unchecked(c_api_instance_pointer)); } *instance = c_api_instance_pointer as *mut wasmer_instance_t; wasmer_result_t::WASMER_OK diff --git a/lib/c-api/src/deprecated/module.rs b/lib/c-api/src/deprecated/module.rs index 4a160898b60..4b72ab960ab 100644 --- a/lib/c-api/src/deprecated/module.rs +++ b/lib/c-api/src/deprecated/module.rs @@ -192,7 +192,8 @@ pub unsafe extern "C" fn wasmer_module_import_instantiate( }; let c_api_instance_pointer = Box::into_raw(Box::new(c_api_instance)); for to_update in import_object.instance_pointers_to_update.iter_mut() { - to_update.as_mut().instance_ptr = Some(NonNull::new_unchecked(c_api_instance_pointer)); + let mut to_update_guard = to_update.lock().unwrap(); + to_update_guard.instance_ptr = Some(NonNull::new_unchecked(c_api_instance_pointer)); } *instance = c_api_instance_pointer as *mut wasmer_instance_t; diff --git a/lib/c-api/src/wasm_c_api/externals/function.rs b/lib/c-api/src/wasm_c_api/externals/function.rs index eb2e1fb7859..8b5a1cc40c2 100644 --- a/lib/c-api/src/wasm_c_api/externals/function.rs +++ b/lib/c-api/src/wasm_c_api/externals/function.rs @@ -101,13 +101,18 @@ pub unsafe extern "C" fn wasm_func_new_with_env( let func_sig = &function_type.inner().function_type; let num_rets = func_sig.results().len(); - #[derive(wasmer::WasmerEnv)] + #[derive(wasmer::WasmerEnv, Clone)] #[repr(C)] struct WrapperEnv { env: *mut c_void, finalizer: Option, }; + // Only relevant when using multiple threads in the C API; + // Synchronization will be done via the C API / on the C side. + unsafe impl Send for WrapperEnv {} + unsafe impl Sync for WrapperEnv {} + impl Drop for WrapperEnv { fn drop(&mut self) { if let Some(finalizer) = self.finalizer { diff --git a/lib/deprecated/runtime-core/Cargo.lock b/lib/deprecated/runtime-core/Cargo.lock index f480eebb9bd..c22464150c3 100644 --- a/lib/deprecated/runtime-core/Cargo.lock +++ b/lib/deprecated/runtime-core/Cargo.lock @@ -584,6 +584,15 @@ dependencies = [ "winapi", ] +[[package]] +name = "memmap2" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d9b70ca2a6103ac8b665dc150b142ef0e4e89df640c9e6cf295d189c3caebe5a" +dependencies = [ + "libc", +] + [[package]] name = "memoffset" version = "0.5.5" @@ -1176,7 +1185,6 @@ version = "1.0.0-beta1" dependencies = [ "blake3", "hex", - "memmap", "thiserror", "wasmer", ] @@ -1270,6 +1278,7 @@ dependencies = [ "backtrace", "bincode", "lazy_static", + "memmap2", "more-asserts", "rustc-demangle", "serde", @@ -1350,6 +1359,7 @@ version = "1.0.0-beta1" dependencies = [ "cranelift-entity", "serde", + "thiserror", ] [[package]] diff --git a/lib/deprecated/runtime-core/src/instance.rs b/lib/deprecated/runtime-core/src/instance.rs index 7cf42b30936..64b2f1b6cf2 100644 --- a/lib/deprecated/runtime-core/src/instance.rs +++ b/lib/deprecated/runtime-core/src/instance.rs @@ -10,31 +10,30 @@ use crate::{ vm, }; use std::{ - cell::{Ref, RefCell, RefMut}, error::Error, - rc::Rc, + sync::{Arc, Mutex, MutexGuard}, }; pub use crate::typed_func::DynamicFunc as DynFunc; #[derive(Debug)] pub(crate) struct PreInstance { - pub(crate) vmctx: Rc>, + pub(crate) vmctx: Arc>, } impl PreInstance { pub(crate) fn new() -> Self { Self { - vmctx: Rc::new(RefCell::new(unsafe { vm::Ctx::new_uninit() })), + vmctx: Arc::new(Mutex::new(unsafe { vm::Ctx::new_uninit() })), } } - pub(crate) fn vmctx(&self) -> Rc> { + pub(crate) fn vmctx(&self) -> Arc> { self.vmctx.clone() } pub(crate) fn vmctx_ptr(&self) -> *mut vm::Ctx { - self.vmctx.as_ptr() + &mut *self.vmctx.lock().unwrap() as _ } } @@ -56,7 +55,7 @@ impl Instance { pub(crate) fn new(pre_instance: Box, new_instance: new::wasmer::Instance) -> Self { // Initialize the `vm::Ctx` { - let mut vmctx = pre_instance.vmctx.borrow_mut(); + let mut vmctx = pre_instance.vmctx.lock().unwrap(); vmctx.module_info = new_instance.module().info() as *const _; } @@ -200,16 +199,16 @@ impl Instance { /// [`Ctx`] used by this Instance. /// /// [`Ctx`]: struct.Ctx.html - pub fn context(&self) -> Ref { - self.pre_instance.vmctx.borrow() + pub fn context(&self) -> MutexGuard { + self.pre_instance.vmctx.lock().unwrap() } /// Returns a mutable reference to the /// [`Ctx`] used by this Instance. /// /// [`Ctx`]: struct.Ctx.html - pub fn context_mut(&mut self) -> RefMut { - self.pre_instance.vmctx.borrow_mut() + pub fn context_mut(&mut self) -> MutexGuard { + self.pre_instance.vmctx.lock().unwrap() } /// Returns an iterator over all of the items diff --git a/lib/deprecated/runtime-core/src/lib.rs b/lib/deprecated/runtime-core/src/lib.rs index 25cc41e089d..d30e5ba2fa8 100644 --- a/lib/deprecated/runtime-core/src/lib.rs +++ b/lib/deprecated/runtime-core/src/lib.rs @@ -40,7 +40,6 @@ //! customize the wasmer runtime. pub(crate) mod new { - pub use wasmer_types; pub use wasmer; pub use wasmer_cache; pub use wasmer_compiler; @@ -52,6 +51,7 @@ pub(crate) mod new { pub use wasmer_compiler_singlepass; pub use wasmer_engine; pub use wasmer_engine_jit; + pub use wasmer_types; pub use wasmer_vm; } @@ -115,8 +115,7 @@ impl GlobalStore { } #[allow(unused_variables)] - let update = |engine: new::wasmer_engine_jit::JIT, - global_store: &GlobalStore| { + let update = |engine: new::wasmer_engine_jit::JIT, global_store: &GlobalStore| { let engine = engine.engine(); *self.store.lock().unwrap() = Arc::new(new::wasmer::Store::new(&engine)); }; @@ -124,18 +123,25 @@ impl GlobalStore { match compiler { #[cfg(feature = "singlepass")] Backend::Singlepass => update( - new::wasmer_engine_jit::JIT::new(new::wasmer_compiler_singlepass::Singlepass::default()), + new::wasmer_engine_jit::JIT::new( + new::wasmer_compiler_singlepass::Singlepass::default(), + ), &self, ), #[cfg(feature = "cranelift")] - Backend::Cranelift => { - update( - new::wasmer_engine_jit::JIT::new(new::wasmer_compiler_cranelift::Cranelift::default()), &self) - } + Backend::Cranelift => update( + new::wasmer_engine_jit::JIT::new( + new::wasmer_compiler_cranelift::Cranelift::default(), + ), + &self, + ), #[cfg(feature = "llvm")] - Backend::LLVM => update(new::wasmer_engine_jit::JIT::new(new::wasmer_compiler_llvm::LLVM::default()), &self), + Backend::LLVM => update( + new::wasmer_engine_jit::JIT::new(new::wasmer_compiler_llvm::LLVM::default()), + &self, + ), Backend::Auto => *self.store.lock().unwrap() = Arc::new(Default::default()), }; diff --git a/lib/deprecated/runtime-core/src/typed_func.rs b/lib/deprecated/runtime-core/src/typed_func.rs index 53c88a4f070..54a37fab716 100644 --- a/lib/deprecated/runtime-core/src/typed_func.rs +++ b/lib/deprecated/runtime-core/src/typed_func.rs @@ -224,43 +224,40 @@ pub struct DynamicFunc { new_function: new::wasmer::Function, } -use std::{ - cell::{RefCell, RefMut}, - ops::DerefMut, - rc::Rc, -}; +use std::sync::{Arc, Mutex, MutexGuard}; /// Specific context for `DynamicFunc`. It's a hack. /// /// Initially, it holds an empty `vm::Ctx`, but it is replaced by the /// `vm::Ctx` from `instance::PreInstance` in /// `module::Module::instantiate`. -#[derive(WasmerEnv)] +#[derive(WasmerEnv, Clone)] pub(crate) struct DynamicCtx { - pub(crate) vmctx: Rc>, - inner_func: - Box Result, RuntimeError> + Send + 'static>, + pub(crate) vmctx: Arc>, + inner_func: Arc< + dyn Fn(&mut vm::Ctx, &[Value]) -> Result, RuntimeError> + Send + Sync + 'static, + >, } impl DynamicFunc { /// Create a new `DynamicFunc`. pub fn new(signature: &FuncSig, func: F) -> Self where - F: Fn(&mut vm::Ctx, &[Value]) -> Result, RuntimeError> + Send + 'static, + F: Fn(&mut vm::Ctx, &[Value]) -> Result, RuntimeError> + Send + Sync + 'static, { // Create an empty `vm::Ctx`, that is going to be overwritten by `Instance::new`. let ctx = DynamicCtx { - vmctx: Rc::new(RefCell::new(unsafe { vm::Ctx::new_uninit() })), - inner_func: Box::new(func), + vmctx: Arc::new(Mutex::new(unsafe { vm::Ctx::new_uninit() })), + inner_func: Arc::new(func), }; // Wrapper to safely extract a `&mut vm::Ctx` to pass // to `func`. fn inner(dyn_ctx: &DynamicCtx, params: &[Value]) -> Result, RuntimeError> { - let cell: Rc> = dyn_ctx.vmctx.clone(); - let mut vmctx: RefMut = cell.borrow_mut(); + let cell: Arc> = dyn_ctx.vmctx.clone(); + let mut vmctx: MutexGuard = cell.lock().unwrap(); - (dyn_ctx.inner_func)(vmctx.deref_mut(), params) + (dyn_ctx.inner_func)(&mut *vmctx, params) } Self { diff --git a/lib/deprecated/runtime-core/src/types.rs b/lib/deprecated/runtime-core/src/types.rs index 6323004c229..3b32941e65a 100644 --- a/lib/deprecated/runtime-core/src/types.rs +++ b/lib/deprecated/runtime-core/src/types.rs @@ -1,5 +1,6 @@ use crate::new; +pub use new::wasmer::{FromToNativeWasmType as WasmExternType, Val as Value}; pub use new::wasmer_types::{ // ExportType as ExportDescriptor, @@ -27,7 +28,6 @@ pub use new::wasmer_types::{ Type, ValueType, }; -pub use new::wasmer::{FromToNativeWasmType as WasmExternType, Val as Value}; /// Describes the mutability and type of a Global #[derive(Debug, Copy, Clone, Eq, PartialEq)] diff --git a/lib/deprecated/runtime-core/src/vm.rs b/lib/deprecated/runtime-core/src/vm.rs index b92a7edcf3a..8fd9df8b111 100644 --- a/lib/deprecated/runtime-core/src/vm.rs +++ b/lib/deprecated/runtime-core/src/vm.rs @@ -43,6 +43,11 @@ pub struct Ctx { /// We mark `Ctx` as a legacy env that can be passed by `&mut`. unsafe impl UnsafeMutableEnv for Ctx {} +/// This is correct because of the way we use the data in `Ctx`; +/// it's not correct in general though, so we should be careful when +/// updating this type. +unsafe impl Send for Ctx {} +unsafe impl Sync for Ctx {} impl Ctx { pub(crate) unsafe fn new_uninit() -> Self { diff --git a/lib/deprecated/runtime/Cargo.lock b/lib/deprecated/runtime/Cargo.lock index 1e296021f33..c41e119903d 100644 --- a/lib/deprecated/runtime/Cargo.lock +++ b/lib/deprecated/runtime/Cargo.lock @@ -584,6 +584,15 @@ dependencies = [ "winapi", ] +[[package]] +name = "memmap2" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d9b70ca2a6103ac8b665dc150b142ef0e4e89df640c9e6cf295d189c3caebe5a" +dependencies = [ + "libc", +] + [[package]] name = "memoffset" version = "0.5.5" @@ -1176,7 +1185,6 @@ version = "1.0.0-beta1" dependencies = [ "blake3", "hex", - "memmap", "thiserror", "wasmer", ] @@ -1270,6 +1278,7 @@ dependencies = [ "backtrace", "bincode", "lazy_static", + "memmap2", "more-asserts", "rustc-demangle", "serde", @@ -1357,6 +1366,7 @@ version = "1.0.0-beta1" dependencies = [ "cranelift-entity", "serde", + "thiserror", ] [[package]] diff --git a/lib/emscripten/src/lib.rs b/lib/emscripten/src/lib.rs index 84ad5b08447..8b2a3f76a23 100644 --- a/lib/emscripten/src/lib.rs +++ b/lib/emscripten/src/lib.rs @@ -14,7 +14,6 @@ extern crate log; use lazy_static::lazy_static; -use std::cell::UnsafeCell; use std::collections::HashMap; use std::f64; use std::path::PathBuf; @@ -105,6 +104,20 @@ impl EmEnv { } } +#[derive(Debug, Clone)] +pub struct LibcDirWrapper(pub *mut LibcDir); + +impl std::ops::Deref for LibcDirWrapper { + type Target = *mut LibcDir; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +unsafe impl Send for LibcDirWrapper {} +unsafe impl Sync for LibcDirWrapper {} + // TODO: Magic number - how is this calculated? const TOTAL_STACK: u32 = 5_242_880; // TODO: make this variable @@ -122,7 +135,7 @@ lazy_static! { const GLOBAL_BASE: u32 = 1024; const STATIC_BASE: u32 = GLOBAL_BASE; -#[derive(WasmerEnv, Default)] +#[derive(WasmerEnv, Clone, Default)] pub struct EmscriptenData { pub globals: EmscriptenGlobalsData, @@ -136,8 +149,8 @@ pub struct EmscriptenData { pub memset: LazyInit>, #[wasmer(export)] pub stack_alloc: LazyInit>, - pub jumps: Vec>, - pub opened_dirs: HashMap>, + pub jumps: Arc>>, + pub opened_dirs: HashMap>, #[wasmer(export)] pub dyn_call_i: LazyInit>, diff --git a/lib/emscripten/src/syscalls/unix.rs b/lib/emscripten/src/syscalls/unix.rs index 10fa10ec9d6..cf39590edb0 100644 --- a/lib/emscripten/src/syscalls/unix.rs +++ b/lib/emscripten/src/syscalls/unix.rs @@ -1,4 +1,4 @@ -use crate::{ptr::WasmPtr, varargs::VarArgs}; +use crate::{ptr::WasmPtr, varargs::VarArgs, LibcDirWrapper}; #[cfg(target_os = "macos")] use libc::size_t; /// NOTE: TODO: These syscalls only support wasm_32 for now because they assume offsets are u32 @@ -1064,12 +1064,12 @@ pub fn ___syscall220(ctx: &EmEnv, _which: i32, mut varargs: VarArgs) -> i32 { // let dir: *mut libc::DIR = unsafe { libc::fdopendir(fd) }; let dir = &*opened_dirs .entry(fd) - .or_insert_with(|| unsafe { Box::new(libc::fdopendir(fd)) }); + .or_insert_with(|| unsafe { Box::new(LibcDirWrapper(libc::fdopendir(fd))) }); let mut pos = 0; let offset = 256 + 12; while pos + offset <= count as usize { - let dirent = unsafe { readdir(**dir) }; + let dirent = unsafe { readdir(***dir) }; if dirent.is_null() { break; } diff --git a/lib/engine/src/artifact.rs b/lib/engine/src/artifact.rs index 3d1a8a04b31..3a93fe4ce1f 100644 --- a/lib/engine/src/artifact.rs +++ b/lib/engine/src/artifact.rs @@ -95,7 +95,7 @@ pub trait Artifact: Send + Sync + Upcastable { self.preinstantiate()?; let module = self.module(); - let (imports, import_initializers) = { + let (imports, import_function_envs) = { let mut imports = resolve_imports( &module, resolver, @@ -107,9 +107,9 @@ pub trait Artifact: Send + Sync + Upcastable { // Get the `WasmerEnv::init_with_instance` function pointers and the pointers // to the envs to call it on. - let import_initializers: Vec<(_, _)> = imports.get_import_initializers(); + let import_function_envs = imports.get_imported_function_envs(); - (imports, import_initializers) + (imports, import_function_envs) }; // Get pointers to where metadata about local memories should live in VM memory. @@ -143,7 +143,7 @@ pub trait Artifact: Send + Sync + Upcastable { imports, self.signatures().clone(), host_state, - import_initializers, + import_function_envs, ) .map_err(|trap| InstantiationError::Start(RuntimeError::from_trap(trap)))?; Ok(handle) diff --git a/lib/engine/src/export.rs b/lib/engine/src/export.rs index 8f972c3f79f..7b9ea127544 100644 --- a/lib/engine/src/export.rs +++ b/lib/engine/src/export.rs @@ -3,6 +3,8 @@ use wasmer_vm::{ VMExportTable, }; +use std::sync::Arc; + /// The value of an export passed from one instance to another. #[derive(Debug, Clone)] pub enum Export { @@ -35,7 +37,7 @@ impl From for Export { match other { VMExport::Function(vm_function) => Export::Function(ExportFunction { vm_function, - import_init_function_ptr: None, + metadata: None, }), VMExport::Memory(vm_memory) => Export::Memory(ExportMemory { vm_memory }), VMExport::Table(vm_table) => Export::Table(ExportTable { vm_table }), @@ -44,17 +46,100 @@ impl From for Export { } } +/// Extra metadata about `ExportFunction`s. +/// +/// The metadata acts as a kind of manual virtual dispatch. We store the +/// user-supplied `WasmerEnv` as a void pointer and have methods on it +/// that have been adapted to accept a void pointer. +/// +/// This struct owns the original `host_env`, thus when it gets dropped +/// it calls the `drop` function on it. +#[derive(Debug, PartialEq)] +pub struct ExportFunctionMetadata { + /// This field is stored here to be accessible by `Drop`. + /// + /// At the time it was added, it's not accessed anywhere outside of + /// the `Drop` implementation. This field is the "master copy" of the env, + /// that is, the original env passed in by the user. Every time we create + /// an `Instance` we clone this with the `host_env_clone_fn` field. + /// + /// Thus, we only bother to store the master copy at all here so that + /// we can free it. + /// + /// See `wasmer_vm::export::VMExportFunction::vmctx` for the version of + /// this pointer that is used by the VM when creating an `Instance`. + pub(crate) host_env: *mut std::ffi::c_void, + /// Function pointer to `WasmerEnv::init_with_instance(&mut self, instance: &Instance)`. + /// + /// This function is called to finish setting up the environment after + /// we create the `api::Instance`. + // This one is optional for now because dynamic host envs need the rest + // of this without the init fn + pub(crate) import_init_function_ptr: Option, + /// A function analogous to `Clone::clone` that returns a leaked `Box`. + pub(crate) host_env_clone_fn: fn(*mut std::ffi::c_void) -> *mut std::ffi::c_void, + /// The destructor to free the host environment. + /// + /// # Safety + /// - This function should only be called in when properly synchronized. + /// For example, in the `Drop` implementation of this type. + pub(crate) host_env_drop_fn: unsafe fn(*mut std::ffi::c_void), +} + +/// This can be `Send` because `host_env` comes from `WasmerEnv` which is +/// `Send`. Therefore all operations should work on any thread. +unsafe impl Send for ExportFunctionMetadata {} +/// This data may be shared across threads, `drop` is an unsafe function +/// pointer, so care must be taken when calling it. +unsafe impl Sync for ExportFunctionMetadata {} + +impl ExportFunctionMetadata { + /// Create an `ExportFunctionMetadata` type with information about + /// the exported function. + /// + /// # Safety + /// - the `host_env` must be `Send`. + /// - all function pointers must work on any thread. + pub unsafe fn new( + host_env: *mut std::ffi::c_void, + import_init_function_ptr: Option, + host_env_clone_fn: fn(*mut std::ffi::c_void) -> *mut std::ffi::c_void, + host_env_drop_fn: fn(*mut std::ffi::c_void), + ) -> Self { + Self { + host_env, + import_init_function_ptr, + host_env_clone_fn, + host_env_drop_fn, + } + } +} + +// We have to free `host_env` here because we always clone it before using it +// so all the `host_env`s freed at the `Instance` level won't touch the original. +impl Drop for ExportFunctionMetadata { + fn drop(&mut self) { + if !self.host_env.is_null() { + // # Safety + // - This is correct because we know no other references + // to this data can exist if we're dropping it. + unsafe { + (self.host_env_drop_fn)(self.host_env); + } + } + } +} + /// A function export value with an extra function pointer to initialize /// host environments. #[derive(Debug, Clone, PartialEq)] pub struct ExportFunction { /// The VM function, containing most of the data. pub vm_function: VMExportFunction, - /// Function pointer to `WasmerEnv::init_with_instance(&mut self, instance: &Instance)`. - /// - /// This function is called to finish setting up the environment after - /// we create the `api::Instance`. - pub import_init_function_ptr: Option, + /// Contains functions necessary to create and initialize host envs + /// with each `Instance` as well as being responsible for the + /// underlying memory of the host env. + pub metadata: Option>, } impl From for Export { diff --git a/lib/engine/src/lib.rs b/lib/engine/src/lib.rs index 557b1b039d2..e5754535823 100644 --- a/lib/engine/src/lib.rs +++ b/lib/engine/src/lib.rs @@ -34,7 +34,9 @@ pub use crate::engine::{Engine, EngineId}; pub use crate::error::{ DeserializeError, ImportError, InstantiationError, LinkError, SerializeError, }; -pub use crate::export::{Export, ExportFunction, ExportGlobal, ExportMemory, ExportTable}; +pub use crate::export::{ + Export, ExportFunction, ExportFunctionMetadata, ExportGlobal, ExportMemory, ExportTable, +}; pub use crate::resolver::{ resolve_imports, ChainableNamedResolver, NamedResolver, NamedResolverChain, NullResolver, Resolver, diff --git a/lib/engine/src/resolver.rs b/lib/engine/src/resolver.rs index f8d3ca40d88..acc1c800ca5 100644 --- a/lib/engine/src/resolver.rs +++ b/lib/engine/src/resolver.rs @@ -1,15 +1,15 @@ //! Define the `Resolver` trait, allowing custom resolution for external //! references. -use crate::{Export, ImportError, LinkError}; +use crate::{Export, ExportFunctionMetadata, ImportError, LinkError}; use more_asserts::assert_ge; use wasmer_types::entity::{BoxedSlice, EntityRef, PrimaryMap}; use wasmer_types::{ExternType, FunctionIndex, ImportIndex, MemoryIndex, TableIndex}; use wasmer_vm::{ - FunctionBodyPtr, Imports, MemoryStyle, ModuleInfo, TableStyle, VMDynamicFunctionContext, - VMFunctionBody, VMFunctionImport, VMFunctionKind, VMGlobalImport, VMMemoryImport, - VMTableImport, + FunctionBodyPtr, ImportFunctionEnv, Imports, MemoryStyle, ModuleInfo, TableStyle, + VMFunctionBody, VMFunctionEnvironment, VMFunctionImport, VMFunctionKind, VMGlobalImport, + VMMemoryImport, VMTableImport, }; /// Import resolver connects imports with available exported values. @@ -155,36 +155,13 @@ pub fn resolve_imports( } match resolved { Export::Function(ref f) => { - let (address, env_ptr) = match f.vm_function.kind { + let address = match f.vm_function.kind { VMFunctionKind::Dynamic => { // If this is a dynamic imported function, // the address of the function is the address of the // reverse trampoline. let index = FunctionIndex::new(function_imports.len()); - let address = finished_dynamic_function_trampolines[index].0 - as *mut VMFunctionBody as _; - let env_ptr = if f.import_init_function_ptr.is_some() { - // Our function env looks like: - // Box>> - // Which we can interpret as `*const *const Env` (due to - // the precise layout of these types via `repr(C)`) - // We extract the `*const Env`: - unsafe { - // Box> - let dyn_func_ctx_ptr = f.vm_function.vmctx.host_env - as *mut VMDynamicFunctionContext<*mut std::ffi::c_void>; - // maybe report error here if it's null? - // invariants of these types are not enforced. - - // &VMDynamicFunctionContext<...> - let dyn_func_ctx = &*dyn_func_ctx_ptr; - dyn_func_ctx.ctx - } - } else { - std::ptr::null_mut() - }; - - (address, env_ptr) + finished_dynamic_function_trampolines[index].0 as *mut VMFunctionBody as _ // TODO: We should check that the f.vmctx actually matches // the shape of `VMDynamicFunctionImportContext` @@ -198,17 +175,51 @@ pub fn resolve_imports( assert!(num_params < 9, "Only native functions with less than 9 arguments are allowed in Apple Silicon (for now). Received {} in the import {}.{}", num_params, module_name, field); } - (f.vm_function.address, unsafe { - f.vm_function.vmctx.host_env - }) + f.vm_function.address + } + }; + + // Clone the host env for this `Instance`. + let env = if let Some(ExportFunctionMetadata { + host_env_clone_fn: clone, + .. + }) = f.metadata.as_ref().map(|x| &**x) + { + // TODO: maybe start adding asserts in all these + // unsafe blocks to prevent future changes from + // horribly breaking things. + unsafe { + assert!(!f.vm_function.vmctx.host_env.is_null()); + (clone)(f.vm_function.vmctx.host_env) } + } else { + // No `clone` function means we're dealing with some + // other kind of `vmctx`, not a host env of any + // kind. + unsafe { f.vm_function.vmctx.host_env } }; + function_imports.push(VMFunctionImport { body: address, - environment: f.vm_function.vmctx, + environment: VMFunctionEnvironment { host_env: env }, }); - host_function_env_initializers.push((f.import_init_function_ptr, env_ptr)); + let initializer = f.metadata.as_ref().and_then(|m| m.import_init_function_ptr); + let clone = f.metadata.as_ref().map(|m| m.host_env_clone_fn); + let destructor = f.metadata.as_ref().map(|m| m.host_env_drop_fn); + let import_function_env = + if let (Some(clone), Some(destructor)) = (clone, destructor) { + ImportFunctionEnv::Env { + env, + clone, + initializer, + destructor, + } + } else { + ImportFunctionEnv::NoEnv + }; + + host_function_env_initializers.push(import_function_env); } Export::Table(ref t) => { table_imports.push(VMTableImport { diff --git a/lib/vm/src/export.rs b/lib/vm/src/export.rs index 3497b4db756..e5d3475ea10 100644 --- a/lib/vm/src/export.rs +++ b/lib/vm/src/export.rs @@ -10,7 +10,7 @@ use std::sync::Arc; use wasmer_types::{FunctionType, MemoryType, TableType}; /// The value of an export passed from one instance to another. -#[derive(Debug, Clone)] +#[derive(Debug)] pub enum VMExport { /// A function export value. Function(VMExportFunction), diff --git a/lib/vm/src/imports.rs b/lib/vm/src/imports.rs index 0f9482e4092..22bead4f730 100644 --- a/lib/vm/src/imports.rs +++ b/lib/vm/src/imports.rs @@ -1,7 +1,7 @@ // This file contains code from external sources. // Attributions: https://github.com/wasmerio/wasmer/blob/master/ATTRIBUTIONS.md -use crate::instance::{ImportInitializerFuncPtr, ImportInitializerThunks}; +use crate::instance::ImportFunctionEnv; use crate::vmcontext::{VMFunctionImport, VMGlobalImport, VMMemoryImport, VMTableImport}; use wasmer_types::entity::{BoxedSlice, PrimaryMap}; use wasmer_types::{FunctionIndex, GlobalIndex, MemoryIndex, TableIndex}; @@ -17,9 +17,12 @@ pub struct Imports { /// space may affect Wasm runtime performance due to increased cache pressure. /// /// We make it optional so that we can free the data after use. - pub host_function_env_initializers: Option< - BoxedSlice, *mut std::ffi::c_void)>, - >, + /// + /// We move this data in `get_imported_function_envs` because there's + /// no value to keeping it around; host functions must be initialized + /// exactly once so we save some memory and improve correctness by + /// moving this data. + pub host_function_env_initializers: Option>, /// Resolved addresses for imported tables. pub tables: BoxedSlice, @@ -35,10 +38,7 @@ impl Imports { /// Construct a new `Imports` instance. pub fn new( function_imports: PrimaryMap, - host_function_env_initializers: PrimaryMap< - FunctionIndex, - (Option, *mut std::ffi::c_void), - >, + host_function_env_initializers: PrimaryMap, table_imports: PrimaryMap, memory_imports: PrimaryMap, global_imports: PrimaryMap, @@ -56,7 +56,7 @@ impl Imports { pub fn none() -> Self { Self { functions: PrimaryMap::new().into_boxed_slice(), - host_function_env_initializers: Some(PrimaryMap::new().into_boxed_slice()), + host_function_env_initializers: None, tables: PrimaryMap::new().into_boxed_slice(), memories: PrimaryMap::new().into_boxed_slice(), globals: PrimaryMap::new().into_boxed_slice(), @@ -68,25 +68,9 @@ impl Imports { /// /// This function can only be called once, it deletes the data it returns after /// returning it to ensure that it's not called more than once. - pub fn get_import_initializers(&mut self) -> ImportInitializerThunks { - let result = if let Some(inner) = &self.host_function_env_initializers { - inner - .values() - .cloned() - .map(|(func_init, env_ptr)| { - let host_env = if func_init.is_some() { - env_ptr - } else { - std::ptr::null_mut() - }; - (func_init, host_env) - }) - .collect() - } else { - vec![] - }; - // ensure we only call these functions once and free this now useless memory. - self.host_function_env_initializers = None; - result + pub fn get_imported_function_envs(&mut self) -> BoxedSlice { + self.host_function_env_initializers + .take() + .unwrap_or_else(|| PrimaryMap::new().into_boxed_slice()) } } diff --git a/lib/vm/src/instance/mod.rs b/lib/vm/src/instance/mod.rs index 410a30574d9..44a14cf560e 100644 --- a/lib/vm/src/instance/mod.rs +++ b/lib/vm/src/instance/mod.rs @@ -49,11 +49,6 @@ use wasmer_types::{ pub type ImportInitializerFuncPtr = fn(*mut std::ffi::c_void, *const std::ffi::c_void) -> Result<(), *mut std::ffi::c_void>; -/// This type holds thunks (delayed computations) for initializing the imported -/// function's environments with the [`Instance`]. -pub(crate) type ImportInitializerThunks = - Vec<(Option, *mut std::ffi::c_void)>; - /// A WebAssembly instance. /// /// The type is dynamically-sized. Indeed, the `vmctx` field can @@ -98,13 +93,12 @@ pub(crate) struct Instance { /// Handler run when `SIGBUS`, `SIGFPE`, `SIGILL`, or `SIGSEGV` are caught by the instance thread. pub(crate) signal_handler: Cell>>, - /// Functions to initialize the host environments in the imports - /// and pointers to the environments. These function pointers all - /// come from `WasmerEnv::init_with_instance`. + /// Functions to operate on host environments in the imports + /// and pointers to the environments. /// /// TODO: Be sure to test with serialize/deserialize and imported /// functions from other Wasm modules. - import_initializers: ImportInitializerThunks, + imported_function_envs: BoxedSlice, /// Additional context used by compiled WebAssembly code. This /// field is last, and represents a dynamically-sized array that @@ -113,6 +107,87 @@ pub(crate) struct Instance { vmctx: VMContext, } +/// A collection of data about host envs used by imported functions. +#[derive(Debug)] +pub enum ImportFunctionEnv { + /// The `vmctx` pointer does not refer to a host env, there is no + /// metadata about it. + NoEnv, + /// We're dealing with a user-defined host env. + /// + /// This host env may be either unwrapped (the user-supplied host env + /// directly) or wrapped. i.e. in the case of Dynamic functions, we + /// store our own extra data along with the user supplied env, + /// thus the `env` pointer here points to the outermost type. + Env { + /// The function environment. This is not always the user-supplied + /// env. + env: *mut std::ffi::c_void, + /// A clone function for duplicating the env. + clone: fn(*mut std::ffi::c_void) -> *mut std::ffi::c_void, + /// This field is not always present. When it is present, it + /// should be set to `None` after use to prevent double + /// initialization. + initializer: Option, + /// The destructor to clean up the type in `env`. + /// + /// # Safety + /// - This function must be called ina synchronized way. For + /// example, in the `Drop` implementation of this type. + destructor: unsafe fn(*mut std::ffi::c_void), + }, +} + +impl ImportFunctionEnv { + /// Get the `initializer` function pointer if it exists. + fn initializer(&self) -> Option { + match self { + Self::Env { initializer, .. } => *initializer, + _ => None, + } + } +} + +impl Clone for ImportFunctionEnv { + fn clone(&self) -> Self { + match &self { + Self::NoEnv => Self::NoEnv, + Self::Env { + env, + clone, + destructor, + initializer, + } => { + let new_env = (*clone)(*env); + Self::Env { + env: new_env, + clone: *clone, + destructor: *destructor, + initializer: *initializer, + } + } + } + } +} + +impl Drop for ImportFunctionEnv { + fn drop(&mut self) { + match self { + ImportFunctionEnv::Env { + env, destructor, .. + } => { + // # Safety + // - This is correct because we know no other references + // to this data can exist if we're dropping it. + unsafe { + (destructor)(*env); + } + } + ImportFunctionEnv::NoEnv => (), + } + } +} + impl fmt::Debug for Instance { fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result { formatter.debug_struct("Instance").finish() @@ -159,7 +234,7 @@ impl Instance { &self, index: FunctionIndex, ) -> Option { - self.import_initializers[index.as_u32() as usize].0 + self.imported_function_envs[index].initializer() } /// Return a pointer to the `VMFunctionImport`s. @@ -943,7 +1018,7 @@ impl InstanceHandle { imports: Imports, vmshared_signatures: BoxedSlice, host_state: Box, - import_initializers: ImportInitializerThunks, + imported_function_envs: BoxedSlice, ) -> Result { let vmctx_globals = finished_globals .values() @@ -967,7 +1042,7 @@ impl InstanceHandle { passive_data, host_state, signal_handler: Cell::new(None), - import_initializers, + imported_function_envs, vmctx: VMContext {}, }; @@ -1255,19 +1330,26 @@ impl InstanceHandle { ) -> Result<(), Err> { let instance_ref = self.instance.as_mut(); - for (func, env) in instance_ref.import_initializers.drain(..) { - if let Some(ref f) = func { - // transmute our function pointer into one with the correct error type - let f = mem::transmute::< - &ImportInitializerFuncPtr, - &fn(*mut ffi::c_void, *const ffi::c_void) -> Result<(), Err>, - >(f); - f(env, instance_ptr)?; + for import_function_env in instance_ref.imported_function_envs.values_mut() { + match import_function_env { + ImportFunctionEnv::Env { + env, + ref mut initializer, + .. + } => { + if let Some(f) = initializer { + // transmute our function pointer into one with the correct error type + let f = mem::transmute::< + &ImportInitializerFuncPtr, + &fn(*mut ffi::c_void, *const ffi::c_void) -> Result<(), Err>, + >(f); + f(*env, instance_ptr)?; + } + *initializer = None; + } + ImportFunctionEnv::NoEnv => (), } } - // free memory now that it's empty. - instance_ref.import_initializers.shrink_to_fit(); - Ok(()) } } diff --git a/lib/vm/src/lib.rs b/lib/vm/src/lib.rs index 1d7a6358212..c46eab9fe68 100644 --- a/lib/vm/src/lib.rs +++ b/lib/vm/src/lib.rs @@ -39,7 +39,9 @@ pub mod libcalls; pub use crate::export::*; pub use crate::global::*; pub use crate::imports::Imports; -pub use crate::instance::{ImportInitializerFuncPtr, InstanceAllocator, InstanceHandle}; +pub use crate::instance::{ + ImportFunctionEnv, ImportInitializerFuncPtr, InstanceAllocator, InstanceHandle, +}; pub use crate::memory::{LinearMemory, Memory, MemoryError, MemoryStyle}; pub use crate::mmap::Mmap; pub use crate::module::{ExportsIterator, ImportsIterator, ModuleInfo}; diff --git a/lib/vm/src/vmcontext.rs b/lib/vm/src/vmcontext.rs index 43bdcd59d1f..4f478485423 100644 --- a/lib/vm/src/vmcontext.rs +++ b/lib/vm/src/vmcontext.rs @@ -106,6 +106,15 @@ pub struct VMDynamicFunctionContext { pub ctx: T, } +impl Clone for VMDynamicFunctionContext { + fn clone(&self) -> Self { + Self { + address: self.address, + ctx: self.ctx.clone(), + } + } +} + #[cfg(test)] mod test_vmdynamicfunction_import_context { use super::VMDynamicFunctionContext; diff --git a/tests/compilers/imports.rs b/tests/compilers/imports.rs index 5751f4798f9..fa6317d6477 100644 --- a/tests/compilers/imports.rs +++ b/tests/compilers/imports.rs @@ -343,3 +343,50 @@ fn dynamic_function_with_env_wasmer_env_init_works() -> Result<()> { f.call()?; Ok(()) } + +#[test] +fn multi_use_host_fn_manages_memory_correctly() -> Result<()> { + let store = get_store(false); + let module = get_module2(&store)?; + + #[allow(dead_code)] + #[derive(Clone)] + struct Env { + memory: LazyInit, + }; + + impl WasmerEnv for Env { + fn init_with_instance(&mut self, instance: &Instance) -> Result<(), HostEnvInitError> { + let memory = instance.exports.get_memory("memory")?.clone(); + self.memory.initialize(memory); + Ok(()) + } + } + + let env: Env = Env { + memory: LazyInit::default(), + }; + fn host_fn(env: &Env) { + assert!(env.memory.get_ref().is_some()); + println!("Hello, world!"); + } + + let imports = imports! { + "host" => { + "fn" => Function::new_native_with_env(&store, env.clone(), host_fn), + }, + }; + let instance1 = Instance::new(&module, &imports)?; + let instance2 = Instance::new(&module, &imports)?; + { + let f1: NativeFunc<(), ()> = instance1.exports.get_native_function("main")?; + f1.call()?; + } + drop(instance1); + { + let f2: NativeFunc<(), ()> = instance2.exports.get_native_function("main")?; + f2.call()?; + } + drop(instance2); + Ok(()) +} diff --git a/tests/compilers/native_functions.rs b/tests/compilers/native_functions.rs index f8bfe33d58a..a3400193076 100644 --- a/tests/compilers/native_functions.rs +++ b/tests/compilers/native_functions.rs @@ -1,8 +1,7 @@ use crate::utils::get_store; use anyhow::Result; -use std::cell::RefCell; use std::convert::Infallible; -use std::rc::Rc; +use std::sync::{Arc, Mutex}; use wasmer::*; @@ -315,24 +314,26 @@ fn static_host_function_with_env() -> anyhow::Result<()> { let store = get_store(false); fn f(env: &Env, a: i32, b: i64, c: f32, d: f64) -> (f64, f32, i64, i32) { - assert_eq!(*env.0.borrow(), 100); - env.0.replace(101); + let mut guard = env.0.lock().unwrap(); + assert_eq!(*guard, 100); + *guard = 101; (d * 4.0, c * 3.0, b * 2, a * 1) } fn f_ok(env: &Env, a: i32, b: i64, c: f32, d: f64) -> Result<(f64, f32, i64, i32), Infallible> { - assert_eq!(*env.0.borrow(), 100); - env.0.replace(101); + let mut guard = env.0.lock().unwrap(); + assert_eq!(*guard, 100); + *guard = 101; Ok((d * 4.0, c * 3.0, b * 2, a * 1)) } #[derive(WasmerEnv, Clone)] - struct Env(Rc>); + struct Env(Arc>); impl std::ops::Deref for Env { - type Target = Rc>; + type Target = Arc>; fn deref(&self) -> &Self::Target { &self.0 } @@ -340,32 +341,32 @@ fn static_host_function_with_env() -> anyhow::Result<()> { // Native static host function that returns a tuple. { - let env = Env(Rc::new(RefCell::new(100))); + let env = Env(Arc::new(Mutex::new(100))); let f = Function::new_native_with_env(&store, env.clone(), f); let f_native: NativeFunc<(i32, i64, f32, f64), (f64, f32, i64, i32)> = f.native().unwrap(); - assert_eq!(*env.0.borrow(), 100); + assert_eq!(*env.0.lock().unwrap(), 100); let result = f_native.call(1, 3, 5.0, 7.0)?; assert_eq!(result, (28.0, 15.0, 6, 1)); - assert_eq!(*env.0.borrow(), 101); + assert_eq!(*env.0.lock().unwrap(), 101); } // Native static host function that returns a result of a tuple. { - let env = Env(Rc::new(RefCell::new(100))); + let env = Env(Arc::new(Mutex::new(100))); let f = Function::new_native_with_env(&store, env.clone(), f_ok); let f_native: NativeFunc<(i32, i64, f32, f64), (f64, f32, i64, i32)> = f.native().unwrap(); - assert_eq!(*env.0.borrow(), 100); + assert_eq!(*env.0.lock().unwrap(), 100); let result = f_native.call(1, 3, 5.0, 7.0)?; assert_eq!(result, (28.0, 15.0, 6, 1)); - assert_eq!(*env.0.borrow(), 101); + assert_eq!(*env.0.lock().unwrap(), 101); } Ok(()) @@ -403,16 +404,16 @@ fn dynamic_host_function_with_env() -> anyhow::Result<()> { let store = get_store(false); #[derive(WasmerEnv, Clone)] - struct Env(Rc>); + struct Env(Arc>); impl std::ops::Deref for Env { - type Target = Rc>; + type Target = Arc>; fn deref(&self) -> &Self::Target { &self.0 } } - let env = Env(Rc::new(RefCell::new(100))); + let env = Env(Arc::new(Mutex::new(100))); let f = Function::new_with_env( &store, FunctionType::new( @@ -421,9 +422,10 @@ fn dynamic_host_function_with_env() -> anyhow::Result<()> { ), env.clone(), |env, values| { - assert_eq!(*env.0.borrow(), 100); + let mut guard = env.0.lock().unwrap(); + assert_eq!(*guard, 100); - env.0.replace(101); + *guard = 101; Ok(vec![ Value::F64(values[3].unwrap_f64() * 4.0), @@ -436,12 +438,12 @@ fn dynamic_host_function_with_env() -> anyhow::Result<()> { let f_native: NativeFunc<(i32, i64, f32, f64), (f64, f32, i64, i32)> = f.native().unwrap(); - assert_eq!(*env.0.borrow(), 100); + assert_eq!(*env.0.lock().unwrap(), 100); let result = f_native.call(1, 3, 5.0, 7.0)?; assert_eq!(result, (28.0, 15.0, 6, 1)); - assert_eq!(*env.0.borrow(), 101); + assert_eq!(*env.0.lock().unwrap(), 101); Ok(()) }