Repository: bytecodealliance/wasmtime
Pull request #6892 (2023-08-23 16:54:01)

Rename the wasi-http `struct` module to `types`
Rename the `struct` module to `types`, so that we don't need to use special syntax to avoid the keyword collision with the name.

<!--
Please make sure you include the following information:

- If this work has been discussed elsewhere, please include a link to that
  conversation. If it was discussed in an issue, just mention "issue #...".

- Explain why this change is needed. If the details are in an issue already,
  this can be brief.

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->



Patch:
diff --git a/crates/runtime/src/lib.rs b/crates/runtime/src/lib.rs
index ce4a2d576..3bcde9179 100644
--- a/crates/runtime/src/lib.rs
+++ b/crates/runtime/src/lib.rs
@@ -20,7 +20,7 @@
     )
 )]
 
-use anyhow::Error;
+use anyhow::{Error, Result};
 use std::fmt;
 use std::ptr::NonNull;
 use std::sync::atomic::{AtomicU64, AtomicUsize, Ordering};
@@ -132,7 +132,9 @@ pub unsafe trait Store {
     ) -> Result<bool, Error>;
     /// Callback invoked to notify the store's resource limiter that a memory
     /// grow operation has failed.
-    fn memory_grow_failed(&mut self, error: &Error);
+    ///
+    /// Note that this is not invoked if `memory_growing` returns an error.
+    fn memory_grow_failed(&mut self, error: Error) -> Result<()>;
     /// Callback invoked to allow the store's resource limiter to reject a
     /// table grow operation.
     fn table_growing(
@@ -143,7 +145,9 @@ pub unsafe trait Store {
     ) -> Result<bool, Error>;
     /// Callback invoked to notify the store's resource limiter that a table
     /// grow operation has failed.
-    fn table_grow_failed(&mut self, error: &Error);
+    ///
+    /// Note that this is not invoked if `table_growing` returns an error.
+    fn table_grow_failed(&mut self, error: Error) -> Result<()>;
     /// Callback invoked whenever fuel runs out by a wasm instance. If an error
     /// is returned that's raised as a trap. Otherwise wasm execution will
     /// continue as normal.
diff --git a/crates/runtime/src/memory.rs b/crates/runtime/src/memory.rs
index 6927b931a..84d568859 100644
--- a/crates/runtime/src/memory.rs
+++ b/crates/runtime/src/memory.rs
@@ -116,7 +116,7 @@ pub trait RuntimeLinearMemory: Send + Sync {
                     // to report the growth failure to but the error should not
                     // be dropped
                     // (https://github.com/bytecodealliance/wasmtime/issues/4240).
-                    store.memory_grow_failed(&format_err!("Memory maximum size exceeded"));
+                    store.memory_grow_failed(format_err!("Memory maximum size exceeded"))?;
                 }
                 return Ok(None);
             }
@@ -130,7 +130,7 @@ pub trait RuntimeLinearMemory: Send + Sync {
                 // dropped
                 // (https://github.com/bytecodealliance/wasmtime/issues/4240).
                 if let Some(store) = store {
-                    store.memory_grow_failed(&e);
+                    store.memory_grow_failed(e)?;
                 }
                 Ok(None)
             }
diff --git a/crates/runtime/src/table.rs b/crates/runtime/src/table.rs
index 7fb5ffed7..a978bc892 100644
--- a/crates/runtime/src/table.rs
+++ b/crates/runtime/src/table.rs
@@ -361,7 +361,10 @@ impl Table {
         let old_size = self.size();
         let new_size = match old_size.checked_add(delta) {
             Some(s) => s,
-            None => return Ok(None),
+            None => {
+                store.table_grow_failed(format_err!("overflow calculating new table size"))?;
+                return Ok(None);
+            }
         };
 
         if !store.table_growing(old_size, new_size, self.maximum())? {
@@ -370,7 +373,7 @@ impl Table {
 
         if let Some(max) = self.maximum() {
             if new_size > max {
-                store.table_grow_failed(&format_err!("Table maximum size exceeded"));
+                store.table_grow_failed(format_err!("Table maximum size exceeded"))?;
                 return Ok(None);
             }
         }
diff --git a/crates/wasi-http/src/http_impl.rs b/crates/wasi-http/src/http_impl.rs
index c0f2b7af4..39de620da 100644
--- a/crates/wasi-http/src/http_impl.rs
+++ b/crates/wasi-http/src/http_impl.rs
@@ -1,4 +1,4 @@
-use crate::r#struct::{ActiveFields, ActiveFuture, ActiveResponse, HttpResponse, TableHttpExt};
+use crate::types::{ActiveFields, ActiveFuture, ActiveResponse, HttpResponse, TableHttpExt};
 use crate::wasi::http::types::{FutureIncomingResponse, OutgoingRequest, RequestOptions, Scheme};
 pub use crate::{WasiHttpCtx, WasiHttpView};
 use anyhow::Context;
diff --git a/crates/wasi-http/src/lib.rs b/crates/wasi-http/src/lib.rs
index 95983787b..c2fedf5eb 100644
--- a/crates/wasi-http/src/lib.rs
+++ b/crates/wasi-http/src/lib.rs
@@ -1,6 +1,6 @@
 use crate::component_impl::add_component_to_linker;
 pub use crate::http_impl::WasiHttpViewExt;
-pub use crate::r#struct::{WasiHttpCtx, WasiHttpView};
+pub use crate::types::{WasiHttpCtx, WasiHttpView};
 use core::fmt::Formatter;
 use std::fmt::{self, Display};
 
@@ -15,7 +15,7 @@ wasmtime::component::bindgen!({
 
 pub mod component_impl;
 pub mod http_impl;
-pub mod r#struct;
+pub mod types;
 pub mod types_impl;
 
 pub fn add_to_component_linker<T>(linker: &mut wasmtime::component::Linker<T>) -> anyhow::Result<()>
diff --git a/crates/wasi-http/src/struct.rs b/crates/wasi-http/src/types.rs
similarity index 100%
rename from crates/wasi-http/src/struct.rs
rename to crates/wasi-http/src/types.rs
diff --git a/crates/wasi-http/src/types_impl.rs b/crates/wasi-http/src/types_impl.rs
index 027d1d4e9..11d16a4ab 100644
--- a/crates/wasi-http/src/types_impl.rs
+++ b/crates/wasi-http/src/types_impl.rs
@@ -1,5 +1,5 @@
 use crate::http_impl::WasiHttpViewExt;
-use crate::r#struct::{ActiveFields, ActiveRequest, HttpRequest, TableHttpExt};
+use crate::types::{ActiveFields, ActiveRequest, HttpRequest, TableHttpExt};
 use crate::wasi::http::types::{
     Error, Fields, FutureIncomingResponse, Headers, IncomingRequest, IncomingResponse,
     IncomingStream, Method, OutgoingRequest, OutgoingResponse, OutgoingStream, ResponseOutparam,
diff --git a/crates/wasmtime/src/limits.rs b/crates/wasmtime/src/limits.rs
index 718048544..5d0f934a5 100644
--- a/crates/wasmtime/src/limits.rs
+++ b/crates/wasmtime/src/limits.rs
@@ -76,10 +76,19 @@ pub trait ResourceLimiter {
     /// Notifies the resource limiter that growing a linear memory, permitted by
     /// the `memory_growing` method, has failed.
     ///
+    /// Note that this method is not called if `memory_growing` returns an
+    /// error.
+    ///
     /// 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: &anyhow::Error) {}
+    ///
+    /// See the details on the return values for `memory_growing` for what the
+    /// return value of this function indicates.
+    fn memory_grow_failed(&mut self, error: anyhow::Error) -> Result<()> {
+        log::debug!("ignoring memory growth failure error: {error:?}");
+        Ok(())
+    }
 
     /// Notifies the resource limiter that an instance's table has been
     /// requested to grow.
@@ -99,9 +108,17 @@ pub trait ResourceLimiter {
     /// Notifies the resource limiter that growing a linear memory, permitted by
     /// the `table_growing` method, has failed.
     ///
+    /// Note that this method is not called if `table_growing` returns an error.
+    ///
     /// Reasons for failure include: the growth exceeds the `maximum` passed to
     /// `table_growing`. This could expand in the future.
-    fn table_grow_failed(&mut self, _error: &anyhow::Error) {}
+    ///
+    /// See the details on the return values for `memory_growing` for what the
+    /// return value of this function indicates.
+    fn table_grow_failed(&mut self, error: anyhow::Error) -> Result<()> {
+        log::debug!("ignoring table growth failure error: {error:?}");
+        Ok(())
+    }
 
     /// The maximum number of instances that can be created for a `Store`.
     ///
@@ -160,7 +177,10 @@ pub trait ResourceLimiterAsync {
     ) -> Result<bool>;
 
     /// Identical to [`ResourceLimiter::memory_grow_failed`]
-    fn memory_grow_failed(&mut self, _error: &anyhow::Error) {}
+    fn memory_grow_failed(&mut self, error: anyhow::Error) -> Result<()> {
+        log::debug!("ignoring memory growth failure error: {error:?}");
+        Ok(())
+    }
 
     /// Asynchronous version of [`ResourceLimiter::table_growing`]
     async fn table_growing(
@@ -171,7 +191,10 @@ pub trait ResourceLimiterAsync {
     ) -> Result<bool>;
 
     /// Identical to [`ResourceLimiter::table_grow_failed`]
-    fn table_grow_failed(&mut self, _error: &anyhow::Error) {}
+    fn table_grow_failed(&mut self, error: anyhow::Error) -> Result<()> {
+        log::debug!("ignoring table growth failure error: {error:?}");
+        Ok(())
+    }
 
     /// Identical to [`ResourceLimiter::instances`]`
     fn instances(&self) -> usize {
@@ -328,6 +351,15 @@ impl ResourceLimiter for StoreLimits {
         }
     }
 
+    fn memory_grow_failed(&mut self, error: anyhow::Error) -> Result<()> {
+        if self.trap_on_grow_failure {
+            Err(error.context("forcing a memory growth failure to be a trap"))
+        } else {
+            log::debug!("ignoring memory growth failure error: {error:?}");
+            Ok(())
+        }
+    }
+
     fn table_growing(&mut self, _current: u32, desired: u32, maximum: Option<u32>) -> Result<bool> {
         let allow = match self.table_elements {
             Some(limit) if desired > limit => false,
@@ -343,6 +375,15 @@ impl ResourceLimiter for StoreLimits {
         }
     }
 
+    fn table_grow_failed(&mut self, error: anyhow::Error) -> Result<()> {
+        if self.trap_on_grow_failure {
+            Err(error.context("forcing a table growth failure to be a trap"))
+        } else {
+            log::debug!("ignoring table growth failure error: {error:?}");
+            Ok(())
+        }
+    }
+
     fn instances(&self) -> usize {
         self.instances
     }
diff --git a/crates/wasmtime/src/store.rs b/crates/wasmtime/src/store.rs
index 9da3b1e62..437db281e 100644
--- a/crates/wasmtime/src/store.rs
+++ b/crates/wasmtime/src/store.rs
@@ -1992,7 +1992,7 @@ unsafe impl<T> wasmtime_runtime::Store for StoreInner<T> {
         }
     }
 
-    fn memory_grow_failed(&mut self, error: &anyhow::Error) {
+    fn memory_grow_failed(&mut self, error: anyhow::Error) -> Result<()> {
         match self.limiter {
             Some(ResourceLimiterInner::Sync(ref mut limiter)) => {
                 limiter(&mut self.data).memory_grow_failed(error)
@@ -2001,7 +2001,10 @@ unsafe impl<T> wasmtime_runtime::Store for StoreInner<T> {
             Some(ResourceLimiterInner::Async(ref mut limiter)) => {
                 limiter(&mut self.data).memory_grow_failed(error)
             }
-            None => {}
+            None => {
+                log::debug!("ignoring memory growth failure error: {error:?}");
+                Ok(())
+            }
         }
     }
 
@@ -2039,7 +2042,7 @@ unsafe impl<T> wasmtime_runtime::Store for StoreInner<T> {
         }
     }
 
-    fn table_grow_failed(&mut self, error: &anyhow::Error) {
+    fn table_grow_failed(&mut self, error: anyhow::Error) -> Result<()> {
         match self.limiter {
             Some(ResourceLimiterInner::Sync(ref mut limiter)) => {
                 limiter(&mut self.data).table_grow_failed(error)
@@ -2048,7 +2051,10 @@ unsafe impl<T> wasmtime_runtime::Store for StoreInner<T> {
             Some(ResourceLimiterInner::Async(ref mut limiter)) => {
                 limiter(&mut self.data).table_grow_failed(error)
             }
-            None => {}
+            None => {
+                log::debug!("ignoring table growth failure: {error:?}");
+                Ok(())
+            }
         }
     }
 


Test patch:
diff --git a/tests/all/cli_tests.rs b/tests/all/cli_tests.rs
index 1f8200cc8..29561129c 100644
--- a/tests/all/cli_tests.rs
+++ b/tests/all/cli_tests.rs
@@ -831,3 +831,58 @@ fn run_precompiled_component() -> Result<()> {
 
     Ok(())
 }
+
+#[test]
+fn memory_growth_failure() -> Result<()> {
+    let output = get_wasmtime_command()?
+        .args(&[
+            "run",
+            "--wasm-features=memory64",
+            "--trap-on-grow-failure",
+            "tests/all/cli_tests/memory-grow-failure.wat",
+        ])
+        .output()?;
+    assert!(!output.status.success());
+    let stderr = String::from_utf8_lossy(&output.stderr);
+    assert!(
+        stderr.contains("forcing a memory growth failure to be a trap"),
+        "bad stderr: {stderr}"
+    );
+    Ok(())
+}
+
+#[test]
+fn table_growth_failure() -> Result<()> {
+    let output = get_wasmtime_command()?
+        .args(&[
+            "run",
+            "--trap-on-grow-failure",
+            "tests/all/cli_tests/table-grow-failure.wat",
+        ])
+        .output()?;
+    assert!(!output.status.success());
+    let stderr = String::from_utf8_lossy(&output.stderr);
+    assert!(
+        stderr.contains("forcing trap when growing table"),
+        "bad stderr: {stderr}"
+    );
+    Ok(())
+}
+
+#[test]
+fn table_growth_failure2() -> Result<()> {
+    let output = get_wasmtime_command()?
+        .args(&[
+            "run",
+            "--trap-on-grow-failure",
+            "tests/all/cli_tests/table-grow-failure2.wat",
+        ])
+        .output()?;
+    assert!(!output.status.success());
+    let stderr = String::from_utf8_lossy(&output.stderr);
+    assert!(
+        stderr.contains("forcing a table growth failure to be a trap"),
+        "bad stderr: {stderr}"
+    );
+    Ok(())
+}
diff --git a/tests/all/cli_tests/memory-grow-failure.wat b/tests/all/cli_tests/memory-grow-failure.wat
new file mode 100644
index 000000000..b5aa1b471
--- /dev/null
+++ b/tests/all/cli_tests/memory-grow-failure.wat
@@ -0,0 +1,8 @@
+(module
+  (memory i64 1)
+  (func (export "_start")
+    i64.const 0x0000800000000000
+    memory.grow
+    drop
+  )
+)
diff --git a/tests/all/cli_tests/table-grow-failure.wat b/tests/all/cli_tests/table-grow-failure.wat
new file mode 100644
index 000000000..9c31bc2e8
--- /dev/null
+++ b/tests/all/cli_tests/table-grow-failure.wat
@@ -0,0 +1,9 @@
+(module
+  (table 1 10 funcref)
+  (func (export "_start")
+    ref.null func
+    i32.const 11
+    table.grow
+    drop
+  )
+)
diff --git a/tests/all/cli_tests/table-grow-failure2.wat b/tests/all/cli_tests/table-grow-failure2.wat
new file mode 100644
index 000000000..d5390c5db
--- /dev/null
+++ b/tests/all/cli_tests/table-grow-failure2.wat
@@ -0,0 +1,9 @@
+(module
+  (table 1 funcref)
+  (func (export "_start")
+    ref.null func
+    i32.const -1
+    table.grow
+    drop
+  )
+)
diff --git a/tests/all/limits.rs b/tests/all/limits.rs
index 9adbb7587..cd357d5dc 100644
--- a/tests/all/limits.rs
+++ b/tests/all/limits.rs
@@ -541,7 +541,6 @@ impl ResourceLimiterAsync for MemoryContext {
     ) -> Result<bool> {
         Ok(true)
     }
-    fn table_grow_failed(&mut self, _e: &anyhow::Error) {}
 }
 
 #[tokio::test]
@@ -724,16 +723,18 @@ impl ResourceLimiter for FailureDetector {
         self.memory_desired = desired;
         Ok(true)
     }
-    fn memory_grow_failed(&mut self, err: &anyhow::Error) {
+    fn memory_grow_failed(&mut self, err: anyhow::Error) -> Result<()> {
         self.memory_error = Some(err.to_string());
+        Ok(())
     }
     fn table_growing(&mut self, current: u32, desired: u32, _maximum: Option<u32>) -> Result<bool> {
         self.table_current = current;
         self.table_desired = desired;
         Ok(true)
     }
-    fn table_grow_failed(&mut self, err: &anyhow::Error) {
+    fn table_grow_failed(&mut self, err: anyhow::Error) -> Result<()> {
         self.table_error = Some(err.to_string());
+        Ok(())
     }
 }
 
@@ -826,8 +827,9 @@ impl ResourceLimiterAsync for FailureDetector {
         self.memory_desired = desired;
         Ok(true)
     }
-    fn memory_grow_failed(&mut self, err: &anyhow::Error) {
+    fn memory_grow_failed(&mut self, err: anyhow::Error) -> Result<()> {
         self.memory_error = Some(err.to_string());
+        Ok(())
     }
 
     async fn table_growing(
@@ -840,8 +842,9 @@ impl ResourceLimiterAsync for FailureDetector {
         self.table_desired = desired;
         Ok(true)
     }
-    fn table_grow_failed(&mut self, err: &anyhow::Error) {
+    fn table_grow_failed(&mut self, err: anyhow::Error) -> Result<()> {
         self.table_error = Some(err.to_string());
+        Ok(())
     }
 }
 
diff --git a/tests/rlimited-memory.rs b/tests/rlimited-memory.rs
index bd57fdd0c..4a63e51f6 100644
--- a/tests/rlimited-memory.rs
+++ b/tests/rlimited-memory.rs
@@ -23,8 +23,9 @@ impl ResourceLimiter for MemoryGrowFailureDetector {
         self.desired = desired;
         Ok(true)
     }
-    fn memory_grow_failed(&mut self, err: &anyhow::Error) {
+    fn memory_grow_failed(&mut self, err: anyhow::Error) -> Result<()> {
         self.error = Some(err.to_string());
+        Ok(())
     }
     fn table_growing(
         &mut self,