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

Fix cyclic ref direct instance #2341

Closed
wants to merge 6 commits into from
Closed
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
31 changes: 30 additions & 1 deletion lib/api/src/env.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::{ExportError, Instance};
use crate::{ExportError, Instance, WeakExports, Exportable, WasmTypeList};
use crate::exports::ExportableWithGenerics;
use thiserror::Error;

/// An error while initializing the user supplied host env with the `WasmerEnv` trait.
Expand Down Expand Up @@ -218,3 +219,31 @@ impl<T> Default for LazyInit<T> {
unsafe impl<T: Send> Send for LazyInit<T> {}
// I thought we could opt out of sync..., look into this
// unsafe impl<T> !Sync for InitWithInstance<T> {}

/// Test struct to access with weak instance ref
pub struct InstanceExport {
/// Marker
_pd: std::marker::PhantomData<T>,
exports_ref: WeakExports,
}

impl<T: Sized + Exportable + Clone + 'static> InstanceExport<T> {
/// TODO: document
pub fn new(weak_instance: WeakExports) -> Self {
Self {
_pd: std::marker::PhantomData,
exports_ref: weak_instance,
}
}

/// TODO: document
pub fn get_with_generics<Args, Rets, T2>(&self, name: &str) -> Result<T2, ExportError>
where
Args: WasmTypeList,
Rets: WasmTypeList,
T2: ExportableWithGenerics<Args, Rets>,
{
let exports = self.exports_ref.upgrade().unwrap();
exports.get_with_generics::<T2, Args, Rets>(name)
}
}
65 changes: 54 additions & 11 deletions lib/api/src/exports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use indexmap::IndexMap;
use loupe::MemoryUsage;
use std::fmt;
use std::iter::{ExactSizeIterator, FromIterator};
use std::sync::Arc;
use std::sync::{Arc, Weak};
use thiserror::Error;
use wasmer_engine::Export;

Expand Down Expand Up @@ -67,6 +67,19 @@ pub struct Exports {
map: Arc<IndexMap<String, Extern>>,
}

/// TODO: document this
#[derive(Clone, Default)]
pub struct WeakExports(Weak<IndexMap<String, Extern>>);

impl WeakExports {
/// TODO: document this
pub fn upgrade(&self) -> Option<Exports> {
Some(Exports {
map: self.0.upgrade()?,
})
}
}

impl Exports {
/// Creates a new `Exports`.
pub fn new() -> Self {
Expand Down Expand Up @@ -112,7 +125,7 @@ impl Exports {
///
/// If you want to get an export dynamically handling manually
/// type checking manually, please use `get_extern`.
pub fn get<'a, T: Exportable<'a>>(&'a self, name: &str) -> Result<&'a T, ExportError> {
pub fn get<'a, T: Exportable>(&'a self, name: &str) -> Result<&'a T, ExportError> {
match self.map.get(name) {
None => Err(ExportError::Missing(name.to_string())),
Some(extern_) => T::get_self_from_extern(extern_),
Expand Down Expand Up @@ -154,18 +167,31 @@ impl Exports {
}

/// Hack to get this working with nativefunc too
pub fn get_with_generics<'a, T, Args, Rets>(&'a self, name: &str) -> Result<T, ExportError>
pub fn get_with_generics<T, Args, Rets>(&self, name: &str) -> Result<T, ExportError>
where
Args: WasmTypeList,
Rets: WasmTypeList,
T: ExportableWithGenerics<'a, Args, Rets>,
T: ExportableWithGenerics<Args, Rets>,
{
match self.map.get(name) {
None => Err(ExportError::Missing(name.to_string())),
Some(extern_) => T::get_self_from_extern_with_generics(extern_),
}
}

/// Like `get_with_generics` but with a WeakReference to the `InstanceRef` internally.
/// This is useful for passing data into `WasmerEnv`, for example.
pub fn get_with_generics_weak<T, Args, Rets>(&self, name: &str) -> Result<T, ExportError>
where
Args: WasmTypeList,
Rets: WasmTypeList,
T: ExportableWithGenerics<Args, Rets>,
{
let mut out: T = self.get_with_generics(name)?;
out.into_weak_instance_ref();
Ok(out)
}

/// Get an export as an `Extern`.
pub fn get_extern(&self, name: &str) -> Option<&Extern> {
self.map.get(name)
Expand All @@ -185,6 +211,11 @@ impl Exports {
iter: self.map.iter(),
}
}

/// TODO: document this
pub fn downgrade(&self) -> WeakExports {
WeakExports(Arc::downgrade(&self.map))
}
}

impl fmt::Debug for Exports {
Expand Down Expand Up @@ -282,7 +313,7 @@ impl LikeNamespace for Exports {
/// This trait is used to mark types as gettable from an [`Instance`].
///
/// [`Instance`]: crate::Instance
pub trait Exportable<'a>: Sized {
pub trait Exportable: Sized {
/// This function is used when providedd the [`Extern`] as exportable, so it
/// can be used while instantiating the [`Module`].
///
Expand All @@ -293,21 +324,33 @@ pub trait Exportable<'a>: Sized {
/// from an [`Instance`] by name.
///
/// [`Instance`]: crate::Instance
fn get_self_from_extern(_extern: &'a Extern) -> Result<&'a Self, ExportError>;
fn get_self_from_extern<'a>(_extern: &'a Extern) -> Result<&'a Self, ExportError>;

/// TODO: this method doesn't belong here
fn into_weak_instance_ref(&mut self) {
todo!("into_weak_instance_ref")
}
}

/// A trait for accessing exports (like [`Exportable`]) but it takes generic
/// `Args` and `Rets` parameters so that `NativeFunc` can be accessed directly
/// as well.
pub trait ExportableWithGenerics<'a, Args: WasmTypeList, Rets: WasmTypeList>: Sized {
pub trait ExportableWithGenerics<Args: WasmTypeList, Rets: WasmTypeList>: Sized {
/// Get an export with the given generics.
fn get_self_from_extern_with_generics(_extern: &'a Extern) -> Result<Self, ExportError>;
fn get_self_from_extern_with_generics(_extern: &Extern) -> Result<Self, ExportError>;
/// TODO: this method doesn't belong here
fn into_weak_instance_ref(&mut self);
}

/// We implement it for all concrete [`Exportable`] types (that are `Clone`)
/// with empty `Args` and `Rets`.
impl<'a, T: Exportable<'a> + Clone + 'static> ExportableWithGenerics<'a, (), ()> for T {
fn get_self_from_extern_with_generics(_extern: &'a Extern) -> Result<Self, ExportError> {
T::get_self_from_extern(_extern).map(|i| i.clone())
impl<T: Exportable + Clone + 'static> ExportableWithGenerics<(), ()> for T {
fn get_self_from_extern_with_generics(_extern: &Extern) -> Result<Self, ExportError> {
Self::get_self_from_extern(_extern).map(|i| i.clone())
}

/// TODO: this method doesn't belong here
fn into_weak_instance_ref(&mut self) {
<Self as Exportable>::into_weak_instance_ref(self);
}
}
23 changes: 21 additions & 2 deletions lib/api/src/externals/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -712,19 +712,38 @@ impl Function {
fn closures_unsupported_panic() -> ! {
unimplemented!("Closures (functions with captured environments) are currently unsupported with native functions. See: https://github.com/wasmerio/wasmer/issues/1840")
}

/// Check if the function holds a strong `InstanceRef`.
/// None means there's no `InstanceRef`, strong or weak.
// TODO: maybe feature gate this, we only need it for tests...
pub fn is_strong_instance_ref(&self) -> Option<bool> {
self.exported
.vm_function
.instance_ref
.as_ref()
.map(|v| v.is_strong())
}
}

impl<'a> Exportable<'a> for Function {
impl Exportable for Function {
fn to_export(&self) -> Export {
self.exported.clone().into()
}

fn get_self_from_extern(_extern: &'a Extern) -> Result<&'a Self, ExportError> {
fn get_self_from_extern<'a>(_extern: &'a Extern) -> Result<&'a Self, ExportError> {
match _extern {
Extern::Function(func) => Ok(func),
_ => Err(ExportError::IncompatibleType),
}
}

fn into_weak_instance_ref(&mut self) {
self.exported
.vm_function
.instance_ref
.as_mut()
.map(|v| v.into_weak());
}
}

impl fmt::Debug for Function {
Expand Down
15 changes: 13 additions & 2 deletions lib/api/src/externals/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,13 @@ impl Global {
pub fn same(&self, other: &Self) -> bool {
Arc::ptr_eq(&self.vm_global.from, &other.vm_global.from)
}

/// Check if the global holds a strong `InstanceRef`.
/// None means there's no `InstanceRef`, strong or weak.
// TODO: maybe feature gate this, we only need it for tests...
pub fn is_strong_instance_ref(&self) -> Option<bool> {
self.vm_global.instance_ref.as_ref().map(|v| v.is_strong())
}
}

impl fmt::Debug for Global {
Expand All @@ -220,15 +227,19 @@ impl fmt::Debug for Global {
}
}

impl<'a> Exportable<'a> for Global {
impl Exportable for Global {
fn to_export(&self) -> Export {
self.vm_global.clone().into()
}

fn get_self_from_extern(_extern: &'a Extern) -> Result<&'a Self, ExportError> {
fn get_self_from_extern<'a>(_extern: &'a Extern) -> Result<&'a Self, ExportError> {
match _extern {
Extern::Global(global) => Ok(global),
_ => Err(ExportError::IncompatibleType),
}
}

fn into_weak_instance_ref(&mut self) {
self.vm_global.instance_ref.as_mut().map(|v| v.into_weak());
}
}
15 changes: 13 additions & 2 deletions lib/api/src/externals/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,17 +248,28 @@ impl Memory {
pub fn same(&self, other: &Self) -> bool {
Arc::ptr_eq(&self.vm_memory.from, &other.vm_memory.from)
}

/// Check if the memory holds a strong `InstanceRef`.
/// None means there's no `InstanceRef`, strong or weak.
// TODO: maybe feature gate this, we only need it for tests...
pub fn is_strong_instance_ref(&self) -> Option<bool> {
self.vm_memory.instance_ref.as_ref().map(|v| v.is_strong())
}
}

impl<'a> Exportable<'a> for Memory {
impl Exportable for Memory {
fn to_export(&self) -> Export {
self.vm_memory.clone().into()
}

fn get_self_from_extern(_extern: &'a Extern) -> Result<&'a Self, ExportError> {
fn get_self_from_extern<'a>(_extern: &'a Extern) -> Result<&'a Self, ExportError> {
match _extern {
Extern::Memory(memory) => Ok(memory),
_ => Err(ExportError::IncompatibleType),
}
}

fn into_weak_instance_ref(&mut self) {
self.vm_memory.instance_ref.as_mut().map(|v| v.into_weak());
}
}
13 changes: 11 additions & 2 deletions lib/api/src/externals/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ impl Extern {
}
}

impl<'a> Exportable<'a> for Extern {
impl Exportable for Extern {
fn to_export(&self) -> Export {
match self {
Self::Function(f) => f.to_export(),
Expand All @@ -68,10 +68,19 @@ impl<'a> Exportable<'a> for Extern {
}
}

fn get_self_from_extern(_extern: &'a Self) -> Result<&'a Self, ExportError> {
fn get_self_from_extern<'a>(_extern: &'a Self) -> Result<&'a Self, ExportError> {
// Since this is already an extern, we can just return it.
Ok(_extern)
}

fn into_weak_instance_ref(&mut self) {
match self {
Self::Function(f) => f.into_weak_instance_ref(),
Self::Global(g) => g.into_weak_instance_ref(),
Self::Memory(m) => m.into_weak_instance_ref(),
Self::Table(t) => t.into_weak_instance_ref(),
}
}
}

impl StoreObject for Extern {
Expand Down
15 changes: 13 additions & 2 deletions lib/api/src/externals/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,17 +146,28 @@ impl Table {
pub fn same(&self, other: &Self) -> bool {
Arc::ptr_eq(&self.vm_table.from, &other.vm_table.from)
}

/// Check if the table holds a strong `InstanceRef`.
/// None means there's no `InstanceRef`, strong or weak.
// TODO: maybe feature gate this, we only need it for tests...
pub fn is_strong_instance_ref(&self) -> Option<bool> {
self.vm_table.instance_ref.as_ref().map(|v| v.is_strong())
}
}

impl<'a> Exportable<'a> for Table {
impl Exportable for Table {
fn to_export(&self) -> Export {
self.vm_table.clone().into()
}

fn get_self_from_extern(_extern: &'a Extern) -> Result<&'a Self, ExportError> {
fn get_self_from_extern<'a>(_extern: &'a Extern) -> Result<&'a Self, ExportError> {
match _extern {
Extern::Table(table) => Ok(table),
_ => Err(ExportError::IncompatibleType),
}
}

fn into_weak_instance_ref(&mut self) {
self.vm_table.instance_ref.as_mut().map(|v| v.into_weak());
}
}
6 changes: 3 additions & 3 deletions lib/api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -283,8 +283,8 @@ pub mod internals {
pub use crate::externals::{WithEnv, WithoutEnv};
}

pub use crate::env::{HostEnvInitError, LazyInit, WasmerEnv};
pub use crate::exports::{ExportError, Exportable, Exports, ExportsIterator};
pub use crate::env::{HostEnvInitError, LazyInit, WasmerEnv, InstanceExport};
pub use crate::exports::{ExportError, Exportable, Exports, ExportsIterator, WeakExports};
pub use crate::externals::{
Extern, FromToNativeWasmType, Function, Global, HostFunction, Memory, Table, WasmTypeList,
};
Expand Down Expand Up @@ -322,7 +322,7 @@ pub use wasmer_types::{
};

// TODO: should those be moved into wasmer::vm as well?
pub use wasmer_vm::{raise_user_trap, MemoryError};
pub use wasmer_vm::{raise_user_trap, MemoryError, WeakInstanceRef};
pub mod vm {
//! The vm module re-exports wasmer-vm types.

Expand Down
12 changes: 11 additions & 1 deletion lib/api/src/native.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,10 +196,16 @@ macro_rules! impl_native_traits {
}

}
/// Check if the function holds a strong `InstanceRef`.
/// None means there's no `InstanceRef`, strong or weak.
// TODO: maybe feature gate this, we only need it for tests...
pub fn is_strong_instance_ref(&self) -> Option<bool> {
self.exported.vm_function.instance_ref.as_ref().map(|v| v.is_strong())
}
}

#[allow(unused_parens)]
impl<'a, $( $x, )* Rets> crate::exports::ExportableWithGenerics<'a, ($( $x ),*), Rets> for NativeFunc<( $( $x ),* ), Rets>
impl<$( $x, )* Rets> crate::exports::ExportableWithGenerics<($( $x ),*), Rets> for NativeFunc<( $( $x ),* ), Rets>
where
$( $x: FromToNativeWasmType, )*
Rets: WasmTypeList,
Expand All @@ -208,6 +214,10 @@ macro_rules! impl_native_traits {
use crate::exports::Exportable;
crate::Function::get_self_from_extern(_extern)?.native().map_err(|_| crate::exports::ExportError::IncompatibleType)
}

fn into_weak_instance_ref(&mut self) {
self.exported.vm_function.instance_ref.as_mut().map(|v| v.into_weak());
}
}
};
}
Expand Down
Loading