Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Experimenting with Send and Sync bounds #1989

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 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
Loading