Merge branch 'master' into engine

# Conflicts:
#	lib/api/src/lib.rs
#	lib/engine-jit/src/engine.rs
#	lib/engine-jit/src/module.rs

lib/api/src/module.rs

@@ -184,7 +184,17 @@ impl Module {
         &self,
         resolver: &dyn Resolver,
     ) -> Result<InstanceHandle, InstantiationError> {
-        self.store.engine().instantiate(&self.compiled, resolver)
+        unsafe {
+            let instance_handle = self.store.engine().instantiate(&self.compiled, resolver)?;
+
+            // After the instance handle is created, we need to initialize
+            // the data, call the start function and so. However, if any
+            // of this steps traps, we still need to keep the instance alive
+            // as some of the Instance elements may have placed in other
+            // instance tables.
+            self.compiled.finish_instantiation(&instance_handle)?;
+            Ok(instance_handle)
+        }
     }
 
     /// Returns the name of the current module.

lib/engine-jit/src/engine.rs

@@ -121,7 +121,7 @@ impl Engine for JITEngine {
     }
 
     /// Instantiates a WebAssembly module
-    fn instantiate(
+    unsafe fn instantiate(
         &self,
         compiled_module: &Arc<dyn BaseCompiledModule>,
         resolver: &dyn Resolver,

lib/engine-jit/src/module.rs

@@ -207,17 +207,7 @@ impl CompiledModule {
     ) -> Result<InstanceHandle, InstantiationError> {
         let jit_compiler = jit.compiler();
         let tunables = jit.tunables();
-        let is_bulk_memory: bool = self.serializable.features.bulk_memory;
         let sig_registry: &SignatureRegistry = jit_compiler.signatures();
-        let data_initializers = self
-            .serializable
-            .data_initializers
-            .iter()
-            .map(|init| DataInitializer {
-                location: init.location.clone(),
-                data: &*init.data,
-            })
-            .collect::<Vec<_>>();
         let imports = resolve_imports(
             &self.serializable.module,
             &sig_registry,
@@ -250,14 +240,23 @@ impl CompiledModule {
             finished_tables,
             finished_globals,
             imports,
-            &data_initializers,
             self.signatures.clone(),
-            is_bulk_memory,
             host_state,
         )
         .map_err(|trap| InstantiationError::Start(RuntimeError::from_trap(trap)))
     }
 
+    /// Returns data initializers to pass to `InstanceHandle::initialize`
+    pub fn data_initializers(&self) -> Vec<DataInitializer<'_>> {
+        self.serializable
+            .data_initializers
+            .iter()
+            .map(|init| DataInitializer {
+                location: init.location.clone(),
+                data: &*init.data,
+            })
+            .collect::<Vec<_>>()
+    }
     /// Register this module's stack frame information into the global scope.
     ///
     /// This is required to ensure that any traps can be properly symbolicated.
@@ -280,6 +279,16 @@ unsafe impl Sync for CompiledModule {}
 unsafe impl Send for CompiledModule {}
 
 impl BaseCompiledModule for CompiledModule {
+    unsafe fn finish_instantiation(
+        &self,
+        handle: &InstanceHandle,
+    ) -> Result<(), InstantiationError> {
+        let is_bulk_memory: bool = self.serializable.features.bulk_memory;
+        handle
+            .finish_instantiation(is_bulk_memory, &self.data_initializers())
+            .map_err(|trap| InstantiationError::Start(RuntimeError::from_trap(trap)))
+    }
+
     fn module(&self) -> &Arc<Module> {
         &self.serializable.module
     }

lib/engine/src/engine.rs

@@ -32,7 +32,7 @@ pub trait Engine {
     fn compile(&self, binary: &[u8]) -> Result<Arc<CompiledModule>, CompileError>;
 
     /// Instantiates a WebAssembly module
-    fn instantiate(
+    unsafe fn instantiate(
         &self,
         compiled_module: &Arc<CompiledModule>,
         resolver: &dyn Resolver,

lib/engine/src/module.rs

@@ -1,4 +1,6 @@
+use crate::error::InstantiationError;
 use std::sync::Arc;
+use wasmer_runtime::InstanceHandle;
 use wasmer_runtime::Module;
 
 use downcast_rs::{impl_downcast, DowncastSync};
@@ -6,6 +8,16 @@ use downcast_rs::{impl_downcast, DowncastSync};
 /// The `CompiledModule` trait is used by engine implementors, such
 /// as a JIT or Native execution.
 pub trait CompiledModule: DowncastSync {
+    /// Finish instantiation of a `InstanceHandle`
+    ///
+    /// # Unsafety
+    ///
+    /// See `InstanceHandle::finish_instantiation`
+    unsafe fn finish_instantiation(
+        &self,
+        handle: &InstanceHandle,
+    ) -> Result<(), InstantiationError>;
+
     /// Return a reference-counting pointer to a module.
     fn module(&self) -> &Arc<Module>;
 

lib/engine/src/resolver.rs

@@ -3,7 +3,6 @@
 
 use crate::error::{ImportError, LinkError};
 use more_asserts::assert_ge;
-use std::collections::HashSet;
 use wasm_common::entity::PrimaryMap;
 use wasm_common::{ExternType, ImportIndex, MemoryIndex, TableIndex};
 use wasmer_runtime::{
@@ -95,8 +94,6 @@ pub fn resolve_imports(
     memory_plans: &PrimaryMap<MemoryIndex, MemoryPlan>,
     _table_plans: &PrimaryMap<TableIndex, TablePlan>,
 ) -> Result<Imports, LinkError> {
-    let dependencies = HashSet::new();
-
     let mut function_imports = PrimaryMap::with_capacity(module.num_imported_funcs);
     let mut table_imports = PrimaryMap::with_capacity(module.num_imported_tables);
     let mut memory_imports = PrimaryMap::with_capacity(module.num_imported_memories);
@@ -125,16 +122,12 @@ pub fn resolve_imports(
         }
         match resolved {
             Export::Function(ref f) => {
-                // TODO: Syrus - fix this
-                // dependencies.insert(unsafe { InstanceHandle::from_vmctx(f.vmctx) });
                 function_imports.push(VMFunctionImport {
                     body: f.address,
                     vmctx: f.vmctx,
                 });
             }
             Export::Table(ref t) => {
-                // TODO: Syrus - fix this
-                // dependencies.insert(unsafe { InstanceHandle::from_vmctx(t.vmctx) });
                 table_imports.push(VMTableImport {
                     definition: t.definition,
                     from: t.from,
@@ -168,16 +161,13 @@ pub fn resolve_imports(
                     }
                 }
 
-                // TODO: Syrus - fix this
-                // dependencies.insert(unsafe { InstanceHandle::from_vmctx(m.vmctx) });
                 memory_imports.push(VMMemoryImport {
                     definition: m.definition,
                     from: m.from,
                 });
             }
+
             Export::Global(ref g) => {
-                // TODO: Syrus - fix this
-                // dependencies.insert(unsafe { InstanceHandle::from_vmctx(g.vmctx) });
                 global_imports.push(VMGlobalImport {
                     definition: g.definition,
                 });
@@ -186,7 +176,6 @@ pub fn resolve_imports(
     }
 
     Ok(Imports::new(
-        dependencies,
         function_imports,
         table_imports,
         memory_imports,

lib/runtime/src/imports.rs

@@ -1,4 +1,3 @@
-use crate::instance::InstanceHandle;
 use crate::vmcontext::{VMFunctionImport, VMGlobalImport, VMMemoryImport, VMTableImport};
 use std::collections::HashSet;
 use wasm_common::entity::{BoxedSlice, PrimaryMap};
@@ -7,9 +6,6 @@ use wasm_common::{FuncIndex, GlobalIndex, MemoryIndex, TableIndex};
 /// Resolved import pointers.
 #[derive(Clone)]
 pub struct Imports {
-    /// The set of instances that the imports depend on.
-    pub dependencies: HashSet<InstanceHandle>,
-
     /// Resolved addresses for imported functions.
     pub functions: BoxedSlice<FuncIndex, VMFunctionImport>,
 
@@ -26,14 +22,12 @@ pub struct Imports {
 impl Imports {
     /// Construct a new `Imports` instance.
     pub fn new(
-        dependencies: HashSet<InstanceHandle>,
         function_imports: PrimaryMap<FuncIndex, VMFunctionImport>,
         table_imports: PrimaryMap<TableIndex, VMTableImport>,
         memory_imports: PrimaryMap<MemoryIndex, VMMemoryImport>,
         global_imports: PrimaryMap<GlobalIndex, VMGlobalImport>,
     ) -> Self {
         Self {
-            dependencies,
             functions: function_imports.into_boxed_slice(),
             tables: table_imports.into_boxed_slice(),
             memories: memory_imports.into_boxed_slice(),
@@ -44,7 +38,6 @@ impl Imports {
     /// Construct a new `Imports` instance with no imports.
     pub fn none() -> Self {
         Self {
-            dependencies: HashSet::new(),
             functions: PrimaryMap::new().into_boxed_slice(),
             tables: PrimaryMap::new().into_boxed_slice(),
             memories: PrimaryMap::new().into_boxed_slice(),

lib/runtime/src/instance.rs

@@ -35,7 +35,7 @@ cfg_if::cfg_if! {
 
         impl InstanceHandle {
             /// Set a custom signal handler
-            pub fn set_signal_handler<H>(&mut self, handler: H)
+            pub fn set_signal_handler<H>(&self, handler: H)
             where
                 H: 'static + Fn(libc::c_int, *const libc::siginfo_t, *const libc::c_void) -> bool,
             {
@@ -47,7 +47,7 @@ cfg_if::cfg_if! {
 
         impl InstanceHandle {
             /// Set a custom signal handler
-            pub fn set_signal_handler<H>(&mut self, handler: H)
+            pub fn set_signal_handler<H>(&self, handler: H)
             where
                 H: 'static + Fn(winapi::um::winnt::PEXCEPTION_POINTERS) -> bool,
             {
@@ -62,14 +62,6 @@ cfg_if::cfg_if! {
 /// This is repr(C) to ensure that the vmctx field is last.
 #[repr(C)]
 pub(crate) struct Instance {
-    /// The number of references to this `Instance`.
-    refcount: Cell<usize>,
-
-    /// `Instance`s from which this `Instance` imports. These won't
-    /// create reference cycles because wasm instances can't cyclically
-    /// import from each other.
-    dependencies: HashSet<InstanceHandle>,
-
     /// The `Module` this `Instance` was instantiated from.
     module: Arc<Module>,
 
@@ -788,9 +780,7 @@ impl InstanceHandle {
         finished_tables: BoxedSlice<LocalTableIndex, Table>,
         finished_globals: BoxedSlice<LocalGlobalIndex, VMGlobalDefinition>,
         imports: Imports,
-        data_initializers: &[DataInitializer<'_>],
         vmshared_signatures: BoxedSlice<SignatureIndex, VMSharedSignatureIndex>,
-        is_bulk_memory: bool,
         host_state: Box<dyn Any>,
     ) -> Result<Self, Trap> {
         let vmctx_tables = finished_tables
@@ -813,8 +803,6 @@ impl InstanceHandle {
 
         let handle = {
             let instance = Instance {
-                refcount: Cell::new(1),
-                dependencies: imports.dependencies,
                 module,
                 offsets,
                 memories: finished_memories,
@@ -883,29 +871,42 @@ impl InstanceHandle {
             VMBuiltinFunctionsArray::initialized(),
         );
 
+        // Ensure that our signal handlers are ready for action.
+        init_traps();
+
+        // Perform infallible initialization in this constructor, while fallible
+        // initialization is deferred to the `initialize` method.
+        initialize_passive_elements(instance);
+        initialize_globals(instance);
+
+        Ok(handle)
+    }
+
+    /// Finishes the instantiation process started by `Instance::new`.
+    ///
+    /// Only safe to call immediately after instantiation.
+    pub unsafe fn finish_instantiation(
+        &self,
+        is_bulk_memory: bool,
+        data_initializers: &[DataInitializer<'_>],
+    ) -> Result<(), Trap> {
         // Check initializer bounds before initializing anything. Only do this
         // when bulk memory is disabled, since the bulk memory proposal changes
         // instantiation such that the intermediate results of failed
         // initializations are visible.
         if !is_bulk_memory {
-            check_table_init_bounds(instance)?;
+            check_table_init_bounds(self.instance())?;
-            check_memory_init_bounds(instance, data_initializers)?;
+            check_memory_init_bounds(self.instance(), data_initializers)?;
         }
 
         // Apply the initializers.
-        initialize_tables(instance)?;
+        initialize_tables(self.instance())?;
-        initialize_passive_elements(instance);
+        initialize_memories(self.instance(), data_initializers)?;
-        initialize_memories(instance, data_initializers)?;
-        initialize_globals(instance);
-
-        // Ensure that our signal handlers are ready for action.
-        init_traps();
 
         // The WebAssembly spec specifies that the start function is
         // invoked automatically at instantiation time.
-        instance.invoke_start_function()?;
+        self.instance().invoke_start_function()?;
-
+        Ok(())
-        Ok(handle)
     }
 
     /// Create a new `InstanceHandle` pointing at the instance
@@ -916,7 +917,6 @@ impl InstanceHandle {
     /// be a `VMContext` allocated as part of an `Instance`.
     pub unsafe fn from_vmctx(vmctx: *mut VMContext) -> Self {
         let instance = (&mut *vmctx).instance();
-        instance.refcount.set(instance.refcount.get() + 1);
         Self {
             instance: instance as *const Instance as *mut Instance,
         }
@@ -1031,32 +1031,36 @@ impl InstanceHandle {
     pub(crate) fn instance(&self) -> &Instance {
         unsafe { &*(self.instance as *const Instance) }
     }
+
+    /// Deallocates memory associated with this instance.
+    ///
+    /// Note that this is unsafe because there might be other handles to this
+    /// `InstanceHandle` elsewhere, and there's nothing preventing usage of
+    /// this handle after this function is called.
+    pub unsafe fn dealloc(&self) {
+        let instance = self.instance();
+        let layout = instance.alloc_layout();
+        ptr::drop_in_place(self.instance);
+        alloc::dealloc(self.instance.cast(), layout);
+    }
 }
 
 impl Clone for InstanceHandle {
     fn clone(&self) -> Self {
-        let instance = self.instance();
-        instance.refcount.set(instance.refcount.get() + 1);
         Self {
             instance: self.instance,
         }
     }
 }
 
-impl Drop for InstanceHandle {
+// TODO: uncomment this, as we need to store the handles
-    fn drop(&mut self) {
+// in the store, and once the store is dropped, then the instances
-        let instance = self.instance();
+// will too.
-        let count = instance.refcount.get();
+// impl Drop for InstanceHandle {
-        instance.refcount.set(count - 1);
+//     fn drop(&mut self) {
-        if count == 1 {
+//         unsafe { self.dealloc() }
-            let layout = instance.alloc_layout();
+//     }
-            unsafe {
+// }
-                ptr::drop_in_place(self.instance);
-                alloc::dealloc(self.instance.cast(), layout);
-            }
-        }
-    }
-}
 
 fn check_table_init_bounds(instance: &Instance) -> Result<(), Trap> {
     let module = Arc::clone(&instance.module);