Skip to content

Commit

Permalink
Address feedback, make code more explicit about the different cases
Browse files Browse the repository at this point in the history
  • Loading branch information
Mark McCaskey committed Dec 11, 2020
1 parent be3c381 commit ec4d6e6
Show file tree
Hide file tree
Showing 4 changed files with 192 additions and 66 deletions.
14 changes: 7 additions & 7 deletions lib/api/src/externals/function.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,12 +117,12 @@ impl Function {
store: store.clone(),
definition: FunctionDefinition::Host(HostFunctionDefinition { has_env: false }),
exported: ExportFunction {
metadata: Some(Arc::new(ExportFunctionMetadata {
metadata: Some(Arc::new(ExportFunctionMetadata::new(
host_env,
import_init_function_ptr: None,
None,
host_env_clone_fn,
host_env_drop_fn,
})),
))),
vm_function: VMExportFunction {
address,
kind: VMFunctionKind::Dynamic,
Expand Down Expand Up @@ -210,12 +210,12 @@ impl Function {
store: store.clone(),
definition: FunctionDefinition::Host(HostFunctionDefinition { has_env: true }),
exported: ExportFunction {
metadata: Some(Arc::new(ExportFunctionMetadata {
metadata: Some(Arc::new(ExportFunctionMetadata::new(
host_env,
import_init_function_ptr,
host_env_clone_fn,
host_env_drop_fn,
})),
))),
vm_function: VMExportFunction {
address,
kind: VMFunctionKind::Dynamic,
Expand Down Expand Up @@ -349,12 +349,12 @@ impl Function {
store: store.clone(),
definition: FunctionDefinition::Host(HostFunctionDefinition { has_env: true }),
exported: ExportFunction {
metadata: Some(Arc::new(ExportFunctionMetadata {
metadata: Some(Arc::new(ExportFunctionMetadata::new(
host_env,
import_init_function_ptr,
host_env_clone_fn,
host_env_drop_fn,
})),
))),
vm_function: VMExportFunction {
address,
kind: VMFunctionKind::Static,
Expand Down
35 changes: 29 additions & 6 deletions lib/engine/src/export.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ impl From<VMExport> for Export {
/// The metadata acts as a kind of manual virtual dispatch. We store the
/// user-supplied `WasmerEnv` as a void pointer and have methods on it
/// that have been adapted to accept a void pointer.
///
/// This struct owns the original `host_env`, thus when it gets dropped
/// it calls the `drop` function on it.
#[derive(Debug, PartialEq)]
pub struct ExportFunctionMetadata {
/// This field is stored here to be accessible by `Drop`.
Expand All @@ -65,18 +68,36 @@ pub struct ExportFunctionMetadata {
///
/// See `wasmer_vm::export::VMExportFunction::vmctx` for the version of
/// this pointer that is used by the VM when creating an `Instance`.
pub host_env: *mut std::ffi::c_void,
pub(crate) host_env: *mut std::ffi::c_void,
/// Function pointer to `WasmerEnv::init_with_instance(&mut self, instance: &Instance)`.
///
/// This function is called to finish setting up the environment after
/// we create the `api::Instance`.
// This one is optional for now because dynamic host envs need the rest
// of this without the init fn
pub import_init_function_ptr: Option<ImportInitializerFuncPtr>,
/// TODO:
pub host_env_clone_fn: fn(*mut std::ffi::c_void) -> *mut std::ffi::c_void,
pub(crate) import_init_function_ptr: Option<ImportInitializerFuncPtr>,
/// A function analogous to `Clone::clone` that returns a leaked `Box`.
pub(crate) host_env_clone_fn: fn(*mut std::ffi::c_void) -> *mut std::ffi::c_void,
/// The destructor to free the host environment.
pub host_env_drop_fn: fn(*mut std::ffi::c_void),
pub(crate) host_env_drop_fn: fn(*mut std::ffi::c_void),
}

impl ExportFunctionMetadata {
/// Create an `ExportFunctionMetadata` type with information about
/// the exported function.
pub fn new(
host_env: *mut std::ffi::c_void,
import_init_function_ptr: Option<ImportInitializerFuncPtr>,
host_env_clone_fn: fn(*mut std::ffi::c_void) -> *mut std::ffi::c_void,
host_env_drop_fn: fn(*mut std::ffi::c_void),
) -> Self {
Self {
host_env,
import_init_function_ptr,
host_env_clone_fn,
host_env_drop_fn,
}
}
}

// We have to free `host_env` here because we always clone it before using it
Expand All @@ -95,7 +116,9 @@ impl Drop for ExportFunctionMetadata {
pub struct ExportFunction {
/// The VM function, containing most of the data.
pub vm_function: VMExportFunction,
/// TODO:
/// Contains functions necessary to create and initialize host envs
/// with each `Instance` as well as being responsible for the
/// underlying memory of the host env.
pub metadata: Option<Arc<ExportFunctionMetadata>>,
}

Expand Down
34 changes: 19 additions & 15 deletions lib/engine/src/resolver.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,10 +179,7 @@ pub fn resolve_imports(
}
};

// TODO: add lots of documentation for this really strange
// looking code.
// Also, keep eyes open for refactor opportunities to clean
// this all up.
// Clone the host env for this `Instance`.
let env = if let Some(ExportFunctionMetadata {
host_env_clone_fn: clone,
..
Expand All @@ -196,24 +193,31 @@ pub fn resolve_imports(
(clone)(f.vm_function.vmctx.host_env)
}
} else {
unsafe {
//assert!(f.vm_function.vmctx.host_env.is_null());
f.vm_function.vmctx.host_env
}
// No `clone` function means we're dealing with some
// other kind of `vmctx`, not a host env of any
// kind.
unsafe { f.vm_function.vmctx.host_env }
};

function_imports.push(VMFunctionImport {
body: address,
environment: VMFunctionEnvironment { host_env: env },
});

host_function_env_initializers.push(ImportEnv {
env,
// TODO: consider just passing metadata directly
initializer: f.metadata.as_ref().and_then(|m| m.import_init_function_ptr),
clone: f.metadata.as_ref().map(|m| m.host_env_clone_fn),
destructor: f.metadata.as_ref().map(|m| m.host_env_drop_fn),
});
let initializer = f.metadata.as_ref().and_then(|m| m.import_init_function_ptr);
let clone = f.metadata.as_ref().map(|m| m.host_env_clone_fn);
let destructor = f.metadata.as_ref().map(|m| m.host_env_drop_fn);
let import_env = if let (Some(clone), Some(destructor)) = (clone, destructor) {
if let Some(initializer) = initializer {
ImportEnv::new_host_env(env, clone, initializer, destructor)
} else {
ImportEnv::new_dynamic_host_env_with_no_inner_env(env, clone, destructor)
}
} else {
ImportEnv::new_no_env()
};

host_function_env_initializers.push(import_env);
}
Export::Table(ref t) => {
table_imports.push(VMTableImport {
Expand Down
175 changes: 137 additions & 38 deletions lib/vm/src/instance.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,41 +105,135 @@ pub(crate) struct Instance {
vmctx: VMContext,
}

/// TODO: figure out what to do with Clone here
/// We can probably remove the Arc... just need to figure that out.
/// TODO:
/// A collection of data about host envs.
///
/// Due to invariants in the `HostEnv` variant, this type should only
/// be constructed by one of the given methods, not directly.
#[derive(Debug)]
pub struct ImportEnv {
/// TODO:
pub env: *mut std::ffi::c_void,
/// TODO:
pub clone: Option<fn(*mut std::ffi::c_void) -> *mut std::ffi::c_void>,
/// TODO:
pub initializer: Option<ImportInitializerFuncPtr>,
/// TODO:
pub destructor: Option<fn(*mut std::ffi::c_void)>,
pub enum ImportEnv {
/// The `vmctx` pointer does not refer to a host env, there is no
/// metadata about it.
NoEnv,
/// We're dealing with a user-defined host env.
///
/// This host env may be either unwrapped (the user-supplied host env
/// directly) or wrapped. i.e. in the case of Dynamic functions, we
/// store our own extra data along with the user supplied env,
/// thus the `env` pointer here points to the outermost type.
HostEnv {
/// The function environment. This is not always the user-supplied
/// env.
env: *mut std::ffi::c_void,
/// A clone function for duplicating the env.
clone: fn(*mut std::ffi::c_void) -> *mut std::ffi::c_void,
/// This type must start out as Some. `None` indicates that the
/// initializer has been called and must not be called again.
initializer: Option<ImportInitializerFuncPtr>,
/// The destructor to clean up the type in `env`.
destructor: fn(*mut std::ffi::c_void),
},
/// Like `Self::HostEnv` but contains no user supplied env.
///
/// i.e. Dynamic functions without an env.
DynamicHostEnvWithNoInnerEnv {
/// Some type that does not contain a user supplied env at all,
/// thus we have no need to initialize it.
env: *mut std::ffi::c_void,
/// A clone function for duplicating the env.
clone: fn(*mut std::ffi::c_void) -> *mut std::ffi::c_void,
/// The destructor to clean up the type in `env`.
destructor: fn(*mut std::ffi::c_void),
},
}

impl ImportEnv {
/// Make a new `Self::HostEnv`.
pub fn new_host_env(
env: *mut std::ffi::c_void,
clone: fn(*mut std::ffi::c_void) -> *mut std::ffi::c_void,
initializer: ImportInitializerFuncPtr,
destructor: fn(*mut std::ffi::c_void),
) -> Self {
Self::HostEnv {
env,
clone,
initializer: Some(initializer),
destructor,
}
}

/// Make a new `Self::DynamicHostEnvWithNoInnerEnv`.
pub fn new_dynamic_host_env_with_no_inner_env(
env: *mut std::ffi::c_void,
clone: fn(*mut std::ffi::c_void) -> *mut std::ffi::c_void,
destructor: fn(*mut std::ffi::c_void),
) -> Self {
Self::DynamicHostEnvWithNoInnerEnv {
env,
clone,
destructor,
}
}

/// Make a new `Self::NoEnv`.
pub fn new_no_env() -> Self {
Self::NoEnv
}

fn initializer(&self) -> Option<ImportInitializerFuncPtr> {
match self {
Self::HostEnv { initializer, .. } => *initializer,
_ => None,
}
}
}

impl Clone for ImportEnv {
fn clone(&self) -> Self {
let env = if let Some(clone) = self.clone {
(clone)(self.env)
} else {
self.env
};
Self {
env,
clone: self.clone.clone(),
initializer: self.initializer.clone(),
destructor: self.destructor.clone(),
match &self {
Self::NoEnv => Self::NoEnv,
Self::HostEnv {
env,
clone,
destructor,
initializer,
} => {
let new_env = (*clone)(*env);
Self::HostEnv {
env: new_env,
clone: *clone,
destructor: *destructor,
initializer: *initializer,
}
}
Self::DynamicHostEnvWithNoInnerEnv {
env,
clone,
destructor,
} => {
let new_env = (*clone)(*env);
Self::DynamicHostEnvWithNoInnerEnv {
env: new_env,
clone: *clone,
destructor: *destructor,
}
}
}
}
}

impl Drop for ImportEnv {
fn drop(&mut self) {
if let Some(destructor) = self.destructor {
(destructor)(self.env);
match self {
ImportEnv::DynamicHostEnvWithNoInnerEnv {
env, destructor, ..
}
| ImportEnv::HostEnv {
env, destructor, ..
} => {
(destructor)(*env);
}
ImportEnv::NoEnv => (),
}
}
}
Expand Down Expand Up @@ -190,7 +284,7 @@ impl Instance {
&self,
index: FunctionIndex,
) -> Option<ImportInitializerFuncPtr> {
self.import_envs[index].initializer
self.import_envs[index].initializer()
}

/// Return a pointer to the `VMFunctionImport`s.
Expand Down Expand Up @@ -1446,21 +1540,26 @@ impl InstanceHandle {
) -> Result<(), Err> {
let instance_ref = self.instance.as_mut();

for ImportEnv {
env, initializer, ..
} in instance_ref.import_envs.values_mut()
{
if let Some(ref f) = initializer {
// transmute our function pointer into one with the correct error type
let f = mem::transmute::<
&ImportInitializerFuncPtr,
&fn(*mut ffi::c_void, *const ffi::c_void) -> Result<(), Err>,
>(f);
f(*env, instance_ptr)?;
*initializer = None;
for import_env in instance_ref.import_envs.values_mut() {
match import_env {
ImportEnv::HostEnv {
env,
ref mut initializer,
..
} => {
if let Some(f) = initializer {
// transmute our function pointer into one with the correct error type
let f = mem::transmute::<
&ImportInitializerFuncPtr,
&fn(*mut ffi::c_void, *const ffi::c_void) -> Result<(), Err>,
>(f);
f(*env, instance_ptr)?;
}
*initializer = None;
}
ImportEnv::DynamicHostEnvWithNoInnerEnv { .. } | ImportEnv::NoEnv => (),
}
}

Ok(())
}
}
Expand Down

0 comments on commit ec4d6e6

Please sign in to comment.