add a hook to ResourceLimiter to detect memory grow failure.

* allow the ResourceLimiter to reject a memory grow before the
memory's own maximum.
* add a hook so a ResourceLimiter can detect any reason that
a memory grow fails, including if the OS denies additional memory
* add tests for this new functionality. I only took the time to
test the OS denial on Linux, it should be possible on Mac OS
as well but I don't have a test setup. I have no idea how to
do this on windows.

crates/runtime/src/instance.rs

@@ -12,6 +12,7 @@ use crate::vmcontext::{
     VMInterrupts, VMMemoryDefinition, VMMemoryImport, VMTableDefinition, VMTableImport,
 };
 use crate::{ExportFunction, ExportGlobal, ExportMemory, ExportTable, Store};
+use anyhow::Error;
 use memoffset::offset_of;
 use more_asserts::assert_lt;
 use std::alloc::Layout;
@@ -64,6 +65,14 @@ pub trait ResourceLimiter {
     /// this method.
     fn memory_growing(&mut self, current: usize, desired: usize, maximum: Option<usize>) -> bool;
 
+    /// Notifies the resource limiter that growing a linear memory, permitted by
+    /// the `memory_growing` method, has failed.
+    ///
+    /// Reasons for failure include: the growth exceeds the `maximum` passed to
+    /// `memory_growing`, or the operating system failed to allocate additional
+    /// memory. In that case, `error` might be downcastable to a `std::io::Error`.
+    fn memory_grow_failed(&mut self, _error: &Error) {}
+
     /// Notifies the resource limiter that an instance's table has been requested to grow.
     ///
     /// * `current` is the current number of elements in the table.

crates/runtime/src/memory.rs

@@ -5,7 +5,7 @@
 use crate::mmap::Mmap;
 use crate::vmcontext::VMMemoryDefinition;
 use crate::ResourceLimiter;
-use anyhow::{bail, format_err, Result};
+use anyhow::{bail, format_err, Error, Result};
 use more_asserts::{assert_ge, assert_le};
 use std::convert::TryFrom;
 use wasmtime_environ::{MemoryPlan, MemoryStyle, WASM32_MAX_PAGES, WASM64_MAX_PAGES};
@@ -50,9 +50,9 @@ pub trait RuntimeLinearMemory: Send + Sync {
 
     /// Grow memory to the specified amount of bytes.
     ///
-    /// Returns `None` if memory can't be grown by the specified amount
+    /// Returns an error if memory can't be grown by the specified amount
     /// of bytes.
-    fn grow_to(&mut self, size: usize) -> Option<()>;
+    fn grow_to(&mut self, size: usize) -> Result<()>;
 
     /// Return a `VMMemoryDefinition` for exposing the memory to compiled wasm
     /// code.
@@ -137,7 +137,7 @@ impl RuntimeLinearMemory for MmapMemory {
         self.maximum
     }
 
-    fn grow_to(&mut self, new_size: usize) -> Option<()> {
+    fn grow_to(&mut self, new_size: usize) -> Result<()> {
         if new_size > self.mmap.len() - self.offset_guard_size - self.pre_guard_size {
             // If the new size of this heap exceeds the current size of the
             // allocation we have, then this must be a dynamic heap. Use
@@ -145,14 +145,13 @@ impl RuntimeLinearMemory for MmapMemory {
             // and then copy over the memory from before.
             let request_bytes = self
                 .pre_guard_size
-                .checked_add(new_size)?
+                .checked_add(new_size)
-                .checked_add(self.extra_to_reserve_on_growth)?
+                .and_then(|s| s.checked_add(self.extra_to_reserve_on_growth))
-                .checked_add(self.offset_guard_size)?;
+                .and_then(|s| s.checked_add(self.offset_guard_size))
+                .ok_or_else(|| format_err!("overflow calculating size of memory allocation"))?;
 
-            let mut new_mmap = Mmap::accessible_reserved(0, request_bytes).ok()?;
+            let mut new_mmap = Mmap::accessible_reserved(0, request_bytes)?;
-            new_mmap
+            new_mmap.make_accessible(self.pre_guard_size, new_size)?;
-                .make_accessible(self.pre_guard_size, new_size)
-                .ok()?;
 
             new_mmap.as_mut_slice()[self.pre_guard_size..][..self.accessible]
                 .copy_from_slice(&self.mmap.as_slice()[self.pre_guard_size..][..self.accessible]);
@@ -166,17 +165,15 @@ impl RuntimeLinearMemory for MmapMemory {
             // initial allocation to grow into before the heap is moved in
             // memory.
             assert!(new_size > self.accessible);
-            self.mmap
+            self.mmap.make_accessible(
-                .make_accessible(
+                self.pre_guard_size + self.accessible,
-                    self.pre_guard_size + self.accessible,
+                new_size - self.accessible,
-                    new_size - self.accessible,
+            )?;
-                )
-                .ok()?;
         }
 
         self.accessible = new_size;
 
-        Some(())
+        Ok(())
     }
 
     fn vmmemory(&self) -> VMMemoryDefinition {
@@ -215,6 +212,25 @@ pub enum Memory {
     Dynamic(Box<dyn RuntimeLinearMemory>),
 }
 
+fn memory_growing(
+    limiter: &mut Option<&mut dyn ResourceLimiter>,
+    current: usize,
+    desired: usize,
+    maximum: Option<usize>,
+) -> bool {
+    match limiter {
+        Some(ref mut l) => l.memory_growing(current, desired, maximum),
+        None => true,
+    }
+}
+
+fn memory_grow_failed(limiter: &mut Option<&mut dyn ResourceLimiter>, error: &Error) {
+    match limiter {
+        Some(l) => l.memory_grow_failed(error),
+        None => {}
+    }
+}
+
 impl Memory {
     /// Create a new dynamic (movable) memory instance for the specified plan.
     pub fn new_dynamic(
@@ -260,7 +276,7 @@ impl Memory {
     /// bytes.
     fn limit_new(
         plan: &MemoryPlan,
-        limiter: Option<&mut dyn ResourceLimiter>,
+        mut limiter: Option<&mut dyn ResourceLimiter>,
     ) -> Result<(usize, Option<usize>)> {
         // Sanity-check what should already be true from wasm module validation.
         let absolute_max = if plan.memory.memory64 {
@@ -322,13 +338,11 @@ impl Memory {
         // calculation overflowed. This means that the `minimum` we're informing
         // the limiter is lossy and may not be 100% accurate, but for now the
         // expected uses of `limiter` means that's ok.
-        if let Some(limiter) = limiter {
+        if !memory_growing(&mut limiter, 0, minimum.unwrap_or(absolute_max), maximum) {
-            if !limiter.memory_growing(0, minimum.unwrap_or(absolute_max), maximum) {
+            bail!(
-                bail!(
+                "memory minimum size of {} pages exceeds memory limits",
-                    "memory minimum size of {} pages exceeds memory limits",
+                plan.memory.minimum
-                    plan.memory.minimum
+            );
-                );
-            }
         }
 
         // At this point we need to actually handle overflows, so bail out with
@@ -389,26 +403,40 @@ impl Memory {
     pub unsafe fn grow(
         &mut self,
         delta_pages: u64,
-        limiter: Option<&mut dyn ResourceLimiter>,
+        mut limiter: Option<&mut dyn ResourceLimiter>,
     ) -> Option<usize> {
         let old_byte_size = self.byte_size();
+        // Wasm spec: when growing by 0 pages, always return the current size.
         if delta_pages == 0 {
             return Some(old_byte_size);
         }
 
+        // largest wasm-page-aligned region of memory it is possible to
+        // represent in a usize. This will be impossible for the system to
+        // actually allocate.
+        let absolute_max = 0usize.wrapping_sub(WASM_PAGE_SIZE);
+        // calculate byte size of the new allocation. Let it overflow up to
+        // usize::MAX, then clamp it down to absolute_max.
         let new_byte_size = usize::try_from(delta_pages)
-            .ok()?
+            .unwrap_or(usize::MAX)
-            .checked_mul(WASM_PAGE_SIZE)?
+            .saturating_mul(WASM_PAGE_SIZE)
-            .checked_add(old_byte_size)?;
+            .saturating_add(old_byte_size);
-        let maximum = self.maximum_byte_size();
+        let new_byte_size = if new_byte_size > absolute_max {
+            absolute_max
+        } else {
+            new_byte_size
+        };
 
+        let maximum = self.maximum_byte_size();
+        // Limiter gets first chance to reject memory_growing.
+        if !memory_growing(&mut limiter, old_byte_size, new_byte_size, maximum) {
+            return None;
+        }
+
+        // Never exceed maximum, even if limiter permitted it.
         if let Some(max) = maximum {
             if new_byte_size > max {
-                return None;
+                memory_grow_failed(&mut limiter, &format_err!("Memory maximum size exceeded"));
-            }
-        }
-        if let Some(limiter) = limiter {
-            if !limiter.memory_growing(old_byte_size, new_byte_size, maximum) {
                 return None;
             }
         }
@@ -428,19 +456,25 @@ impl Memory {
                 make_accessible,
                 ..
             } => {
+                // Never exceed static memory size
                 if new_byte_size > base.len() {
+                    memory_grow_failed(&mut limiter, &format_err!("static memory size exceeded"));
                     return None;
                 }
 
-                make_accessible(
+                // Operating system can fail to make memory accessible
+                let r = make_accessible(
                     base.as_mut_ptr().add(old_byte_size),
                     new_byte_size - old_byte_size,
-                )
+                );
-                .ok()?;
+                r.map_err(|e| memory_grow_failed(&mut limiter, &e)).ok()?;
 
                 *size = new_byte_size;
             }
-            Memory::Dynamic(mem) => mem.grow_to(new_byte_size)?,
+            Memory::Dynamic(mem) => {
+                let r = mem.grow_to(new_byte_size);
+                r.map_err(|e| memory_grow_failed(&mut limiter, &e)).ok()?;
+            }
         }
         Some(old_byte_size)
     }

crates/wasmtime/src/memory.rs

@@ -524,9 +524,10 @@ pub unsafe trait LinearMemory: Send + Sync + 'static {
 
     /// Grows this memory to have the `new_size`, in bytes, specified.
     ///
-    /// Returns `None` if memory can't be grown by the specified amount
+    /// Returns `Err` if memory can't be grown by the specified amount
-    /// of bytes. Returns `Some` if memory was grown successfully.
+    /// of bytes. The error may be downcastable to `std::io::Error`.
-    fn grow_to(&mut self, new_size: usize) -> Option<()>;
+    /// Returns `Ok` if memory was grown successfully.
+    fn grow_to(&mut self, new_size: usize) -> Result<()>;
 
     /// Return the allocated memory as a mutable pointer to u8.
     fn as_ptr(&self) -> *mut u8;

crates/wasmtime/src/trampoline/memory.rs

@@ -36,7 +36,7 @@ impl RuntimeLinearMemory for LinearMemoryProxy {
         self.mem.maximum_byte_size()
     }
 
-    fn grow_to(&mut self, new_size: usize) -> Option<()> {
+    fn grow_to(&mut self, new_size: usize) -> Result<()> {
         self.mem.grow_to(new_size)
     }
 

tests/all/limits.rs

@@ -1,4 +1,4 @@
-use anyhow::Result;
+use anyhow::{bail, Result};
 use wasmtime::*;
 
 const WASM_PAGE_SIZE: usize = wasmtime_environ::WASM_PAGE_SIZE as usize;
@@ -385,3 +385,161 @@ fn test_custom_limiter() -> Result<()> {
 
     Ok(())
 }
+
+#[derive(Default)]
+struct MemoryGrowFailureDetector {
+    /// Arguments of most recent call to memory_growing
+    current: usize,
+    desired: usize,
+    /// Display impl of most recent call to memory_grow_failed
+    error: Option<String>,
+}
+
+impl ResourceLimiter for MemoryGrowFailureDetector {
+    fn memory_growing(&mut self, current: usize, desired: usize, _maximum: Option<usize>) -> bool {
+        self.current = current;
+        self.desired = desired;
+        true
+    }
+
+    fn memory_grow_failed(&mut self, err: &anyhow::Error) {
+        self.error = Some(err.to_string());
+    }
+
+    fn table_growing(&mut self, _current: u32, _desired: u32, _maximum: Option<u32>) -> bool {
+        true
+    }
+}
+
+#[test]
+fn custom_limiter_detect_grow_failure() -> Result<()> {
+    let mut config = Config::new();
+    config.allocation_strategy(InstanceAllocationStrategy::Pooling {
+        strategy: PoolingAllocationStrategy::NextAvailable,
+        module_limits: ModuleLimits {
+            memory_pages: 10,
+            ..Default::default()
+        },
+        instance_limits: InstanceLimits {
+            ..Default::default()
+        },
+    });
+    let engine = Engine::new(&config).unwrap();
+    let linker = Linker::new(&engine);
+
+    let module = Module::new(&engine, r#"(module (memory (export "m") 0))"#)?;
+
+    let context = MemoryGrowFailureDetector::default();
+
+    let mut store = Store::new(&engine, context);
+    store.limiter(|s| s as &mut dyn ResourceLimiter);
+    let instance = linker.instantiate(&mut store, &module)?;
+    let memory = instance.get_memory(&mut store, "m").unwrap();
+
+    // Grow the memory by 640 KiB (10 pages)
+    memory.grow(&mut store, 10)?;
+
+    assert!(store.data().error.is_none());
+    assert_eq!(store.data().current, 0);
+    assert_eq!(store.data().desired, 10 * 64 * 1024);
+
+    // Grow past the static limit set by ModuleLimits.
+    // The ResourcLimiter will permit this, but the grow will fail.
+    assert_eq!(
+        memory.grow(&mut store, 1).unwrap_err().to_string(),
+        "failed to grow memory by `1`"
+    );
+
+    assert_eq!(store.data().current, 10 * 64 * 1024);
+    assert_eq!(store.data().desired, 11 * 64 * 1024);
+    assert_eq!(
+        store.data().error.as_ref().unwrap(),
+        "Memory maximum size exceeded"
+    );
+
+    drop(store);
+
+    Ok(())
+}
+
+#[cfg(target_os = "linux")]
+#[test]
+fn custom_limiter_detect_os_oom_failure() -> Result<()> {
+    let pid = unsafe { libc::fork() };
+    if pid == 0 {
+        // Child process
+        let r = std::panic::catch_unwind(|| {
+            // Ask Linux to limit this process to 128MiB of memory
+            let process_max_memory: usize = 128 * 1024 * 1024;
+            unsafe {
+                // limit process to 128MiB memory
+                let rlimit = libc::rlimit {
+                    rlim_cur: 0,
+                    rlim_max: process_max_memory as u64,
+                };
+                let res = libc::setrlimit(libc::RLIMIT_DATA, &rlimit);
+                assert_eq!(res, 0, "setrlimit failed: {}", res);
+            };
+
+            // Default behavior of on-demand memory allocation so that a
+            // memory grow will hit Linux for a larger mmap.
+            let engine = Engine::default();
+            let linker = Linker::new(&engine);
+            let module = Module::new(&engine, r#"(module (memory (export "m") 0))"#).unwrap();
+
+            let context = MemoryGrowFailureDetector::default();
+
+            let mut store = Store::new(&engine, context);
+            store.limiter(|s| s as &mut dyn ResourceLimiter);
+            let instance = linker.instantiate(&mut store, &module).unwrap();
+            let memory = instance.get_memory(&mut store, "m").unwrap();
+
+            // Small (640KiB) grow should succeed
+            memory.grow(&mut store, 10).unwrap();
+            assert!(store.data().error.is_none());
+            assert_eq!(store.data().current, 0);
+            assert_eq!(store.data().desired, 10 * 64 * 1024);
+
+            // Try to grow past the process's memory limit.
+            // This should fail.
+            let pages_exceeding_limit = process_max_memory / (64 * 1024);
+            let err_msg = memory
+                .grow(&mut store, pages_exceeding_limit as u64)
+                .unwrap_err()
+                .to_string();
+            assert!(
+                err_msg.starts_with("failed to grow memory"),
+                "unexpected error: {}",
+                err_msg
+            );
+
+            assert_eq!(store.data().current, 10 * 64 * 1024);
+            assert_eq!(
+                store.data().desired,
+                (pages_exceeding_limit + 10) * 64 * 1024
+            );
+            // The memory_grow_failed hook should show Linux gave OOM:
+            let err_msg = store.data().error.as_ref().unwrap();
+            assert!(
+                err_msg.starts_with("System call failed: Cannot allocate memory"),
+                "unexpected error: {}",
+                err_msg
+            );
+        });
+        // on assertion failure, exit 1 so parent process can fail test.
+        std::process::exit(if r.is_err() { 1 } else { 0 });
+    } else {
+        // Parent process
+        let mut wstatus: libc::c_int = 0;
+        unsafe { libc::wait(&mut wstatus) };
+        if libc::WIFEXITED(wstatus) {
+            if libc::WEXITSTATUS(wstatus) == 0 {
+                Ok(())
+            } else {
+                bail!("child exited with failure");
+            }
+        } else {
+            bail!("child didnt exit??")
+        }
+    }
+}

tests/all/memory_creator.rs

@@ -62,7 +62,7 @@ mod not_for_windows {
             Some(self.size - self.guard_size)
         }
 
-        fn grow_to(&mut self, new_size: usize) -> Option<()> {
+        fn grow_to(&mut self, new_size: usize) -> Result<(), anyhow::Error> {
             println!("grow to {:x}", new_size);
             let delta = new_size - self.used_wasm_bytes;
             unsafe {
@@ -73,7 +73,7 @@ mod not_for_windows {
 
             *self.glob_bytes_counter.lock().unwrap() += delta;
             self.used_wasm_bytes = new_size;
-            Some(())
+            Ok(())
         }
 
         fn as_ptr(&self) -> *mut u8 {