avoid swapping an entire Instance of memory during drop, note sharp edges of Instance drop
@@ -9,6 +9,7 @@ use crate::embed_ctx::CtxMap;
use crate::error::Error;
use crate::instance::siginfo_ext::SiginfoExt;
use crate::module::{self, Global, Module};
+use crate::region::RegionInternal;
use crate::sysdeps::UContext;
use crate::val::{UntypedRetVal, Val};
use crate::WASM_PAGE_SIZE;
@@ -141,12 +142,23 @@ impl DerefMut for InstanceHandle {
impl Drop for InstanceHandle {
fn drop(&mut self) {
if self.needs_inst_drop {
- // run the destructor by taking and dropping the inner `Instance`
unsafe {
- // make sure magic is zeroed, but allow everything else to be uninitialized
- let mut uninit: Instance = mem::uninitialized();
- uninit.magic = 0;
- mem::replace(self.inst.as_mut(), uninit);
+ let inst = self.inst.as_mut();
+
+ // Grab a handle to the region to ensure it outlives `inst`.
+ //
+ // This ensures that the region won't be dropped by `inst` being
+ // dropped, which could result in `inst` being unmapped by the
+ // Region *during* drop of the Instance's fields.
+ let region: Arc<dyn RegionInternal> = inst.alloc().region.clone();
+
+ // drop the actual instance
+ std::ptr::drop_in_place(inst);
+
+ // and now we can drop what may be the last Arc<Region>. If it is
+ // it can safely do what it needs with memory; we're not running
+ // destructors on it anymore.
+ mem::drop(region);
}
}
}
@@ -211,6 +223,21 @@ pub struct Instance {
_padding: (),
}
+/// Users of `Instance` must be very careful about when instances are dropped!
+///
+/// Typically you will not have to worry about this, as InstanceHandle will robustly handle
+/// Instance drop semantics. If an instance is dropped, and the Region it's in has already dropped,
+/// it may contain the last reference counted pointer to its Region. If so, when Instance's
+/// destructor runs, Region will be dropped, and may free or otherwise invalidate the memory that
+/// this Instance exists in, *while* the Instance destructor is executing.
+impl Drop for Instance {
+ fn drop(&mut self) {
+ // Reset magic to indicate this instance
+ // is no longer valid
+ self.magic = 0;
+ }
+}
+
/// APIs that are internal, but useful to implementors of extension modules; you probably don't want
/// this trait!
///
