Merge #807

807: Implement Send for Instance r=MarkMcCaskey a=MarkMcCaskey

# Review

- [x] Create a short description of the the change in the CHANGELOG.md file

Resolves #748 

WIP

## List of changes
### Commit 1
- `Global`s use Arc instead of RC
- Export `Context` and `FuncPointer` manually implement Send
- `ImportObject` uses `Arc<Mutex<HashMap<...>>>` Instead of `Rc<RefCell<HashMap<...>>>`; removed `get_namespace` method in favor of continuation style to deal with locking the Mutex
- `Func` manually implements `Send` (TODO: this change needs to be checked in depth)
### Commit 2
- `unsafe impl Send for Export {}` (temporary to allow Memory to be not Send)
- RefCell -> Mutex in Global and Table
- Rc -> Arc in Table
- Namespace's `IsExport`s must be `Send` (Done to avoid touching much more of the code (i.e. `trait IsExport: Send` requires a lot -- maybe this is what we should do though)
- Make `Host` and `Wasm` `Func`s Send (manual implementation)
- Manual implementation for `LocalTable` and `AnyFunc`
### Commit 3
- rm placeholder `unsafe impl Send for Export {}`
- Manual implementation for `LocalBacking` and `ImportBacking` (both seemed to be not Send due to direct ownership of mutable pointers in their containers)
- ImportObject's state creator Fn trait object is now ` + Send + Sync + 'static` (required because it's in an Arc)
- Manually implement Send for `InstanceInner` because it holds a raw pointer, `LocalBacking` and `ImportBacking` are marked Send separately 
- Memory: All Rc -> Arc (including unshared memory); All RefCell -> Mutex (including unshared memory)
- Manual implementation of Send and Sync on `UnsharedMemoryInternal`
- Manual implementation of Send and Sync on `SharedMemoryInternal`
- Change `runtime-core::unix::memory::Memory.fd` from `Option<Rc<Rawfd>>` to `Option<Arc<Rawfd>>` (not strictly required for this change because Memory has manual implementations of Send and Sync, but Arc seems more correct here and there's no comment justifying the use of Rc)
- Manual implementation of Send for `ImportedFunc`

Co-authored-by: Mark McCaskey <mark@wasmer.io>
Co-authored-by: Mark McCaskey <markmccaskey@users.noreply.github.com>

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.

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<ImportedGlobalIndex, *mut vm::LocalGlobal>,
 }
 
+// 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
+        let import =
-            .get_namespace(namespace)
+            imports.maybe_with_namespace(namespace, |namespace| namespace.get_export(name));
-            .and_then(|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
+        let memory_import =
-            .get_namespace(&namespace)
+            imports.maybe_with_namespace(namespace, |namespace| namespace.get_export(name));
-            .and_then(|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
+        let table_import =
-            .get_namespace(&namespace)
+            imports.maybe_with_namespace(namespace, |namespace| namespace.get_export(name));
-            .and_then(|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
+        let import =
-            .get_namespace(namespace)
+            imports.maybe_with_namespace(namespace, |namespace| namespace.get_export(name));
-            .and_then(|namespace| namespace.get_export(name));
         match import {
             Some(Export::Global(mut global)) => {
                 if global.descriptor() == *imported_global_desc {

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

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<RefCell<vm::LocalGlobal>>,
+    storage: Arc<Mutex<vm::LocalGlobal>>,
 }
 
 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),
         }
     }
 }

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<RefCell<HashMap<String, Box<dyn LikeNamespace>>>>,
+    map: Arc<Mutex<HashMap<String, Box<dyn LikeNamespace + Send>>>>,
-    pub(crate) state_creator: Option<Rc<dyn Fn() -> (*mut c_void, fn(*mut c_void))>>,
+    pub(crate) state_creator:
+        Option<Arc<dyn Fn() -> (*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<F>(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())),
+            map: Arc::new(Mutex::new(HashMap::new())),
-            state_creator: Some(Rc::new(state_creator)),
+            state_creator: Some(Arc::new(state_creator)),
             allow_missing_functions: false,
         }
     }
@@ -92,9 +93,10 @@ impl ImportObject {
     pub fn register<S, N>(&mut self, name: S, namespace: N) -> Option<Box<dyn LikeNamespace>>
     where
         S: Into<String>,
-        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<Ref<dyn LikeNamespace + 'static>> {
+    /// Apply a function on the namespace if it exists
-        let map_ref = self.map.borrow();
+    /// If your function can fail, consider using `maybe_with_namespace`
-
+    pub fn with_namespace<Func, InnerRet>(&self, namespace: &str, f: Func) -> Option<InnerRet>
+    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<Func, InnerRet>(&self, namespace: &str, f: Func) -> Option<InnerRet>
+    where
+        Func: FnOnce(&(dyn LikeNamespace + Send)) -> Option<InnerRet>,
+        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<T: IntoIterator<Item = (String, String, Export)>>(&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<String, Box<dyn IsExport>>,
+    map: HashMap<String, Box<dyn IsExport + Send>>,
 }
 
 impl Namespace {
@@ -182,10 +207,10 @@ impl Namespace {
         }
     }
 
-    pub fn insert<S, E>(&mut self, name: S, export: E) -> Option<Box<dyn IsExport>>
+    pub fn insert<S, E>(&mut self, name: S, export: E) -> Option<Box<dyn IsExport + Send>>
     where
         S: Into<String>,
-        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();
+        let small_cat_export =
-        assert!(cat_ns.get_export("small").is_some());
+            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();
+        let entries = imports1.maybe_with_namespace("dog", |dog_ns| {
-        assert!(dog_ns.get_export("happy").is_some());
+            Some((dog_ns.get_export("happy")?, dog_ns.get_export("small")?))
-        assert!(dog_ns.get_export("small").is_some());
+        });
+        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!(
+        assert!(if let Export::Global(happy_dog_global) = happy_dog_entry {
-            if let Export::Global(happy_dog_global) = dog_ns.get_export("happy").unwrap() {
+            happy_dog_global.get() == Value::I32(4)
-                happy_dog_global.get() == Value::I32(4)
+        } else {
-            } else {
+            false
-                false
+        });
-            }
-        );
     }
 }

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<Instance> {
     }
 }
 
+impl LikeNamespace for Arc<Mutex<Instance>> {
+    fn get_export(&self, name: &str) -> Option<Export> {
+        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<T: Send>() {}
+
+    #[test]
+    fn test_instance_is_send() {
+        is_send::<Instance>();
+    }
+}

lib/runtime-core/src/memory/mod.rs

@@ -8,12 +8,9 @@ use crate::{
     units::Pages,
     vm,
 };
-use std::{
+use std::{cell::Cell, fmt, mem, sync::Arc};
-    cell::{Cell, RefCell},
+
-    fmt, mem,
+use std::sync::Mutex as StdMutex;
-    rc::Rc,
-    sync::Arc,
-};
 
 pub use self::dynamic::DynamicMemory;
 pub use self::static_::StaticMemory;
@@ -208,14 +205,18 @@ enum UnsharedMemoryStorage {
 }
 
 pub struct UnsharedMemory {
-    internal: Rc<UnsharedMemoryInternal>,
+    internal: Arc<UnsharedMemoryInternal>,
 }
 
 struct UnsharedMemoryInternal {
-    storage: RefCell<UnsharedMemoryStorage>,
+    storage: StdMutex<UnsharedMemoryStorage>,
     local: Cell<vm::LocalMemory>,
 }
 
+// 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<Self, CreationError> {
         let mut local = vm::LocalMemory {
@@ -235,15 +236,15 @@ impl UnsharedMemory {
         };
 
         Ok(Self {
-            internal: Rc::new(UnsharedMemoryInternal {
+            internal: Arc::new(UnsharedMemoryInternal {
-                storage: RefCell::new(storage),
+                storage: StdMutex::new(storage),
                 local: Cell::new(local),
             }),
         })
     }
 
     pub fn grow(&self, delta: Pages) -> Result<Pages, GrowError> {
-        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<Box<StaticMemory>>,
+    memory: StdMutex<Box<StaticMemory>>,
     local: Cell<vm::LocalMemory>,
     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<Self, CreationError> {
         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<Pages, GrowError> {
         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()
     }

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<Rc<RawFd>>,
+    fd: Option<Arc<RawFd>>,
 }
 
 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)),
             })
         }
     }

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<RefCell<(TableStorage, vm::LocalTable)>>,
+    storage: Arc<Mutex<(TableStorage, vm::LocalTable)>>,
 }
 
 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),
         }
     }
 }

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,

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 {

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<F, R>(
+        maybe_instance: Option<Arc<Mutex<Instance>>>,
+        named_modules: &HashMap<String, Arc<Mutex<Instance>>>,
+        module: &Option<String>,
+        f: F,
+    ) -> Option<R>
+    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<Rc<Instance>> = None;
+        let mut instance: Option<Arc<Mutex<Instance>>> = None;
 
-        let mut named_modules: HashMap<String, Rc<Instance>> = HashMap::new();
+        let mut named_modules: HashMap<String, Arc<Mutex<Instance>>> = HashMap::new();
 
-        let mut registered_modules: HashMap<String, Rc<Instance>> = HashMap::new();
+        let mut registered_modules: HashMap<String, Arc<Mutex<Instance>>> = 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 {
+                            let maybe_call_result = with_instance(
-                                Some(ref name) => {
+                                instance.clone(),
-                                    let i = named_modules.get(name);
+                                &named_modules,
-                                    match i {
+                                &module,
-                                        Some(ins) => Some(ins.borrow()),
+                                |instance| {
-                                        None => None,
+                                    let params: Vec<wasmer_runtime_core::types::Value> =
-                                    }
+                                        args.iter().cloned().map(convert_value).collect();
-                                }
+                                    instance.call(&field, &params[..])
-                                None => match instance {
-                                    Some(ref i) => Some(i.borrow()),
-                                    None => None,
                                 },
-                            };
+                            );
-                            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<wasmer_runtime_core::types::Value> =
+                                let call_result = maybe_call_result.unwrap();
-                                    args.iter().cloned().map(|x| convert_value(x)).collect();
-                                let call_result = instance.unwrap().call(&field, &params[..]);
                                 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 {
+                            let maybe_call_result = with_instance(
-                                Some(ref name) => {
+                                instance.clone(),
-                                    let i = named_modules.get(name);
+                                &named_modules,
-                                    match i {
+                                &module,
-                                        Some(ins) => Some(ins.borrow()),
+                                |instance| {
-                                        None => None,
+                                    instance
-                                    }
+                                        .get_export(&field)
-                                }
+                                        .expect(&format!("missing global {:?}", &field))
-                                None => match instance {
-                                    Some(ref i) => Some(i.borrow()),
-                                    None => None,
                                 },
-                            };
+                            );
-                            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
+                                let export: Export = maybe_call_result.unwrap();
-                                    .unwrap()
-                                    .get_export(&field)
-                                    .expect(&format!("missing global {:?}", &field));
                                 match export {
                                     Export::Global(g) => {
                                         let value = g.get();
@@ -392,20 +399,13 @@ mod tests {
                         field,
                         args,
                     } => {
-                        let instance: Option<&Instance> = match module {
+                        let maybe_call_result =
-                            Some(ref name) => {
+                            with_instance(instance.clone(), &named_modules, &module, |instance| {
-                                let i = named_modules.get(name);
+                                let params: Vec<wasmer_runtime_core::types::Value> =
-                                match i {
+                                    args.iter().cloned().map(convert_value).collect();
-                                    Some(ins) => Some(ins.borrow()),
+                                instance.call(&field, &params[..])
-                                    None => None,
+                            });
-                                }
+                        if maybe_call_result.is_none() {
-                            }
-                            None => match instance {
-                                Some(ref i) => Some(i.borrow()),
-                                None => None,
-                            },
-                        };
-                        if instance.is_none() {
                             test_report.add_failure(
                                 SpecFailure {
                                     file: filename.to_string(),
@@ -417,9 +417,7 @@ mod tests {
                                 excludes,
                             );
                         } else {
-                            let params: Vec<wasmer_runtime_core::types::Value> =
+                            let call_result = maybe_call_result.unwrap();
-                                args.iter().cloned().map(|x| convert_value(x)).collect();
-                            let call_result = instance.unwrap().call(&field, &params[..]);
                             match call_result {
                                 Err(e) => {
                                     test_report.add_failure(
@@ -468,20 +466,13 @@ mod tests {
                         field,
                         args,
                     } => {
-                        let instance: Option<&Instance> = match module {
+                        let maybe_call_result =
-                            Some(ref name) => {
+                            with_instance(instance.clone(), &named_modules, &module, |instance| {
-                                let i = named_modules.get(name);
+                                let params: Vec<wasmer_runtime_core::types::Value> =
-                                match i {
+                                    args.iter().cloned().map(convert_value).collect();
-                                    Some(ins) => Some(ins.borrow()),
+                                instance.call(&field, &params[..])
-                                    None => None,
+                            });
-                                }
+                        if maybe_call_result.is_none() {
-                            }
-                            None => match instance {
-                                Some(ref i) => Some(i.borrow()),
-                                None => None,
-                            },
-                        };
-                        if instance.is_none() {
                             test_report.add_failure(
                                 SpecFailure {
                                     file: filename.to_string(),
@@ -493,9 +484,7 @@ mod tests {
                                 excludes,
                             );
                         } else {
-                            let params: Vec<wasmer_runtime_core::types::Value> =
+                            let call_result = maybe_call_result.unwrap();
-                                args.iter().cloned().map(|x| convert_value(x)).collect();
-                            let call_result = instance.unwrap().call(&field, &params[..]);
                             match call_result {
                                 Err(e) => {
                                     test_report.add_failure(
@@ -544,20 +533,13 @@ mod tests {
                         field,
                         args,
                     } => {
-                        let instance: Option<&Instance> = match module {
+                        let maybe_call_result =
-                            Some(ref name) => {
+                            with_instance(instance.clone(), &named_modules, &module, |instance| {
-                                let i = named_modules.get(name);
+                                let params: Vec<wasmer_runtime_core::types::Value> =
-                                match i {
+                                    args.iter().cloned().map(convert_value).collect();
-                                    Some(ins) => Some(ins.borrow()),
+                                instance.call(&field, &params[..])
-                                    None => None,
+                            });
-                                }
+                        if maybe_call_result.is_none() {
-                            }
-                            None => match instance {
-                                Some(ref i) => Some(i.borrow()),
-                                None => None,
-                            },
-                        };
-                        if instance.is_none() {
                             test_report.add_failure(
                                 SpecFailure {
                                     file: filename.to_string(),
@@ -569,9 +551,7 @@ mod tests {
                                 excludes,
                             );
                         } else {
-                            let params: Vec<wasmer_runtime_core::types::Value> =
+                            let call_result = maybe_call_result.unwrap();
-                                args.iter().cloned().map(|x| convert_value(x)).collect();
-                            let call_result = instance.unwrap().call(&field, &params[..]);
                             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 {
+                            let maybe_call_result = with_instance(
-                                Some(ref name) => {
+                                instance.clone(),
-                                    let i = named_modules.get(name);
+                                &named_modules,
-                                    match i {
+                                &module,
-                                        Some(ins) => Some(ins.borrow()),
+                                |instance| {
-                                        None => None,
+                                    let params: Vec<wasmer_runtime_core::types::Value> =
-                                    }
+                                        args.iter().cloned().map(convert_value).collect();
-                                }
+                                    instance.call(&field, &params[..])
-                                None => match instance {
-                                    Some(ref i) => Some(i.borrow()),
-                                    None => None,
                                 },
-                            };
+                            );
-                            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<wasmer_runtime_core::types::Value> =
+                                let call_result = maybe_call_result.unwrap();
-                                    args.iter().cloned().map(|x| convert_value(x)).collect();
-                                let call_result = instance.unwrap().call(&field, &params[..]);
                                 match call_result {
                                     Err(_e) => {
                                         // TODO is specific error required?
@@ -871,16 +846,16 @@ mod tests {
                     }
                 }
                 CommandKind::Register { name, as_name } => {
-                    let instance: Option<Rc<Instance>> = match name {
+                    let instance: Option<Arc<Mutex<Instance>>> = 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 {
+                        let maybe_call_result =
-                            Some(ref name) => {
+                            with_instance(instance.clone(), &named_modules, &module, |instance| {
-                                let i = named_modules.get(name);
+                                let params: Vec<wasmer_runtime_core::types::Value> =
-                                match i {
+                                    args.iter().cloned().map(convert_value).collect();
-                                    Some(ins) => Some(ins.borrow()),
+                                instance.call(&field, &params[..])
-                                    None => None,
+                            });
-                                }
+                        if maybe_call_result.is_none() {
-                            }
-                            None => match instance {
-                                Some(ref i) => Some(i.borrow()),
-                                None => None,
-                            },
-                        };
-
-                        if instance.is_none() {
                             test_report.add_failure(
                                 SpecFailure {
                                     file: filename.to_string(),
@@ -932,9 +899,7 @@ mod tests {
                                 excludes,
                             );
                         } else {
-                            let params: Vec<wasmer_runtime_core::types::Value> =
+                            let call_result = maybe_call_result.unwrap();
-                                args.iter().cloned().map(|x| convert_value(x)).collect();
-                            let call_result = instance.unwrap().call(&field, &params[..]);
                             match call_result {
                                 Err(e) => {
                                     test_report.add_failure(
@@ -1054,7 +1019,7 @@ mod tests {
     }
 
     fn get_spectest_import_object(
-        registered_modules: &HashMap<String, Rc<Instance>>,
+        registered_modules: &HashMap<String, Arc<Mutex<Instance>>>,
     ) -> 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
     }