c-api: Review feedback

Signed-off-by: Tyler Rockwood <rockwood@redpanda.com>

crates/c-api/include/wasmtime/config.h

@@ -367,35 +367,18 @@ WASM_API_EXTERN void wasmtime_config_cranelift_flag_enable(wasm_config_t*, const
  */
 WASM_API_EXTERN void wasmtime_config_cranelift_flag_set(wasm_config_t*, const char *key, const char *value);
 
-/**
- * A callback to create a new LinearMemory from the specified parameters.
- *
- * The result should be written to `memory_ret` and optionally a finalizer for the returned memory 
- * can be returned in the finalizer pointer.
- *
- * For more information about the parameters see the Rust documentation at
- * https://docs.wasmtime.dev/api/wasmtime/trait.MemoryCreator.html#tymethod.new_memory
- */
-typedef wasmtime_error_t *(*wasmtime_new_memory_callback_t)(
-    wasm_memorytype_t *ty,
-    size_t minimum,
-    size_t maximum,
-    size_t reserved_size_in_bytes,
-    size_t guard_size_in_bytes,
-    void **memory_ret,
-    void (**finalizer)(void*));
 
 /**
  * Return the data from a LinearMemory instance created from a #wasmtime_new_memory_t callback.
  *
  * The size in bytes as well as the maximum number of bytes that can be allocated should be 
  * returned as well.
  *
- * For more information about the parameters see the Rust documentation at
+ * For more information about see the Rust documentation at
  * https://docs.wasmtime.dev/api/wasmtime/trait.LinearMemory.html
  */
-typedef void *(*wasmtime_memory_get_callback_t)(
-    void *memory_ptr,
+typedef uint8_t *(*wasmtime_memory_get_callback_t)(
+    void *env,
     size_t *byte_size,
     size_t *maximum_byte_size);
 
@@ -406,19 +389,52 @@ typedef void *(*wasmtime_memory_get_callback_t)(
  * https://docs.wasmtime.dev/api/wasmtime/trait.LinearMemory.html#tymethod.grow_to
  */
 typedef wasmtime_error_t *(*wasmtime_memory_grow_callback_t)(
-    void *memory_ptr,
+    void *env,
     size_t new_size);
 
+/**
+ * A LinearMemory instance created from a #wasmtime_new_memory_callback_t.
+ *
+ * For more information see the Rust documentation at
+ * https://docs.wasmtime.dev/api/wasmtime/trait.LinearMemory.html
+ */
+typedef struct {
+  void *env;
+  wasmtime_memory_get_callback_t get_memory;
+  wasmtime_memory_grow_callback_t grow_memory;
+  void (*finalizer)(void*);
+} wasmtime_linear_memory_t;
+
+/**
+ * A callback to create a new LinearMemory from the specified parameters.
+ *
+ * The result should be written to `memory_ret` and wasmtime will own the values written 
+ * into that struct.
+ *
+ * This callback must be thread-safe.
+ *
+ * For more information about the parameters see the Rust documentation at
+ * https://docs.wasmtime.dev/api/wasmtime/trait.MemoryCreator.html#tymethod.new_memory
+ */
+typedef wasmtime_error_t *(*wasmtime_new_memory_callback_t)(
+    void *env,
+    const wasm_memorytype_t *ty,
+    size_t minimum,
+    size_t maximum,
+    size_t reserved_size_in_bytes,
+    size_t guard_size_in_bytes,
+    wasmtime_linear_memory_t *memory_ret);
+
 /**
  * A representation of custom memory creator and methods for an instance of LinearMemory.
  *
  * For more information see the Rust documentation at
  * https://docs.wasmtime.dev/api/wasmtime/trait.MemoryCreator.html
  */
 typedef struct {
+  void* env;
   wasmtime_new_memory_callback_t new_memory;
-  wasmtime_memory_get_callback_t get_memory;
-  wasmtime_memory_grow_callback_t grow_memory;
+  void (*finalizer)(void*);
 } wasmtime_memory_creator_t;
 
 /**
@@ -427,6 +443,9 @@ typedef struct {
  * Custom memory creators are used when creating host Memory objects or when creating instance
  * linear memories for the on-demand instance allocation strategy.
  *
+ * The config does **not** take ownership of the #wasmtime_memory_creator_t passed in, but 
+ * instead copies all the values in the struct.
+ *
  * For more information see the Rust documentation at
  * https://docs.wasmtime.dev/api/wasmtime/struct.Config.html#method.with_host_memory
  */

crates/c-api/src/config.rs

@@ -3,9 +3,9 @@
 #![cfg_attr(not(feature = "cache"), allow(unused_imports))]
 
 use crate::{handle_result, wasm_memorytype_t, wasmtime_error_t};
+use std::mem::MaybeUninit;
 use std::ops::Range;
 use std::os::raw::c_char;
-use std::ptr;
 use std::{ffi::CStr, sync::Arc};
 use wasmtime::{
     Config, LinearMemory, MemoryCreator, OptLevel, ProfilingStrategy, Result, Strategy,
@@ -260,34 +260,33 @@ pub unsafe extern "C" fn wasmtime_config_cranelift_flag_set(
     c.config.cranelift_flag_set(flag, value);
 }
 
-pub type wasmtime_new_memory_callback_t = extern "C" fn(
-    ty: &wasm_memorytype_t,
-    minimum: usize,
-    maximum: usize,
-    reserved_size_in_bytes: usize,
-    guard_size_in_bytes: usize,
-    memory_ret: &mut *mut std::ffi::c_void,
-    finalizer_ret: &mut Option<extern "C" fn(arg1: *mut std::ffi::c_void)>,
-) -> Option<Box<wasmtime_error_t>>;
-
 pub type wasmtime_memory_get_callback_t = extern "C" fn(
-    memory_ptr: *mut std::ffi::c_void,
+    env: *mut std::ffi::c_void,
     byte_size: &mut usize,
     maximum_byte_size: &mut usize,
 ) -> *mut u8;
 
-pub type wasmtime_memory_grow_callback_t = extern "C" fn(
-    memory_ptr: *mut std::ffi::c_void,
-    new_size: usize,
-) -> Option<Box<wasmtime_error_t>>;
+pub type wasmtime_memory_grow_callback_t =
+    extern "C" fn(env: *mut std::ffi::c_void, new_size: usize) -> Option<Box<wasmtime_error_t>>;
 
 #[repr(C)]
-pub struct wasmtime_memory_creator_t {
-    new_memory: wasmtime_new_memory_callback_t,
+pub struct wasmtime_linear_memory_t {
+    env: *mut std::ffi::c_void,
     get_memory: wasmtime_memory_get_callback_t,
     grow_memory: wasmtime_memory_grow_callback_t,
+    finalizer: Option<extern "C" fn(arg1: *mut std::ffi::c_void)>,
 }
 
+pub type wasmtime_new_memory_callback_t = extern "C" fn(
+    env: *mut std::ffi::c_void,
+    ty: &wasm_memorytype_t,
+    minimum: usize,
+    maximum: usize,
+    reserved_size_in_bytes: usize,
+    guard_size_in_bytes: usize,
+    memory_ret: *mut wasmtime_linear_memory_t,
+) -> Option<Box<wasmtime_error_t>>;
+
 struct CHostLinearMemory {
     foreign: crate::ForeignData,
     get_memory: wasmtime_memory_get_callback_t,
@@ -340,8 +339,16 @@ unsafe impl LinearMemory for CHostLinearMemory {
     }
 }
 
+#[repr(C)]
+pub struct wasmtime_memory_creator_t {
+    env: *mut std::ffi::c_void,
+    new_memory: wasmtime_new_memory_callback_t,
+    finalizer: Option<extern "C" fn(arg1: *mut std::ffi::c_void)>,
+}
+
 struct CHostMemoryCreator {
-    creator: Box<wasmtime_memory_creator_t>,
+    foreign: crate::ForeignData,
+    new_memory: wasmtime_new_memory_callback_t,
 }
 unsafe impl Send for CHostMemoryCreator {}
 unsafe impl Sync for CHostMemoryCreator {}
@@ -355,28 +362,28 @@ unsafe impl MemoryCreator for CHostMemoryCreator {
         reserved_size_in_bytes: Option<usize>,
         guard_size_in_bytes: usize,
     ) -> Result<Box<dyn wasmtime::LinearMemory>, String> {
-        let mut memory = ptr::null_mut();
-        let mut finalizer = None;
-        let cb = self.creator.new_memory;
+        let mut memory = MaybeUninit::uninit();
+        let cb = self.new_memory;
         let error = cb(
+            self.foreign.data,
             &wasm_memorytype_t::new(ty),
             minimum,
             maximum.unwrap_or(usize::MAX),
             reserved_size_in_bytes.unwrap_or(0),
             guard_size_in_bytes,
-            &mut memory,
-            &mut finalizer,
+            memory.as_mut_ptr(),
         );
         match error {
             None => {
+                let memory = unsafe { memory.assume_init() };
                 let foreign = crate::ForeignData {
-                    data: memory,
-                    finalizer,
+                    data: memory.env,
+                    finalizer: memory.finalizer,
                 };
                 Ok(Box::new(CHostLinearMemory {
                     foreign,
-                    get_memory: self.creator.get_memory,
-                    grow_memory: self.creator.grow_memory,
+                    get_memory: memory.get_memory,
+                    grow_memory: memory.grow_memory,
                 }))
             }
             Some(err) => {
@@ -390,8 +397,13 @@ unsafe impl MemoryCreator for CHostMemoryCreator {
 #[no_mangle]
 pub unsafe extern "C" fn wasmtime_config_host_memory_creator_set(
     c: &mut wasm_config_t,
-    creator: Box<wasmtime_memory_creator_t>,
+    creator: &wasmtime_memory_creator_t,
 ) {
-    c.config
-        .with_host_memory(Arc::new(CHostMemoryCreator { creator }));
+    c.config.with_host_memory(Arc::new(CHostMemoryCreator {
+        foreign: crate::ForeignData {
+            data: creator.env,
+            finalizer: creator.finalizer,
+        },
+        new_memory: creator.new_memory,
+    }));
 }