diff --git a/CHANGELOG.md b/CHANGELOG.md index a7fc541716b..20432fe832c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ Blocks of changes will separated by version increments. ## **[Unreleased]** +- [#807](https://github.com/wasmerio/wasmer/pull/807) Implement Send for `Instance`, breaking change on `ImportObject`, remove method `get_namespace` replaced with `with_namespace` and `maybe_with_namespace` - [#817](https://github.com/wasmerio/wasmer/pull/817) Add document for tracking features across backends and language integrations, [docs/feature_matrix.md] - [#823](https://github.com/wasmerio/wasmer/issues/823) Improved Emscripten / WASI integration - [#821](https://github.com/wasmerio/wasmer/issues/821) Remove patch version on most deps Cargo manifests. This gives Wasmer library users more control over which versions of the deps they use. diff --git a/lib/runtime-core/src/backing.rs b/lib/runtime-core/src/backing.rs index 814dc56e79c..ef43bd58c64 100644 --- a/lib/runtime-core/src/backing.rs +++ b/lib/runtime-core/src/backing.rs @@ -54,6 +54,9 @@ pub struct LocalBacking { pub(crate) internals: Internals, } +// Manually implemented because LocalBacking contains raw pointers directly +unsafe impl Send for LocalBacking {} + impl LocalBacking { pub(crate) fn new( module: &ModuleInner, @@ -461,6 +464,9 @@ pub struct ImportBacking { pub(crate) vm_globals: BoxedMap, } +// manually implemented because ImportBacking contains raw pointers directly +unsafe impl Send for ImportBacking {} + impl ImportBacking { pub fn new( module: &ModuleInner, @@ -536,9 +542,8 @@ fn import_functions( let namespace = module.info.namespace_table.get(*namespace_index); let name = module.info.name_table.get(*name_index); - let import = imports - .get_namespace(namespace) - .and_then(|namespace| namespace.get_export(name)); + let import = + imports.maybe_with_namespace(namespace, |namespace| namespace.get_export(name)); match import { Some(Export::Function { func, @@ -624,9 +629,8 @@ fn import_memories( let namespace = module.info.namespace_table.get(*namespace_index); let name = module.info.name_table.get(*name_index); - let memory_import = imports - .get_namespace(&namespace) - .and_then(|namespace| namespace.get_export(&name)); + let memory_import = + imports.maybe_with_namespace(namespace, |namespace| namespace.get_export(name)); match memory_import { Some(Export::Memory(memory)) => { if expected_memory_desc.fits_in_imported(memory.descriptor()) { @@ -696,9 +700,8 @@ fn import_tables( let namespace = module.info.namespace_table.get(*namespace_index); let name = module.info.name_table.get(*name_index); - let table_import = imports - .get_namespace(&namespace) - .and_then(|namespace| namespace.get_export(&name)); + let table_import = + imports.maybe_with_namespace(namespace, |namespace| namespace.get_export(name)); match table_import { Some(Export::Table(mut table)) => { if expected_table_desc.fits_in_imported(table.descriptor()) { @@ -767,9 +770,8 @@ fn import_globals( { let namespace = module.info.namespace_table.get(*namespace_index); let name = module.info.name_table.get(*name_index); - let import = imports - .get_namespace(namespace) - .and_then(|namespace| namespace.get_export(name)); + let import = + imports.maybe_with_namespace(namespace, |namespace| namespace.get_export(name)); match import { Some(Export::Global(mut global)) => { if global.descriptor() == *imported_global_desc { diff --git a/lib/runtime-core/src/export.rs b/lib/runtime-core/src/export.rs index 52ea9cf618e..7960d76e699 100644 --- a/lib/runtime-core/src/export.rs +++ b/lib/runtime-core/src/export.rs @@ -11,6 +11,9 @@ pub enum Context { Internal, } +// Manually implemented because context contains a raw pointer to Ctx +unsafe impl Send for Context {} + #[derive(Debug, Clone)] pub enum Export { Function { @@ -26,6 +29,9 @@ pub enum Export { #[derive(Debug, Clone)] pub struct FuncPointer(*const vm::Func); +// Manually implemented because FuncPointer contains a raw pointer to Ctx +unsafe impl Send for FuncPointer {} + impl FuncPointer { /// This needs to be unsafe because there is /// no way to check whether the passed function diff --git a/lib/runtime-core/src/global.rs b/lib/runtime-core/src/global.rs index 8d421253112..b59d1599b8e 100644 --- a/lib/runtime-core/src/global.rs +++ b/lib/runtime-core/src/global.rs @@ -4,11 +4,14 @@ use crate::{ types::{GlobalDescriptor, Type, Value}, vm, }; -use std::{cell::RefCell, fmt, rc::Rc}; +use std::{ + fmt, + sync::{Arc, Mutex}, +}; pub struct Global { desc: GlobalDescriptor, - storage: Rc>, + storage: Arc>, } impl Global { @@ -56,7 +59,7 @@ impl Global { Self { desc, - storage: Rc::new(RefCell::new(local_global)), + storage: Arc::new(Mutex::new(local_global)), } } @@ -83,7 +86,8 @@ impl Global { Value::V128(x) => x, }, }; - *self.storage.borrow_mut() = local_global; + let mut storage = self.storage.lock().unwrap(); + *storage = local_global; } else { panic!("Wrong type for setting this global") } @@ -94,7 +98,8 @@ impl Global { /// Get the value held by this global. pub fn get(&self) -> Value { - let data = self.storage.borrow().data; + let storage = self.storage.lock().unwrap(); + let data = storage.data; match self.desc.ty { Type::I32 => Value::I32(data as i32), @@ -105,8 +110,10 @@ impl Global { } } + // TODO: think about this and if this should now be unsafe pub(crate) fn vm_local_global(&mut self) -> *mut vm::LocalGlobal { - &mut *self.storage.borrow_mut() + let mut storage = self.storage.lock().unwrap(); + &mut *storage } } @@ -120,7 +127,7 @@ impl Clone for Global { fn clone(&self) -> Self { Self { desc: self.desc, - storage: Rc::clone(&self.storage), + storage: Arc::clone(&self.storage), } } } diff --git a/lib/runtime-core/src/import.rs b/lib/runtime-core/src/import.rs index fc1da43b730..2cccf2325af 100644 --- a/lib/runtime-core/src/import.rs +++ b/lib/runtime-core/src/import.rs @@ -2,9 +2,9 @@ use crate::export::Export; use std::collections::VecDeque; use std::collections::{hash_map::Entry, HashMap}; use std::{ - cell::{Ref, RefCell}, + borrow::{Borrow, BorrowMut}, ffi::c_void, - rc::Rc, + sync::{Arc, Mutex}, }; pub trait LikeNamespace { @@ -45,8 +45,9 @@ impl IsExport for Export { /// } /// ``` pub struct ImportObject { - map: Rc>>>, - pub(crate) state_creator: Option (*mut c_void, fn(*mut c_void))>>, + map: Arc>>>, + pub(crate) state_creator: + Option (*mut c_void, fn(*mut c_void)) + Send + Sync + 'static>>, pub allow_missing_functions: bool, } @@ -54,7 +55,7 @@ impl ImportObject { /// Create a new `ImportObject`. pub fn new() -> Self { Self { - map: Rc::new(RefCell::new(HashMap::new())), + map: Arc::new(Mutex::new(HashMap::new())), state_creator: None, allow_missing_functions: false, } @@ -62,11 +63,11 @@ impl ImportObject { pub fn new_with_data(state_creator: F) -> Self where - F: Fn() -> (*mut c_void, fn(*mut c_void)) + 'static, + F: Fn() -> (*mut c_void, fn(*mut c_void)) + 'static + Send + Sync, { Self { - map: Rc::new(RefCell::new(HashMap::new())), - state_creator: Some(Rc::new(state_creator)), + map: Arc::new(Mutex::new(HashMap::new())), + state_creator: Some(Arc::new(state_creator)), allow_missing_functions: false, } } @@ -92,9 +93,10 @@ impl ImportObject { pub fn register(&mut self, name: S, namespace: N) -> Option> where S: Into, - N: LikeNamespace + 'static, + N: LikeNamespace + Send + 'static, { - let mut map = self.map.borrow_mut(); + let mut guard = self.map.lock().unwrap(); + let map = guard.borrow_mut(); match map.entry(name.into()) { Entry::Vacant(empty) => { @@ -105,19 +107,39 @@ impl ImportObject { } } - pub fn get_namespace(&self, namespace: &str) -> Option> { - let map_ref = self.map.borrow(); - + /// Apply a function on the namespace if it exists + /// If your function can fail, consider using `maybe_with_namespace` + pub fn with_namespace(&self, namespace: &str, f: Func) -> Option + where + Func: FnOnce(&(dyn LikeNamespace + Send)) -> InnerRet, + InnerRet: Sized, + { + let guard = self.map.lock().unwrap(); + let map_ref = guard.borrow(); if map_ref.contains_key(namespace) { - Some(Ref::map(map_ref, |map| &*map[namespace])) + Some(f(map_ref[namespace].as_ref())) } else { None } } + /// The same as `with_namespace` but takes a function that may fail + pub fn maybe_with_namespace(&self, namespace: &str, f: Func) -> Option + where + Func: FnOnce(&(dyn LikeNamespace + Send)) -> Option, + InnerRet: Sized, + { + let guard = self.map.lock().unwrap(); + let map_ref = guard.borrow(); + map_ref + .get(namespace) + .map(|ns| ns.as_ref()) + .and_then(|ns| f(ns)) + } + pub fn clone_ref(&self) -> Self { Self { - map: Rc::clone(&self.map), + map: Arc::clone(&self.map), state_creator: self.state_creator.clone(), allow_missing_functions: false, } @@ -125,7 +147,9 @@ impl ImportObject { fn get_objects(&self) -> VecDeque<(String, String, Export)> { let mut out = VecDeque::new(); - for (name, ns) in self.map.borrow().iter() { + let guard = self.map.lock().unwrap(); + let map = guard.borrow(); + for (name, ns) in map.iter() { for (id, exp) in ns.get_exports() { out.push_back((name.clone(), id, exp)); } @@ -158,7 +182,8 @@ impl IntoIterator for ImportObject { impl Extend<(String, String, Export)> for ImportObject { fn extend>(&mut self, iter: T) { - let mut map = self.map.borrow_mut(); + let mut guard = self.map.lock().unwrap(); + let map = guard.borrow_mut(); for (ns, id, exp) in iter.into_iter() { if let Some(like_ns) = map.get_mut(&ns) { like_ns.maybe_insert(&id, exp); @@ -172,7 +197,7 @@ impl Extend<(String, String, Export)> for ImportObject { } pub struct Namespace { - map: HashMap>, + map: HashMap>, } impl Namespace { @@ -182,10 +207,10 @@ impl Namespace { } } - pub fn insert(&mut self, name: S, export: E) -> Option> + pub fn insert(&mut self, name: S, export: E) -> Option> where S: Into, - E: IsExport + 'static, + E: IsExport + Send + 'static, { self.map.insert(name.into(), Box::new(export)) } @@ -241,12 +266,14 @@ mod test { imports1.extend(imports2); - let cat_ns = imports1.get_namespace("cat").unwrap(); - assert!(cat_ns.get_export("small").is_some()); + let small_cat_export = + imports1.maybe_with_namespace("cat", |cat_ns| cat_ns.get_export("small")); + assert!(small_cat_export.is_some()); - let dog_ns = imports1.get_namespace("dog").unwrap(); - assert!(dog_ns.get_export("happy").is_some()); - assert!(dog_ns.get_export("small").is_some()); + let entries = imports1.maybe_with_namespace("dog", |dog_ns| { + Some((dog_ns.get_export("happy")?, dog_ns.get_export("small")?)) + }); + assert!(entries.is_some()); } #[test] @@ -264,14 +291,14 @@ mod test { }; imports1.extend(imports2); - let dog_ns = imports1.get_namespace("dog").unwrap(); + let happy_dog_entry = imports1 + .maybe_with_namespace("dog", |dog_ns| dog_ns.get_export("happy")) + .unwrap(); - assert!( - if let Export::Global(happy_dog_global) = dog_ns.get_export("happy").unwrap() { - happy_dog_global.get() == Value::I32(4) - } else { - false - } - ); + assert!(if let Export::Global(happy_dog_global) = happy_dog_entry { + happy_dog_global.get() == Value::I32(4) + } else { + false + }); } } diff --git a/lib/runtime-core/src/instance.rs b/lib/runtime-core/src/instance.rs index f1ce0a19376..85e9f8f768b 100644 --- a/lib/runtime-core/src/instance.rs +++ b/lib/runtime-core/src/instance.rs @@ -16,7 +16,12 @@ use crate::{ vm::{self, InternalField}, }; use smallvec::{smallvec, SmallVec}; -use std::{mem, pin::Pin, ptr::NonNull, sync::Arc}; +use std::{ + mem, + pin::Pin, + ptr::NonNull, + sync::{Arc, Mutex}, +}; pub(crate) struct InstanceInner { #[allow(dead_code)] @@ -25,6 +30,9 @@ pub(crate) struct InstanceInner { pub(crate) vmctx: *mut vm::Ctx, } +// manually implemented because InstanceInner contains a raw pointer to Ctx +unsafe impl Send for InstanceInner {} + impl Drop for InstanceInner { fn drop(&mut self) { // Drop the vmctx. @@ -517,6 +525,27 @@ impl LikeNamespace for Rc { } } +impl LikeNamespace for Arc> { + fn get_export(&self, name: &str) -> Option { + let instance = self.lock().unwrap(); + let export_index = instance.module.info.exports.get(name)?; + + Some( + instance + .inner + .get_export_from_index(&instance.module, export_index), + ) + } + + fn get_exports(&self) -> Vec<(String, Export)> { + unimplemented!("Use the exports method instead"); + } + + fn maybe_insert(&mut self, _name: &str, _export: Export) -> Option<()> { + None + } +} + #[must_use] fn call_func_with_index( info: &ModuleInfo, @@ -763,3 +792,15 @@ impl<'a> DynFunc<'a> { } } } + +#[cfg(test)] +mod test { + use super::*; + + fn is_send() {} + + #[test] + fn test_instance_is_send() { + is_send::(); + } +} diff --git a/lib/runtime-core/src/memory/mod.rs b/lib/runtime-core/src/memory/mod.rs index 75e9ea007dc..7f272be37bb 100644 --- a/lib/runtime-core/src/memory/mod.rs +++ b/lib/runtime-core/src/memory/mod.rs @@ -8,12 +8,9 @@ use crate::{ units::Pages, vm, }; -use std::{ - cell::{Cell, RefCell}, - fmt, mem, - rc::Rc, - sync::Arc, -}; +use std::{cell::Cell, fmt, mem, sync::Arc}; + +use std::sync::Mutex as StdMutex; pub use self::dynamic::DynamicMemory; pub use self::static_::StaticMemory; @@ -208,14 +205,18 @@ enum UnsharedMemoryStorage { } pub struct UnsharedMemory { - internal: Rc, + internal: Arc, } struct UnsharedMemoryInternal { - storage: RefCell, + storage: StdMutex, local: Cell, } +// Manually implemented because UnsharedMemoryInternal uses `Cell` and is used in an Arc; +// this is safe because the lock for storage can be used to protect (seems like a weak reason: PLEASE REVIEW!) +unsafe impl Sync for UnsharedMemoryInternal {} + impl UnsharedMemory { pub fn new(desc: MemoryDescriptor) -> Result { let mut local = vm::LocalMemory { @@ -235,15 +236,15 @@ impl UnsharedMemory { }; Ok(Self { - internal: Rc::new(UnsharedMemoryInternal { - storage: RefCell::new(storage), + internal: Arc::new(UnsharedMemoryInternal { + storage: StdMutex::new(storage), local: Cell::new(local), }), }) } pub fn grow(&self, delta: Pages) -> Result { - let mut storage = self.internal.storage.borrow_mut(); + let mut storage = self.internal.storage.lock().unwrap(); let mut local = self.internal.local.get(); @@ -260,7 +261,7 @@ impl UnsharedMemory { } pub fn size(&self) -> Pages { - let storage = self.internal.storage.borrow(); + let storage = self.internal.storage.lock().unwrap(); match &*storage { UnsharedMemoryStorage::Dynamic(ref dynamic_memory) => dynamic_memory.size(), @@ -276,7 +277,7 @@ impl UnsharedMemory { impl Clone for UnsharedMemory { fn clone(&self) -> Self { UnsharedMemory { - internal: Rc::clone(&self.internal), + internal: Arc::clone(&self.internal), } } } @@ -286,11 +287,15 @@ pub struct SharedMemory { } pub struct SharedMemoryInternal { - memory: RefCell>, + memory: StdMutex>, local: Cell, lock: Mutex<()>, } +// Manually implemented because SharedMemoryInternal uses `Cell` and is used in Arc; +// this is safe because of `lock`; accesing `local` without locking `lock` is not safe (Maybe we could put the lock on Local then?) +unsafe impl Sync for SharedMemoryInternal {} + impl SharedMemory { fn new(desc: MemoryDescriptor) -> Result { let mut local = vm::LocalMemory { @@ -303,7 +308,7 @@ impl SharedMemory { Ok(Self { internal: Arc::new(SharedMemoryInternal { - memory: RefCell::new(memory), + memory: StdMutex::new(memory), local: Cell::new(local), lock: Mutex::new(()), }), @@ -313,15 +318,18 @@ impl SharedMemory { pub fn grow(&self, delta: Pages) -> Result { let _guard = self.internal.lock.lock(); let mut local = self.internal.local.get(); - let pages = self.internal.memory.borrow_mut().grow(delta, &mut local); + let mut memory = self.internal.memory.lock().unwrap(); + let pages = memory.grow(delta, &mut local); pages } pub fn size(&self) -> Pages { let _guard = self.internal.lock.lock(); - self.internal.memory.borrow_mut().size() + let memory = self.internal.memory.lock().unwrap(); + memory.size() } + // This function is scary, because the mutex is not locked here pub(crate) fn vm_local_memory(&self) -> *mut vm::LocalMemory { self.internal.local.as_ptr() } diff --git a/lib/runtime-core/src/sys/unix/memory.rs b/lib/runtime-core/src/sys/unix/memory.rs index f4af2493b61..fc60d732b6f 100644 --- a/lib/runtime-core/src/sys/unix/memory.rs +++ b/lib/runtime-core/src/sys/unix/memory.rs @@ -4,7 +4,7 @@ use errno; use nix::libc; use page_size; use std::ops::{Bound, RangeBounds}; -use std::{fs::File, os::unix::io::IntoRawFd, path::Path, ptr, rc::Rc, slice}; +use std::{fs::File, os::unix::io::IntoRawFd, path::Path, ptr, slice, sync::Arc}; unsafe impl Send for Memory {} unsafe impl Sync for Memory {} @@ -14,7 +14,7 @@ pub struct Memory { ptr: *mut u8, size: usize, protection: Protect, - fd: Option>, + fd: Option>, } impl Memory { @@ -49,7 +49,7 @@ impl Memory { ptr: ptr as *mut u8, size: file_len as usize, protection, - fd: Some(Rc::new(raw_fd)), + fd: Some(Arc::new(raw_fd)), }) } } diff --git a/lib/runtime-core/src/table/mod.rs b/lib/runtime-core/src/table/mod.rs index 1b61cd75438..8e5c0e03323 100644 --- a/lib/runtime-core/src/table/mod.rs +++ b/lib/runtime-core/src/table/mod.rs @@ -5,7 +5,10 @@ use crate::{ types::{ElementType, TableDescriptor}, vm, }; -use std::{cell::RefCell, fmt, ptr, rc::Rc}; +use std::{ + fmt, ptr, + sync::{Arc, Mutex}, +}; mod anyfunc; @@ -25,7 +28,7 @@ pub enum TableStorage { pub struct Table { desc: TableDescriptor, - storage: Rc>, + storage: Arc>, } impl Table { @@ -71,7 +74,7 @@ impl Table { Ok(Self { desc, - storage: Rc::new(RefCell::new((storage, local))), + storage: Arc::new(Mutex::new((storage, local))), }) } @@ -82,7 +85,8 @@ impl Table { /// Set the element at index. pub fn set(&self, index: u32, element: Element) -> Result<(), ()> { - match &mut *self.storage.borrow_mut() { + let mut storage = self.storage.lock().unwrap(); + match &mut *storage { (TableStorage::Anyfunc(ref mut anyfunc_table), _) => { match element { Element::Anyfunc(anyfunc) => anyfunc_table.set(index, anyfunc), @@ -96,14 +100,16 @@ impl Table { where F: FnOnce(&mut [vm::Anyfunc]) -> R, { - match &mut *self.storage.borrow_mut() { + let mut storage = self.storage.lock().unwrap(); + match &mut *storage { (TableStorage::Anyfunc(ref mut anyfunc_table), _) => f(anyfunc_table.internal_buffer()), } } /// The current size of this table. pub fn size(&self) -> u32 { - match &*self.storage.borrow() { + let storage = self.storage.lock().unwrap(); + match &*storage { (TableStorage::Anyfunc(ref anyfunc_table), _) => anyfunc_table.current_size(), } } @@ -114,7 +120,8 @@ impl Table { return Ok(self.size()); } - match &mut *self.storage.borrow_mut() { + let mut storage = self.storage.lock().unwrap(); + match &mut *storage { (TableStorage::Anyfunc(ref mut anyfunc_table), ref mut local) => anyfunc_table .grow(delta, local) .ok_or(GrowError::TableGrowError), @@ -122,7 +129,8 @@ impl Table { } pub fn vm_local_table(&mut self) -> *mut vm::LocalTable { - &mut self.storage.borrow_mut().1 + let mut storage = self.storage.lock().unwrap(); + &mut storage.1 } } @@ -136,7 +144,7 @@ impl Clone for Table { fn clone(&self) -> Self { Self { desc: self.desc, - storage: Rc::clone(&self.storage), + storage: Arc::clone(&self.storage), } } } diff --git a/lib/runtime-core/src/typed_func.rs b/lib/runtime-core/src/typed_func.rs index 91da26cbe37..0014e13cd7d 100644 --- a/lib/runtime-core/src/typed_func.rs +++ b/lib/runtime-core/src/typed_func.rs @@ -165,6 +165,9 @@ pub struct Func<'a, Args = (), Rets = (), Inner: Kind = Wasm> { _phantom: PhantomData<(&'a (), Args, Rets)>, } +unsafe impl<'a, Args, Rets> Send for Func<'a, Args, Rets, Wasm> {} +unsafe impl<'a, Args, Rets> Send for Func<'a, Args, Rets, Host> {} + impl<'a, Args, Rets> Func<'a, Args, Rets, Wasm> where Args: WasmTypeList, diff --git a/lib/runtime-core/src/vm.rs b/lib/runtime-core/src/vm.rs index e3084a9c5fa..fd8cebbec9a 100644 --- a/lib/runtime-core/src/vm.rs +++ b/lib/runtime-core/src/vm.rs @@ -513,6 +513,9 @@ pub struct ImportedFunc { pub vmctx: *mut Ctx, } +// manually implemented because ImportedFunc contains raw pointers directly; `Func` is marked Send (But `Ctx` actually isn't! (TODO: review this, shouldn't `Ctx` be Send?)) +unsafe impl Send for ImportedFunc {} + impl ImportedFunc { #[allow(clippy::erasing_op)] // TODO pub fn offset_func() -> u8 { @@ -540,6 +543,9 @@ pub struct LocalTable { pub table: *mut (), } +// manually implemented because LocalTable contains raw pointers directly +unsafe impl Send for LocalTable {} + impl LocalTable { #[allow(clippy::erasing_op)] // TODO pub fn offset_base() -> u8 { @@ -569,6 +575,9 @@ pub struct LocalMemory { pub memory: *mut (), } +// manually implemented because LocalMemory contains raw pointers +unsafe impl Send for LocalMemory {} + impl LocalMemory { #[allow(clippy::erasing_op)] // TODO pub fn offset_base() -> u8 { @@ -619,6 +628,9 @@ pub struct Anyfunc { pub sig_id: SigId, } +// manually implemented because Anyfunc contains raw pointers directly +unsafe impl Send for Anyfunc {} + impl Anyfunc { pub fn null() -> Self { Self { diff --git a/lib/spectests/tests/spectest.rs b/lib/spectests/tests/spectest.rs index 9cd18d636a8..66a730d7650 100644 --- a/lib/spectests/tests/spectest.rs +++ b/lib/spectests/tests/spectest.rs @@ -18,7 +18,7 @@ mod tests { // TODO Files could be run with multiple threads // TODO Allow running WAST &str directly (E.g. for use outside of spectests) - use std::rc::Rc; + use std::sync::{Arc, Mutex}; struct SpecFailure { file: String, @@ -119,6 +119,24 @@ mod tests { "unknown" } + fn with_instance( + maybe_instance: Option>>, + named_modules: &HashMap>>, + module: &Option, + f: F, + ) -> Option + where + R: Sized, + F: FnOnce(&Instance) -> R, + { + let ref ins = module + .as_ref() + .and_then(|name| named_modules.get(name).cloned()) + .or(maybe_instance)?; + let guard = ins.lock().unwrap(); + Some(f(guard.borrow())) + } + use glob::glob; use std::collections::HashMap; use std::fs; @@ -173,11 +191,11 @@ mod tests { .expect(&format!("Failed to parse script {}", &filename)); use std::panic; - let mut instance: Option> = None; + let mut instance: Option>> = None; - let mut named_modules: HashMap> = HashMap::new(); + let mut named_modules: HashMap>> = HashMap::new(); - let mut registered_modules: HashMap> = HashMap::new(); + let mut registered_modules: HashMap>> = HashMap::new(); // while let Some(Command { kind, line }) = @@ -236,9 +254,9 @@ mod tests { instance = None; } Ok(i) => { - let i = Rc::new(i); + let i = Arc::new(Mutex::new(i)); if name.is_some() { - named_modules.insert(name.unwrap(), Rc::clone(&i)); + named_modules.insert(name.unwrap(), Arc::clone(&i)); } instance = Some(i); } @@ -251,20 +269,17 @@ mod tests { field, args, } => { - let instance: Option<&Instance> = match module { - Some(ref name) => { - let i = named_modules.get(name); - match i { - Some(ins) => Some(ins.borrow()), - None => None, - } - } - None => match instance { - Some(ref i) => Some(i.borrow()), - None => None, + let maybe_call_result = with_instance( + instance.clone(), + &named_modules, + &module, + |instance| { + let params: Vec = + args.iter().cloned().map(convert_value).collect(); + instance.call(&field, ¶ms[..]) }, - }; - if instance.is_none() { + ); + if maybe_call_result.is_none() { test_report.add_failure( SpecFailure { file: filename.to_string(), @@ -276,9 +291,7 @@ mod tests { excludes, ); } else { - let params: Vec = - args.iter().cloned().map(|x| convert_value(x)).collect(); - let call_result = instance.unwrap().call(&field, ¶ms[..]); + let call_result = maybe_call_result.unwrap(); match call_result { Err(e) => { test_report.add_failure( @@ -316,20 +329,17 @@ mod tests { } } Action::Get { module, field } => { - let instance: Option<&Instance> = match module { - Some(ref name) => { - let i = named_modules.get(name); - match i { - Some(ins) => Some(ins.borrow()), - None => None, - } - } - None => match instance { - Some(ref i) => Some(i.borrow()), - None => None, + let maybe_call_result = with_instance( + instance.clone(), + &named_modules, + &module, + |instance| { + instance + .get_export(&field) + .expect(&format!("missing global {:?}", &field)) }, - }; - if instance.is_none() { + ); + if maybe_call_result.is_none() { test_report.add_failure( SpecFailure { file: filename.to_string(), @@ -341,10 +351,7 @@ mod tests { excludes, ); } else { - let export: Export = instance - .unwrap() - .get_export(&field) - .expect(&format!("missing global {:?}", &field)); + let export: Export = maybe_call_result.unwrap(); match export { Export::Global(g) => { let value = g.get(); @@ -392,20 +399,13 @@ mod tests { field, args, } => { - let instance: Option<&Instance> = match module { - Some(ref name) => { - let i = named_modules.get(name); - match i { - Some(ins) => Some(ins.borrow()), - None => None, - } - } - None => match instance { - Some(ref i) => Some(i.borrow()), - None => None, - }, - }; - if instance.is_none() { + let maybe_call_result = + with_instance(instance.clone(), &named_modules, &module, |instance| { + let params: Vec = + args.iter().cloned().map(convert_value).collect(); + instance.call(&field, ¶ms[..]) + }); + if maybe_call_result.is_none() { test_report.add_failure( SpecFailure { file: filename.to_string(), @@ -417,9 +417,7 @@ mod tests { excludes, ); } else { - let params: Vec = - args.iter().cloned().map(|x| convert_value(x)).collect(); - let call_result = instance.unwrap().call(&field, ¶ms[..]); + let call_result = maybe_call_result.unwrap(); match call_result { Err(e) => { test_report.add_failure( @@ -468,20 +466,13 @@ mod tests { field, args, } => { - let instance: Option<&Instance> = match module { - Some(ref name) => { - let i = named_modules.get(name); - match i { - Some(ins) => Some(ins.borrow()), - None => None, - } - } - None => match instance { - Some(ref i) => Some(i.borrow()), - None => None, - }, - }; - if instance.is_none() { + let maybe_call_result = + with_instance(instance.clone(), &named_modules, &module, |instance| { + let params: Vec = + args.iter().cloned().map(convert_value).collect(); + instance.call(&field, ¶ms[..]) + }); + if maybe_call_result.is_none() { test_report.add_failure( SpecFailure { file: filename.to_string(), @@ -493,9 +484,7 @@ mod tests { excludes, ); } else { - let params: Vec = - args.iter().cloned().map(|x| convert_value(x)).collect(); - let call_result = instance.unwrap().call(&field, ¶ms[..]); + let call_result = maybe_call_result.unwrap(); match call_result { Err(e) => { test_report.add_failure( @@ -544,20 +533,13 @@ mod tests { field, args, } => { - let instance: Option<&Instance> = match module { - Some(ref name) => { - let i = named_modules.get(name); - match i { - Some(ins) => Some(ins.borrow()), - None => None, - } - } - None => match instance { - Some(ref i) => Some(i.borrow()), - None => None, - }, - }; - if instance.is_none() { + let maybe_call_result = + with_instance(instance.clone(), &named_modules, &module, |instance| { + let params: Vec = + args.iter().cloned().map(convert_value).collect(); + instance.call(&field, ¶ms[..]) + }); + if maybe_call_result.is_none() { test_report.add_failure( SpecFailure { file: filename.to_string(), @@ -569,9 +551,7 @@ mod tests { excludes, ); } else { - let params: Vec = - args.iter().cloned().map(|x| convert_value(x)).collect(); - let call_result = instance.unwrap().call(&field, ¶ms[..]); + let call_result = maybe_call_result.unwrap(); use wasmer_runtime_core::error::{CallError, RuntimeError}; match call_result { Err(e) => { @@ -749,20 +729,17 @@ mod tests { field, args, } => { - let instance: Option<&Instance> = match module { - Some(ref name) => { - let i = named_modules.get(name); - match i { - Some(ins) => Some(ins.borrow()), - None => None, - } - } - None => match instance { - Some(ref i) => Some(i.borrow()), - None => None, + let maybe_call_result = with_instance( + instance.clone(), + &named_modules, + &module, + |instance| { + let params: Vec = + args.iter().cloned().map(convert_value).collect(); + instance.call(&field, ¶ms[..]) }, - }; - if instance.is_none() { + ); + if maybe_call_result.is_none() { test_report.add_failure( SpecFailure { file: filename.to_string(), @@ -774,9 +751,7 @@ mod tests { excludes, ); } else { - let params: Vec = - args.iter().cloned().map(|x| convert_value(x)).collect(); - let call_result = instance.unwrap().call(&field, ¶ms[..]); + let call_result = maybe_call_result.unwrap(); match call_result { Err(_e) => { // TODO is specific error required? @@ -871,16 +846,16 @@ mod tests { } } CommandKind::Register { name, as_name } => { - let instance: Option> = match name { + let instance: Option>> = match name { Some(ref name) => { let i = named_modules.get(name); match i { - Some(ins) => Some(Rc::clone(ins)), + Some(ins) => Some(Arc::clone(ins)), None => None, } } None => match instance { - Some(ref i) => Some(Rc::clone(i)), + Some(ref i) => Some(Arc::clone(i)), None => None, }, }; @@ -906,21 +881,13 @@ mod tests { field, args, } => { - let instance: Option<&Instance> = match module { - Some(ref name) => { - let i = named_modules.get(name); - match i { - Some(ins) => Some(ins.borrow()), - None => None, - } - } - None => match instance { - Some(ref i) => Some(i.borrow()), - None => None, - }, - }; - - if instance.is_none() { + let maybe_call_result = + with_instance(instance.clone(), &named_modules, &module, |instance| { + let params: Vec = + args.iter().cloned().map(convert_value).collect(); + instance.call(&field, ¶ms[..]) + }); + if maybe_call_result.is_none() { test_report.add_failure( SpecFailure { file: filename.to_string(), @@ -932,9 +899,7 @@ mod tests { excludes, ); } else { - let params: Vec = - args.iter().cloned().map(|x| convert_value(x)).collect(); - let call_result = instance.unwrap().call(&field, ¶ms[..]); + let call_result = maybe_call_result.unwrap(); match call_result { Err(e) => { test_report.add_failure( @@ -1054,7 +1019,7 @@ mod tests { } fn get_spectest_import_object( - registered_modules: &HashMap>, + registered_modules: &HashMap>>, ) -> ImportObject { let memory = Memory::new(MemoryDescriptor { minimum: Pages(1), @@ -1091,7 +1056,7 @@ mod tests { }; for (name, instance) in registered_modules.iter() { - import_object.register(name.clone(), Rc::clone(instance)); + import_object.register(name.clone(), Arc::clone(instance)); } import_object }