Code review feedback.

* Implemented `StoreLimits` and `StoreLimitsBuilder`.
* Moved `max_instances`, `max_memories`, `max_tables` out of `Config` and into
  `StoreLimits`.
* Moved storage of the limiter in the runtime into `Memory` and `Table`.
* Made `InstanceAllocationRequest` use a reference to the limiter.
* Updated docs.
* Made `ResourceLimiterProxy` generic to remove a level of indirection.
* Fixed the limiter not being used for `wasmtime::Memory` and
  `wasmtime::Table`.

crates/c-api/src/config.rs

@@ -176,8 +176,3 @@ pub extern "C" fn wasmtime_config_static_memory_guard_size_set(c: &mut wasm_conf
 pub extern "C" fn wasmtime_config_dynamic_memory_guard_size_set(c: &mut wasm_config_t, size: u64) {
     c.config.dynamic_memory_guard_size(size);
 }
-
-#[no_mangle]
-pub extern "C" fn wasmtime_config_max_instances_set(c: &mut wasm_config_t, limit: usize) {
-    c.config.max_instances(limit);
-}

crates/fuzzing/src/lib.rs

@@ -39,9 +39,6 @@ pub fn fuzz_default_config(strategy: wasmtime::Strategy) -> anyhow::Result<wasmt
         .wasm_bulk_memory(true)
         .wasm_reference_types(true)
         .wasm_module_linking(true)
-        .max_instances(100)
-        .max_tables(100)
-        .max_memories(100)
         .strategy(strategy)?;
     Ok(config)
 }

crates/fuzzing/src/oracles.rs

@@ -40,6 +40,17 @@ fn log_wasm(wasm: &[u8]) {
     }
 }
 
+fn create_store(engine: &Engine) -> Store {
+    Store::new_with_limits(
+        &engine,
+        StoreLimitsBuilder::new()
+            .instances(100)
+            .tables(100)
+            .memories(100)
+            .build(),
+    )
+}
+
 /// Methods of timing out execution of a WebAssembly module
 #[derive(Debug)]
 pub enum Timeout {
@@ -91,7 +102,7 @@ pub fn instantiate_with_config(
         _ => false,
     });
     let engine = Engine::new(&config).unwrap();
-    let store = Store::new(&engine);
+    let store = create_store(&engine);
 
     let mut timeout_state = SignalOnDrop::default();
     match timeout {
@@ -181,7 +192,7 @@ pub fn differential_execution(
 
     for config in &configs {
         let engine = Engine::new(config).unwrap();
-        let store = Store::new(&engine);
+        let store = create_store(&engine);
 
         let module = Module::new(&engine, &wasm).unwrap();
 
@@ -326,7 +337,7 @@ pub fn make_api_calls(api: crate::generators::api::ApiCalls) {
             ApiCall::StoreNew => {
                 log::trace!("creating store");
                 assert!(store.is_none());
-                store = Some(Store::new(engine.as_ref().unwrap()));
+                store = Some(create_store(engine.as_ref().unwrap()));
             }
 
             ApiCall::ModuleNew { id, wasm } => {
@@ -416,7 +427,7 @@ pub fn spectest(fuzz_config: crate::generators::Config, test: crate::generators:
     let mut config = fuzz_config.to_wasmtime();
     config.wasm_reference_types(false);
     config.wasm_bulk_memory(false);
-    let store = Store::new(&Engine::new(&config).unwrap());
+    let store = create_store(&Engine::new(&config).unwrap());
     if fuzz_config.consume_fuel {
         store.add_fuel(u64::max_value()).unwrap();
     }
@@ -440,7 +451,7 @@ pub fn table_ops(
         let mut config = fuzz_config.to_wasmtime();
         config.wasm_reference_types(true);
         let engine = Engine::new(&config).unwrap();
-        let store = Store::new(&engine);
+        let store = create_store(&engine);
         if fuzz_config.consume_fuel {
             store.add_fuel(u64::max_value()).unwrap();
         }
@@ -555,7 +566,7 @@ pub fn differential_wasmi_execution(wasm: &[u8], config: &crate::generators::Con
     let mut wasmtime_config = config.to_wasmtime();
     wasmtime_config.cranelift_nan_canonicalization(true);
     let wasmtime_engine = Engine::new(&wasmtime_config).unwrap();
-    let wasmtime_store = Store::new(&wasmtime_engine);
+    let wasmtime_store = create_store(&wasmtime_engine);
     if config.consume_fuel {
         wasmtime_store.add_fuel(u64::max_value()).unwrap();
     }

crates/runtime/src/instance.rs

@@ -66,6 +66,21 @@ pub trait ResourceLimiter {
     /// `false` if not permitted. Returning `true` when a maximum has been exceeded will have no
     /// effect as the table will not grow.
     fn table_growing(&self, current: u32, desired: u32, maximum: Option<u32>) -> bool;
+
+    /// The maximum number of instances that can be created for a `Store`.
+    ///
+    /// Module instantiation will fail if this limit is exceeded.
+    fn instances(&self) -> usize;
+
+    /// The maximum number of tables that can be created for a `Store`.
+    ///
+    /// Module instantiation will fail if this limit is exceeded.
+    fn tables(&self) -> usize;
+
+    /// The maximum number of tables that can be created for a `Store`.
+    ///
+    /// Module instantiation will fail if this limit is exceeded.
+    fn memories(&self) -> usize;
 }
 
 /// Runtime representation of an instance value, which erases all `Instance`
@@ -100,9 +115,6 @@ pub(crate) struct Instance {
     /// Hosts can store arbitrary per-instance information here.
     host_state: Box<dyn Any>,
 
-    /// The limiter to use for the instance, if there is one.
-    limiter: Option<Arc<dyn ResourceLimiter>>,
-
     /// Additional context used by compiled wasm code. This field is last, and
     /// represents a dynamically-sized array that extends beyond the nominal
     /// end of the struct (similar to a flexible array member).
@@ -417,15 +429,6 @@ impl Instance {
             .get(memory_index)
             .unwrap_or_else(|| panic!("no memory for index {}", memory_index.index()));
 
-        if let Some(limiter) = &self.limiter {
-            let current = memory.size();
-            let desired = current.checked_add(delta)?;
-
-            if !limiter.memory_growing(current, desired, memory.maximum()) {
-                return None;
-            }
-        }
-
         let result = unsafe { memory.grow(delta) };
 
         // Keep current the VMContext pointers used by compiled wasm code.
@@ -509,15 +512,6 @@ impl Instance {
             .get(table_index)
             .unwrap_or_else(|| panic!("no table for index {}", table_index.index()));
 
-        if let Some(limiter) = &self.limiter {
-            let current = table.size();
-            let desired = current.checked_add(delta)?;
-
-            if !limiter.table_growing(current, desired, table.maximum()) {
-                return None;
-            }
-        }
-
         let result = unsafe { table.grow(delta, init_value) };
 
         // Keep the `VMContext` pointers used by compiled Wasm code up to

crates/runtime/src/instance/allocator.rs

@@ -61,7 +61,7 @@ pub struct InstanceAllocationRequest<'a> {
     pub stack_map_registry: *mut StackMapRegistry,
 
     /// The resource limiter to use for the instance.
-    pub limiter: Option<Arc<dyn ResourceLimiter>>,
+    pub limiter: Option<&'a Arc<dyn ResourceLimiter>>,
 }
 
 /// An link error while instantiating a module.
@@ -550,19 +550,23 @@ impl OnDemandInstanceAllocator {
         Self { mem_creator }
     }
 
-    fn create_tables(module: &Module) -> PrimaryMap<DefinedTableIndex, Table> {
+    fn create_tables(
+        module: &Module,
+        limiter: &Option<&Arc<dyn ResourceLimiter>>,
+    ) -> PrimaryMap<DefinedTableIndex, Table> {
         let num_imports = module.num_imported_tables;
         let mut tables: PrimaryMap<DefinedTableIndex, _> =
             PrimaryMap::with_capacity(module.table_plans.len() - num_imports);
         for table in &module.table_plans.values().as_slice()[num_imports..] {
-            tables.push(Table::new_dynamic(table));
+            tables.push(Table::new_dynamic(table, limiter));
         }
         tables
     }
 
     fn create_memories(
         &self,
         module: &Module,
+        limiter: &Option<&Arc<dyn ResourceLimiter>>,
     ) -> Result<PrimaryMap<DefinedMemoryIndex, Memory>, InstantiationError> {
         let creator = self
             .mem_creator
@@ -572,8 +576,10 @@ impl OnDemandInstanceAllocator {
         let mut memories: PrimaryMap<DefinedMemoryIndex, _> =
             PrimaryMap::with_capacity(module.memory_plans.len() - num_imports);
         for plan in &module.memory_plans.values().as_slice()[num_imports..] {
-            memories
-                .push(Memory::new_dynamic(plan, creator).map_err(InstantiationError::Resource)?);
+            memories.push(
+                Memory::new_dynamic(plan, creator, limiter)
+                    .map_err(InstantiationError::Resource)?,
+            );
         }
         Ok(memories)
     }
@@ -584,11 +590,10 @@ unsafe impl InstanceAllocator for OnDemandInstanceAllocator {
         &self,
         mut req: InstanceAllocationRequest,
     ) -> Result<InstanceHandle, InstantiationError> {
-        let memories = self.create_memories(&req.module)?;
-        let tables = Self::create_tables(&req.module);
+        let memories = self.create_memories(&req.module, &req.limiter)?;
+        let tables = Self::create_tables(&req.module, &req.limiter);
 
         let host_state = std::mem::replace(&mut req.host_state, Box::new(()));
-        let limiter = std::mem::take(&mut req.limiter);
 
         let handle = {
             let instance = Instance {
@@ -601,7 +606,6 @@ unsafe impl InstanceAllocator for OnDemandInstanceAllocator {
                 )),
                 dropped_data: RefCell::new(EntitySet::with_capacity(req.module.passive_data.len())),
                 host_state,
-                limiter,
                 vmctx: VMContext {},
             };
             let layout = instance.alloc_layout();

crates/runtime/src/instance/allocator/pooling.rs

@@ -9,7 +9,7 @@
 
 use super::{
     initialize_instance, initialize_vmcontext, FiberStackError, InstanceAllocationRequest,
-    InstanceAllocator, InstanceHandle, InstantiationError,
+    InstanceAllocator, InstanceHandle, InstantiationError, ResourceLimiter,
 };
 use crate::{instance::Instance, Memory, Mmap, Table, VMContext};
 use anyhow::{anyhow, bail, Context, Result};
@@ -367,7 +367,6 @@ impl InstancePool {
                     dropped_elements: RefCell::new(EntitySet::new()),
                     dropped_data: RefCell::new(EntitySet::new()),
                     host_state: Box::new(()),
-                    limiter: None,
                     vmctx: VMContext {},
                 },
             );
@@ -389,7 +388,6 @@ impl InstancePool {
         };
 
         let host_state = std::mem::replace(&mut req.host_state, Box::new(()));
-        let limiter = std::mem::take(&mut req.limiter);
 
         unsafe {
             let instance = self.instance(index);
@@ -400,14 +398,20 @@ impl InstancePool {
                 instance.module.as_ref(),
             );
             instance.host_state = host_state;
-            instance.limiter = limiter;
 
             Self::set_instance_memories(
                 instance,
                 self.memories.get(index),
                 self.memories.max_wasm_pages,
+                &req.limiter,
+            )?;
+
+            Self::set_instance_tables(
+                instance,
+                self.tables.get(index),
+                self.tables.max_elements,
+                &req.limiter,
             )?;
-            Self::set_instance_tables(instance, self.tables.get(index), self.tables.max_elements)?;
 
             initialize_vmcontext(instance, req);
 
@@ -465,9 +469,8 @@ impl InstancePool {
         instance.tables.clear();
         instance.dropped_elements.borrow_mut().clear();
 
-        // Drop any host state and limiter
+        // Drop any host state
         instance.host_state = Box::new(());
-        instance.limiter = None;
 
         self.free_list.lock().unwrap().push(index);
     }
@@ -476,6 +479,7 @@ impl InstancePool {
         instance: &mut Instance,
         mut memories: impl Iterator<Item = *mut u8>,
         max_pages: u32,
+        limiter: &Option<&Arc<dyn ResourceLimiter>>,
     ) -> Result<(), InstantiationError> {
         let module = instance.module.as_ref();
 
@@ -490,6 +494,7 @@ impl InstancePool {
                     memories.next().unwrap(),
                     max_pages,
                     commit_memory_pages,
+                    limiter,
                 )
                 .map_err(InstantiationError::Resource)?,
             );
@@ -506,6 +511,7 @@ impl InstancePool {
         instance: &mut Instance,
         mut tables: impl Iterator<Item = *mut u8>,
         max_elements: u32,
+        limiter: &Option<&Arc<dyn ResourceLimiter>>,
     ) -> Result<(), InstantiationError> {
         let module = instance.module.as_ref();
 
@@ -519,7 +525,7 @@ impl InstancePool {
 
             instance
                 .tables
-                .push(Table::new_static(plan, base as _, max_elements));
+                .push(Table::new_static(plan, base as _, max_elements, limiter));
         }
 
         let mut dropped_elements = instance.dropped_elements.borrow_mut();