review feedback

This commit is contained in:
Pat Hickey
2021-10-21 12:04:20 -07:00
parent abbe28d833
commit 6c70b81ff5
3 changed files with 21 additions and 13 deletions

View File

@@ -97,10 +97,10 @@ impl StorePtr {
self.0.clone() self.0.clone()
} }
/// Use the StorePtr as a mut ref to the Store. /// Use the StorePtr as a mut ref to the Store.
// XXX should this be an unsafe fn? is it always safe at a use site? /// Safety: must not be used outside the original lifetime of the borrow.
pub(crate) fn get(&mut self) -> Option<&mut dyn Store> { pub(crate) unsafe fn get(&mut self) -> Option<&mut dyn Store> {
match self.0 { match self.0 {
Some(ptr) => Some(unsafe { &mut *ptr }), Some(ptr) => Some(&mut *ptr),
None => None, None => None,
} }
} }
@@ -630,12 +630,11 @@ impl OnDemandInstanceAllocator {
PrimaryMap::with_capacity(module.table_plans.len() - num_imports); PrimaryMap::with_capacity(module.table_plans.len() - num_imports);
for table in &module.table_plans.values().as_slice()[num_imports..] { for table in &module.table_plans.values().as_slice()[num_imports..] {
tables.push( tables.push(
Table::new_dynamic( Table::new_dynamic(table, unsafe {
table,
store store
.get() .get()
.expect("if module has table plans, store is not empty"), .expect("if module has table plans, store is not empty")
) })
.map_err(InstantiationError::Resource)?, .map_err(InstantiationError::Resource)?,
); );
} }
@@ -656,13 +655,11 @@ impl OnDemandInstanceAllocator {
PrimaryMap::with_capacity(module.memory_plans.len() - num_imports); PrimaryMap::with_capacity(module.memory_plans.len() - num_imports);
for plan in &module.memory_plans.values().as_slice()[num_imports..] { for plan in &module.memory_plans.values().as_slice()[num_imports..] {
memories.push( memories.push(
Memory::new_dynamic( Memory::new_dynamic(plan, creator, unsafe {
plan,
creator,
store store
.get() .get()
.expect("if module has memory plans, store is not empty"), .expect("if module has memory plans, store is not empty")
) })
.map_err(InstantiationError::Resource)?, .map_err(InstantiationError::Resource)?,
); );
} }

View File

@@ -378,6 +378,9 @@ impl Memory {
/// ///
/// Generally, prefer using `InstanceHandle::memory_grow`, which encapsulates /// Generally, prefer using `InstanceHandle::memory_grow`, which encapsulates
/// this unsafety. /// this unsafety.
///
/// Ensure that the provided Store is not used to get access any Memory
/// which lives inside it.
pub unsafe fn grow( pub unsafe fn grow(
&mut self, &mut self,
delta_pages: u64, delta_pages: u64,

View File

@@ -223,8 +223,12 @@ impl Memory {
Memory::_new(store.as_context_mut().0, ty) Memory::_new(store.as_context_mut().0, ty)
} }
/// Async variant of [`Memory::new`]. You must use this variant with Stores which have a /// Async variant of [`Memory::new`]. You must use this variant with [`Store`]s which have a
/// [`ResourceLimiterAsync`]. /// [`ResourceLimiterAsync`].
///
/// # Panics
///
/// This function will panic when used with a non-async [`Store`].
#[cfg(feature = "async")] #[cfg(feature = "async")]
pub async fn new_async<T>( pub async fn new_async<T>(
mut store: impl AsContextMut<Data = T>, mut store: impl AsContextMut<Data = T>,
@@ -491,6 +495,10 @@ impl Memory {
} }
/// Async variant of [`Memory::grow`]. Required when using a [`ResourceLimiterAsync`]. /// Async variant of [`Memory::grow`]. Required when using a [`ResourceLimiterAsync`].
///
/// # Panics
///
/// This function will panic when used with a non-async [`Store`].
#[cfg(feature = "async")] #[cfg(feature = "async")]
pub async fn grow_async<T>( pub async fn grow_async<T>(
&self, &self,