Use more solid Send and Sync bounds

Author: chipshort

packages/std/src/testing/mock.rs

@@ -606,6 +606,12 @@ impl Default for WasmQuerier {
             };
             SystemResult::Err(err)
         });
+
+        // check that this handler is Send + Sync
+        // see `cosmwasm_vm::MockQuerier`'s `Send` and `Sync` impls for more details
+        fn assert_send_sync(_: &(impl Send + Sync)) {}
+        assert_send_sync(&handler);
+
         Self::new(handler)
     }
 }

packages/vm/src/backend.rs

@@ -85,7 +85,7 @@ pub struct Backend<A: BackendApi, S: Storage, Q: Querier> {
 }
 
 /// Access to the VM's backend storage, i.e. the chain
-pub trait Storage {
+pub trait Storage: Send + Sync {
     /// Returns Err on error.
     /// Returns Ok(None) when key does not exist.
     /// Returns Ok(Some(Vec<u8>)) when key exists.
@@ -171,7 +171,7 @@ pub trait BackendApi: Clone + Send {
     fn addr_humanize(&self, canonical: &[u8]) -> BackendResult<String>;
 }
 
-pub trait Querier {
+pub trait Querier: Send + Sync {
     /// This is all that must be implemented for the Querier.
     /// This allows us to pass through binary queries from one level to another without
     /// knowing the custom format, or we can decode it, with the knowledge of the allowed

packages/vm/src/environment.rs

@@ -1,10 +1,8 @@
 //! Internal details to be used by instance.rs only
 use std::borrow::BorrowMut;
-use std::cell::RefCell;
 use std::marker::PhantomData;
 use std::ptr::NonNull;
-use std::rc::Rc;
-use std::sync::{Arc, RwLock};
+use std::sync::{Arc, Mutex, RwLock};
 
 use derivative::Derivative;
 use wasmer::{AsStoreMut, Instance as WasmerInstance, Memory, MemoryView, Value};
@@ -106,7 +104,7 @@ pub struct DebugInfo<'a> {
 //                            /- BEGIN TRAIT                          END TRAIT \
 //                            |                                                 |
 //                            v                                                 v
-pub type DebugHandlerFn = dyn for<'a, 'b> FnMut(/* msg */ &'a str, DebugInfo<'b>);
+pub type DebugHandlerFn = dyn for<'a, 'b> FnMut(/* msg */ &'a str, DebugInfo<'b>) + Send + Sync;
 
 /// A environment that provides access to the ContextData.
 /// The environment is clonable but clones access the same underlying data.
@@ -117,10 +115,6 @@ pub struct Environment<A, S, Q> {
     data: Arc<RwLock<ContextData<S, Q>>>,
 }
 
-unsafe impl<A: BackendApi, S: Storage, Q: Querier> Send for Environment<A, S, Q> {}
-
-unsafe impl<A: BackendApi, S: Storage, Q: Querier> Sync for Environment<A, S, Q> {}
-
 impl<A: BackendApi, S: Storage, Q: Querier> Clone for Environment<A, S, Q> {
     fn clone(&self) -> Self {
         Environment {
@@ -142,15 +136,15 @@ impl<A: BackendApi, S: Storage, Q: Querier> Environment<A, S, Q> {
         }
     }
 
-    pub fn set_debug_handler(&self, debug_handler: Option<Rc<RefCell<DebugHandlerFn>>>) {
+    pub fn set_debug_handler(&self, debug_handler: Option<Arc<Mutex<DebugHandlerFn>>>) {
         self.with_context_data_mut(|context_data| {
             context_data.debug_handler = debug_handler;
         })
     }
 
-    pub fn debug_handler(&self) -> Option<Rc<RefCell<DebugHandlerFn>>> {
+    pub fn debug_handler(&self) -> Option<Arc<Mutex<DebugHandlerFn>>> {
         self.with_context_data(|context_data| {
-            // This clone here requires us to wrap the function in Rc instead of Box
+            // This clone here requires us to wrap the function in Arc instead of Box
             context_data.debug_handler.clone()
         })
     }
@@ -282,7 +276,7 @@ impl<A: BackendApi, S: Storage, Q: Querier> Environment<A, S, Q> {
     /// Creates a back reference from a contact to its partent instance
     pub fn set_wasmer_instance(&self, wasmer_instance: Option<NonNull<WasmerInstance>>) {
         self.with_context_data_mut(|context_data| {
-            context_data.wasmer_instance = wasmer_instance;
+            context_data.wasmer_instance = wasmer_instance.map(WasmerRef);
         });
     }
 
@@ -399,9 +393,42 @@ pub struct ContextData<S, Q> {
     storage_readonly: bool,
     call_depth: usize,
     querier: Option<Q>,
-    debug_handler: Option<Rc<RefCell<DebugHandlerFn>>>,
+    debug_handler: Option<Arc<Mutex<DebugHandlerFn>>>,
     /// A non-owning link to the wasmer instance
-    wasmer_instance: Option<NonNull<WasmerInstance>>,
+    wasmer_instance: Option<WasmerRef>,
+}
+
+/// A non-owning link to the wasmer instance
+///
+/// This wrapper type allows us to implement `Send` and `Sync`.
+#[derive(Clone, Copy)]
+struct WasmerRef(NonNull<WasmerInstance>);
+
+impl WasmerRef {
+    pub const unsafe fn as_ref<'a>(&self) -> &'a WasmerInstance {
+        self.0.as_ref()
+    }
+}
+
+//
+unsafe impl Send for WasmerRef {}
+unsafe impl Sync for WasmerRef {}
+
+/// TODO: SAFETY
+// unsafe impl<S: Sync, Q: Sync> Sync for ContextData<S, Q> {}
+
+/// Implementing `Send` is safe here as long as `WasmerInstance` is Send.
+/// This is guaranteed by the function definition below.
+// unsafe impl<S: Send, Q: Send> Send for ContextData<S, Q> {}
+
+#[allow(dead_code)]
+fn assert_is_send<T: Send>(_: PhantomData<T>) {}
+#[allow(dead_code)]
+fn assert_is_sync<T: Sync>(_: PhantomData<T>) {}
+#[allow(dead_code)]
+fn assert_wasmer_instance() {
+    assert_is_send(PhantomData::<WasmerInstance>);
+    assert_is_sync(PhantomData::<WasmerInstance>);
 }
 
 impl<S: Storage, Q: Querier> ContextData<S, Q> {

packages/vm/src/imports.rs

@@ -436,7 +436,7 @@ pub fn do_debug<A: BackendApi + 'static, S: Storage + 'static, Q: Querier + 'sta
         let message_data = read_region(&data.memory(&store), message_ptr, MAX_LENGTH_DEBUG)?;
         let msg = String::from_utf8_lossy(&message_data);
         let gas_remaining = data.get_gas_left(&mut store);
-        debug_handler.borrow_mut()(
+        (debug_handler.lock().unwrap())(
             &msg,
             DebugInfo {
                 gas_remaining,

packages/vm/src/instance.rs

@@ -1,8 +1,6 @@
-use std::cell::RefCell;
 use std::collections::{HashMap, HashSet};
 use std::ptr::NonNull;
-use std::rc::Rc;
-use std::sync::Mutex;
+use std::sync::{Arc, Mutex};
 
 use wasmer::{
     Exports, Function, FunctionEnv, Imports, Instance as WasmerInstance, Module, Store, Value,
@@ -307,11 +305,11 @@ where
 
     pub fn set_debug_handler<H>(&mut self, debug_handler: H)
     where
-        H: for<'a, 'b> FnMut(/* msg */ &'a str, DebugInfo<'b>) + 'static,
+        H: for<'a, 'b> FnMut(/* msg */ &'a str, DebugInfo<'b>) + 'static + Send + Sync,
     {
         self.fe
             .as_ref(&self.store)
-            .set_debug_handler(Some(Rc::new(RefCell::new(debug_handler))));
+            .set_debug_handler(Some(Arc::new(Mutex::new(debug_handler))));
     }
 
     pub fn unset_debug_handler(&mut self) {

packages/vm/src/testing/querier.rs

@@ -19,6 +19,14 @@ pub struct MockQuerier<C: CustomQuery + DeserializeOwned = Empty> {
     querier: StdMockQuerier<C>,
 }
 
+// SAFETY: The only thing in `MockQuerier` that is not `Send + Sync` are
+// the custom handler and wasm handler.
+// The default custom handler does not use the reference it gets,
+// so it should be safe to assume it is `Send + Sync`.
+// When setting these in `update_wasm` or `with_custom_handler`, we require them to be `Send + Sync`.
+unsafe impl<C: CustomQuery + DeserializeOwned> Send for MockQuerier<C> {}
+unsafe impl<C: CustomQuery + DeserializeOwned> Sync for MockQuerier<C> {}
+
 impl<C: CustomQuery + DeserializeOwned> MockQuerier<C> {
     pub fn new(balances: &[(&str, &[Coin])]) -> Self {
         MockQuerier {
@@ -47,14 +55,16 @@ impl<C: CustomQuery + DeserializeOwned> MockQuerier<C> {
 
     pub fn update_wasm<WH: 'static>(&mut self, handler: WH)
     where
-        WH: Fn(&cosmwasm_std::WasmQuery) -> cosmwasm_std::QuerierResult,
+        WH: Fn(&cosmwasm_std::WasmQuery) -> cosmwasm_std::QuerierResult + Sync + Send,
+        // see above for Sync + Send bound explanation
     {
         self.querier.update_wasm(handler)
     }
 
     pub fn with_custom_handler<CH: 'static>(mut self, handler: CH) -> Self
     where
-        CH: Fn(&C) -> MockQuerierCustomHandlerResult,
+        CH: Fn(&C) -> MockQuerierCustomHandlerResult + Sync + Send,
+        // see above for Sync + Send bound explanation
     {
         self.querier = self.querier.with_custom_handler(handler);
         self