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

feat(vm) Ability to use exports after the instance has been freed #1837

Merged
merged 30 commits into from
Dec 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
ca89bd5
feat(api) Rename `Extern::from_export` to `…::from_vm_export`.
Hywan Nov 19, 2020
f338b68
feat(vm) Change visibility of some `Instance`'s methods to `pub(self)`.
Hywan Nov 19, 2020
955068c
feat(vm) Move `Instance::(lookup|lookup_by_declaration|exports)` meth…
Hywan Nov 19, 2020
b93af3b
start big patch
Hywan Nov 23, 2020
e77afea
big patch: Start injecting `InstanceAllocator` everywhere.
Hywan Nov 23, 2020
d7bcc21
big patch: add some traces.
Hywan Nov 23, 2020
d5c7b62
Merge branch 'master' into fix-vm-leak
Hywan Nov 23, 2020
b7cdb8f
doc(vm) Code polishing and write more documentation.
Hywan Nov 24, 2020
0a63e56
fix(vm) Avoid casting to `NonNull<u8>`.
Hywan Nov 24, 2020
f4d9deb
try something different
Hywan Nov 24, 2020
7f06d4f
polish and update documentation
Hywan Nov 24, 2020
f968b24
feat(vm) Remove `InstanceHandle::from_vmctx`.
Hywan Nov 26, 2020
4d073ea
feat(vm) Rename `InstanceAllocator::instance_ref` to `::as_ref`.
Hywan Nov 26, 2020
0e1ed94
fix(vm) Fix the pointer size when doing arithmetics.
Hywan Nov 26, 2020
26d20e0
feat(vm) Move `MAX_REFCOUNT` as an associated constant of `InstanceAl…
Hywan Nov 26, 2020
df0e30b
fix(vm) Fix a typo in a panic message.
Hywan Nov 26, 2020
cae4bc6
fix(api) Fix `new_native_with_unsafe_mutable_env`.
Hywan Nov 26, 2020
081d3e3
fix(vm) Remove debugging statements.
Hywan Nov 26, 2020
e94f994
doc(vm) Add more documentation.
Hywan Nov 26, 2020
70455a9
doc(vm) Give more details in the API documentation.
Hywan Nov 26, 2020
c649bec
doc(vm) Remove a useless part of the documentation.
Hywan Nov 26, 2020
8e17a4c
fixup
Hywan Nov 26, 2020
0ec3b70
feat(vm) Change how the layout of `Instance` is calculated.
Hywan Nov 26, 2020
5a00f26
feat(vm) More functions can be constant functions.
Hywan Nov 26, 2020
dc2f5cf
doc(changelog) Add #1837.
Hywan Nov 26, 2020
286426a
doc(vm) Update documentation of exports for the `instance_allocator` …
Hywan Nov 26, 2020
85ae8c7
Merge branch 'master' into fix-vm-leak
Hywan Nov 26, 2020
7621991
test(api) Test that exports work after the instance has been freed.
Hywan Nov 27, 2020
2f9da8b
feat(vm) Mark `InstanceAllocator::new` as unsafe.
Hywan Dec 1, 2020
7973a96
Merge branch 'master' into fix-vm-leak
Hywan Dec 1, 2020
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

### Added

- [#1837](https://github.com/wasmerio/wasmer/pull/1837) It is now possible to use exports of an `Intance` even after the `Instance` has been freed
- [#1831](https://github.com/wasmerio/wasmer/pull/1831) Added support for Apple Silicon chips (`arm64-apple-darwin`)
- [#1649](https://github.com/wasmerio/wasmer/pull/1649) Add outline of migration to 1.0.0 docs.

Expand Down
7 changes: 6 additions & 1 deletion lib/api/src/externals/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ impl Function {
vmctx,
signature: ty.clone(),
call_trampoline: None,
instance_allocator: None,
},
},
}
Expand Down Expand Up @@ -172,6 +173,7 @@ impl Function {
vmctx,
signature: ty.clone(),
call_trampoline: None,
instance_allocator: None,
},
},
}
Expand Down Expand Up @@ -225,6 +227,7 @@ impl Function {
signature,
kind: VMFunctionKind::Static,
call_trampoline: None,
instance_allocator: None,
},
},
}
Expand Down Expand Up @@ -294,6 +297,7 @@ impl Function {
vmctx,
signature,
call_trampoline: None,
instance_allocator: None,
},
},
}
Expand Down Expand Up @@ -346,6 +350,7 @@ impl Function {
vmctx,
signature,
call_trampoline: None,
instance_allocator: None,
},
},
}
Expand Down Expand Up @@ -528,7 +533,7 @@ impl Function {
Ok(results.into_boxed_slice())
}

pub(crate) fn from_export(store: &Store, wasmer_export: ExportFunction) -> Self {
pub(crate) fn from_vm_export(store: &Store, wasmer_export: ExportFunction) -> Self {
if let Some(trampoline) = wasmer_export.vm_function.call_trampoline {
Self {
store: store.clone(),
Expand Down
3 changes: 2 additions & 1 deletion lib/api/src/externals/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ impl Global {
Ok(())
}

pub(crate) fn from_export(store: &Store, wasmer_export: ExportGlobal) -> Self {
pub(crate) fn from_vm_export(store: &Store, wasmer_export: ExportGlobal) -> Self {
Self {
store: store.clone(),
global: wasmer_export.vm_global.from,
Expand Down Expand Up @@ -220,6 +220,7 @@ impl<'a> Exportable<'a> for Global {
ExportGlobal {
vm_global: VMExportGlobal {
from: self.global.clone(),
instance_allocator: None,
},
}
.into()
Expand Down
3 changes: 2 additions & 1 deletion lib/api/src/externals/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ impl Memory {
unsafe { MemoryView::new(base as _, length as u32) }
}

pub(crate) fn from_export(store: &Store, wasmer_export: ExportMemory) -> Self {
pub(crate) fn from_vm_export(store: &Store, wasmer_export: ExportMemory) -> Self {
Self {
store: store.clone(),
memory: wasmer_export.vm_memory.from,
Expand Down Expand Up @@ -250,6 +250,7 @@ impl<'a> Exportable<'a> for Memory {
ExportMemory {
vm_memory: VMExportMemory {
from: self.memory.clone(),
instance_allocator: None,
},
}
.into()
Expand Down
12 changes: 6 additions & 6 deletions lib/api/src/externals/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,13 @@ impl Extern {
}
}

/// Create an `Extern` from an `Export`.
pub fn from_export(store: &Store, export: Export) -> Self {
/// Create an `Extern` from an `wasmer_vm::Export`.
pub fn from_vm_export(store: &Store, export: Export) -> Self {
match export {
Export::Function(f) => Self::Function(Function::from_export(store, f)),
Export::Memory(m) => Self::Memory(Memory::from_export(store, m)),
Export::Global(g) => Self::Global(Global::from_export(store, g)),
Export::Table(t) => Self::Table(Table::from_export(store, t)),
Export::Function(f) => Self::Function(Function::from_vm_export(store, f)),
Export::Memory(m) => Self::Memory(Memory::from_vm_export(store, m)),
Export::Global(g) => Self::Global(Global::from_vm_export(store, g)),
Export::Table(t) => Self::Table(Table::from_vm_export(store, t)),
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion lib/api/src/externals/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ impl Table {
Ok(())
}

pub(crate) fn from_export(store: &Store, wasmer_export: ExportTable) -> Self {
pub(crate) fn from_vm_export(store: &Store, wasmer_export: ExportTable) -> Self {
Self {
store: store.clone(),
table: wasmer_export.vm_table.from,
Expand All @@ -158,6 +158,7 @@ impl<'a> Exportable<'a> for Table {
ExportTable {
vm_table: VMExportTable {
from: self.table.clone(),
instance_allocator: None,
},
}
.into()
Expand Down
2 changes: 1 addition & 1 deletion lib/api/src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ impl Instance {
.map(|export| {
let name = export.name().to_string();
let export = handle.lookup(&name).expect("export");
let extern_ = Extern::from_export(store, export.into());
let extern_ = Extern::from_vm_export(store, export.into());
(name, extern_)
})
.collect::<Exports>();
Expand Down
3 changes: 3 additions & 0 deletions lib/api/src/native.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ where
signature,
kind: other.arg_kind,
call_trampoline: None,
instance_allocator: None,
}
}
}*/
Expand All @@ -93,6 +94,7 @@ where
signature,
kind: other.arg_kind,
call_trampoline: None,
instance_allocator: None,
},
}
}
Expand All @@ -117,6 +119,7 @@ where
signature,
kind: other.arg_kind,
call_trampoline: None,
instance_allocator: None,
},
},
}
Expand Down
3 changes: 2 additions & 1 deletion lib/api/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,10 @@ impl ValFuncRef for Val {
kind: wasmer_vm::VMFunctionKind::Static,
vmctx: item.vmctx,
call_trampoline: None,
instance_allocator: None,
},
};
let f = Function::from_export(store, export);
let f = Function::from_vm_export(store, export);
Self::FuncRef(f)
}
}
39 changes: 39 additions & 0 deletions lib/api/tests/instance.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
use anyhow::Result;
use wasmer::*;

#[test]
fn exports_work_after_multiple_instances_have_been_freed() -> Result<()> {
let store = Store::default();
let module = Module::new(
&store,
"
(module
(type $sum_t (func (param i32 i32) (result i32)))
(func $sum_f (type $sum_t) (param $x i32) (param $y i32) (result i32)
local.get $x
local.get $y
i32.add)
(export \"sum\" (func $sum_f)))
",
)?;

let import_object = ImportObject::new();
let instance = Instance::new(&module, &import_object)?;
let instance2 = instance.clone();
let instance3 = instance.clone();

// The function is cloned to “break” the connection with `instance`.
let sum = instance.exports.get_function("sum")?.clone();

drop(instance);
drop(instance2);
drop(instance3);

// All instances have been dropped, but `sum` continues to work!
assert_eq!(
sum.call(&[Value::I32(1), Value::I32(2)])?.into_vec(),
vec![Value::I32(3)],
);

Ok(())
}
2 changes: 1 addition & 1 deletion lib/c-api/src/wasm_c_api/wasi/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ unsafe fn wasi_get_imports_inner(
import_type.name()
),
}));
let inner = Extern::from_export(store, export);
let inner = Extern::from_vm_export(store, export);

Some(Box::new(wasm_extern_t {
instance: None,
Expand Down
4 changes: 2 additions & 2 deletions lib/deprecated/runtime-core/src/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,12 +163,12 @@ impl Module {

(
(namespace, name),
new::wasmer::Extern::from_export(store, Export::Function(function)),
new::wasmer::Extern::from_vm_export(store, Export::Function(function)),
)
}
export => (
(namespace, name),
new::wasmer::Extern::from_export(store, export),
new::wasmer::Extern::from_vm_export(store, export),
),
})
.for_each(|((namespace, name), extern_)| {
Expand Down
34 changes: 31 additions & 3 deletions lib/vm/src/export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Attributions: https://github.com/wasmerio/wasmer/blob/master/ATTRIBUTIONS.md

use crate::global::Global;
use crate::instance::InstanceAllocator;
use crate::memory::{Memory, MemoryStyle};
use crate::table::{Table, TableStyle};
use crate::vmcontext::{VMFunctionBody, VMFunctionEnvironment, VMFunctionKind, VMTrampoline};
Expand Down Expand Up @@ -29,15 +30,27 @@ pub enum VMExport {
pub struct VMExportFunction {
/// The address of the native-code function.
pub address: *const VMFunctionBody,

/// Pointer to the containing `VMContext`.
pub vmctx: VMFunctionEnvironment,

/// The function type, used for compatibility checking.
pub signature: FunctionType,
/// The function kind (specifies the calling convention for the function).

/// The function kind (specifies the calling convention for the
/// function).
pub kind: VMFunctionKind,
/// Address of the function call trampoline owned by the same VMContext that owns the VMFunctionBody.
/// May be None when the function is a host-function (FunctionType == Dynamic or vmctx == nullptr).

/// Address of the function call trampoline owned by the same
/// VMContext that owns the VMFunctionBody.
///
/// May be `None` when the function is a host function (`FunctionType`
/// == `Dynamic` or `vmctx` == `nullptr`).
pub call_trampoline: Option<VMTrampoline>,

/// A “reference” to the instance through the
/// `InstanceAllocator`. `None` if it is a host function.
pub instance_allocator: Option<InstanceAllocator>,
}

/// # Safety
Expand All @@ -59,13 +72,18 @@ impl From<VMExportFunction> for VMExport {
pub struct VMExportTable {
/// Pointer to the containing `Table`.
pub from: Arc<dyn Table>,

/// A “reference” to the instance through the
/// `InstanceAllocator`. `None` if it is a host function.
pub instance_allocator: Option<InstanceAllocator>,
}

/// # Safety
/// This is correct because there is no non-threadsafe logic directly in this type;
/// correct use of the raw table from multiple threads via `definition` requires `unsafe`
/// and is the responsibilty of the user of this type.
unsafe impl Send for VMExportTable {}

/// # Safety
/// This is correct because the values directly in `definition` should be considered immutable
/// and the type is both `Send` and `Clone` (thus marking it `Sync` adds no new behavior, it
Expand Down Expand Up @@ -100,13 +118,18 @@ impl From<VMExportTable> for VMExport {
pub struct VMExportMemory {
/// Pointer to the containing `Memory`.
pub from: Arc<dyn Memory>,

/// A “reference” to the instance through the
/// `InstanceAllocator`. `None` if it is a host function.
pub instance_allocator: Option<InstanceAllocator>,
}

/// # Safety
/// This is correct because there is no non-threadsafe logic directly in this type;
/// correct use of the raw memory from multiple threads via `definition` requires `unsafe`
/// and is the responsibilty of the user of this type.
unsafe impl Send for VMExportMemory {}

/// # Safety
/// This is correct because the values directly in `definition` should be considered immutable
/// and the type is both `Send` and `Clone` (thus marking it `Sync` adds no new behavior, it
Expand Down Expand Up @@ -141,13 +164,18 @@ impl From<VMExportMemory> for VMExport {
pub struct VMExportGlobal {
/// The global declaration, used for compatibility checking.
pub from: Arc<Global>,

/// A “reference” to the instance through the
/// `InstanceAllocator`. `None` if it is a host function.
pub instance_allocator: Option<InstanceAllocator>,
}

/// # Safety
/// This is correct because there is no non-threadsafe logic directly in this type;
/// correct use of the raw global from multiple threads via `definition` requires `unsafe`
/// and is the responsibilty of the user of this type.
unsafe impl Send for VMExportGlobal {}

/// # Safety
/// This is correct because the values directly in `definition` should be considered immutable
/// from the perspective of users of this type and the type is both `Send` and `Clone` (thus
Expand Down
Loading