Skip to content

Commit

Permalink
Use more solid Send and Sync bounds
Browse files Browse the repository at this point in the history
  • Loading branch information
chipshort committed Jan 12, 2024
1 parent f6c9c46 commit 86ef98f
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 24 deletions.
6 changes: 6 additions & 0 deletions packages/std/src/testing/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down
4 changes: 2 additions & 2 deletions packages/vm/src/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
55 changes: 41 additions & 14 deletions packages/vm/src/environment.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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.
Expand All @@ -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 {
Expand All @@ -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()
})
}
Expand Down Expand Up @@ -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);
});
}

Expand Down Expand Up @@ -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> {
Expand Down
2 changes: 1 addition & 1 deletion packages/vm/src/imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
8 changes: 3 additions & 5 deletions packages/vm/src/instance.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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) {
Expand Down
14 changes: 12 additions & 2 deletions packages/vm/src/testing/querier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 86ef98f

Please sign in to comment.