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.

Sign up for GitHub

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

Implement Send for Instance #807

Merged
merged 9 commits into from
Sep 23, 2019
Merged

Implement Send for Instance #807

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
17a0e78
Implement Send for Instance
Sep 17, 2019
9e93438
Implement Send for everything except Memory
Sep 17, 2019
83c3909
Implement it for memory and make Instance Send
Sep 17, 2019
9c205e0
Add comments explaining the unsafe impls and simplify the code, too
Sep 18, 2019
05ad1aa
Add test for Instance, fix tests for ImportObject
Sep 23, 2019
1e637ba
Merge remote-tracking branch 'origin/master' into feature/make-instan…
Sep 23, 2019
705287c
Merge branch 'master' into feature/make-instance-send
MarkMcCaskey Sep 23, 2019
c4818f1
Update spectests to work with new Instance; use Arc<Mutex<>>
Sep 23, 2019
9cab6d7
Merge branch 'feature/make-instance-send' of github.com:wasmerio/wasm…
Sep 23, 2019
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
20 changes: 8 additions & 12 deletions lib/runtime-core/src/backing.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -536,9 +536,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,
Expand Down Expand Up @@ -624,9 +623,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()) {
Expand Down Expand Up @@ -696,9 +694,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()) {
Expand Down Expand Up @@ -767,9 +764,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 {
Expand Down
4 changes: 4 additions & 0 deletions lib/runtime-core/src/export.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ pub enum Context {
Internal,
}

unsafe impl Send for Context {}
MarkMcCaskey marked this conversation as resolved.
Show resolved Hide resolved

#[derive(Debug, Clone)]
pub enum Export {
Function {
Expand All @@ -26,6 +28,8 @@ pub enum Export {
#[derive(Debug, Clone)]
pub struct FuncPointer(*const vm::Func);

unsafe impl Send for FuncPointer {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this in particular, I think it's relevant that we don't have any thread-local variables.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, can you explain what you mean? I agree that thread-local variables can't be Send, is that something we've done or talked about in the past with FuncPointer?


impl FuncPointer {
/// This needs to be unsafe because there is
/// no way to check whether the passed function
Expand Down
8 changes: 4 additions & 4 deletions lib/runtime-core/src/global.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ use crate::{
types::{GlobalDescriptor, Type, Value},
vm,
};
use std::{cell::RefCell, fmt, rc::Rc};
use std::{cell::RefCell, fmt, sync::Arc};

pub struct Global {
desc: GlobalDescriptor,
storage: Rc<RefCell<vm::LocalGlobal>>,
storage: Arc<RefCell<vm::LocalGlobal>>,
}

impl Global {
Expand Down Expand Up @@ -56,7 +56,7 @@ impl Global {

Self {
desc,
storage: Rc::new(RefCell::new(local_global)),
storage: Arc::new(RefCell::new(local_global)),
}
}

Expand Down Expand Up @@ -120,7 +120,7 @@ impl Clone for Global {
fn clone(&self) -> Self {
Self {
desc: self.desc,
storage: Rc::clone(&self.storage),
storage: Arc::clone(&self.storage),
}
}
}
Expand Down
71 changes: 54 additions & 17 deletions lib/runtime-core/src/import.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -45,28 +45,28 @@ impl IsExport for Export {
/// }
/// ```
pub struct ImportObject {
map: Rc<RefCell<HashMap<String, Box<dyn LikeNamespace>>>>,
pub(crate) state_creator: Option<Rc<dyn Fn() -> (*mut c_void, fn(*mut c_void))>>,
map: Arc<Mutex<HashMap<String, Box<dyn LikeNamespace + Send>>>>,
pub(crate) state_creator: Option<Arc<dyn Fn() -> (*mut c_void, fn(*mut c_void)) + Send>>,
pub allow_missing_functions: bool,
}

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,
}
}

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,
{
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,
}
}
Expand All @@ -92,9 +92,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) => {
Expand All @@ -105,27 +106,60 @@ impl ImportObject {
}
}

pub fn get_namespace(&self, namespace: &str) -> Option<Ref<dyn LikeNamespace + 'static>> {
let map_ref = self.map.borrow();

/*pub fn get_namespace(
MarkMcCaskey marked this conversation as resolved.
Show resolved Hide resolved
&self,
namespace: &str,
) -> Option<
MutexGuardRef<HashMap<String, Box<dyn LikeNamespace + Send>>, &(dyn LikeNamespace + Send)>,
> {
MutexGuardRef::new(self.map.lock().unwrap())
.try_map(|mg| mg.get(namespace).map(|ns| &ns.as_ref()).ok_or(()))
.ok()
}*/

/// Apply a function on the namespace if it exists
/// 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,
}
}

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));
}
Expand Down Expand Up @@ -158,7 +192,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);
Expand All @@ -175,6 +210,8 @@ pub struct Namespace {
map: HashMap<String, Box<dyn IsExport>>,
}

unsafe impl Send for Namespace {}

impl Namespace {
pub fn new() -> Self {
Self {
Expand Down
2 changes: 2 additions & 0 deletions lib/runtime-core/src/typed_func.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,8 @@ 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> {}

impl<'a, Args, Rets> Func<'a, Args, Rets, Wasm>
where
Args: WasmTypeList,
Expand Down