Repository: bytecodealliance/wasmtime
Pull request #8907 (2024-07-07 02:24:40)

Expose Engine::clone to c-api
This creates an API to allow cloning references to wasm_engine_t in the C API. This is simple in the Rust API which implements Clone, however it is not exposed to the C API.

Use case: in tree-sitter's wasm support, we create a store object and want to instantiate modules into it over the course of the object's lifetime. This requires a handle to the underlying engine, which means our store must be kept alongside a reference to the engine (since tree-sitter is a library, it doesn't want to assume a global wasm engine). This currently requires transferring ownership of the engine to the tree sitter store container, which means we can't reuse engine objects between multiple stores.

This pertains to https://github.com/tree-sitter/tree-sitter/issues/3454.


Patch:
diff --git a/crates/c-api/README.md b/crates/c-api/README.md
index 3c7361616..370be40e6 100644
--- a/crates/c-api/README.md
+++ b/crates/c-api/README.md
@@ -8,7 +8,7 @@ For more information you can find the documentation for this library
 To use Wasmtime from a C or C++ project, you can use Cargo to build the Wasmtime C bindings. From the root of the Wasmtime repository, run the following command:
 
 ```
-cargo build --release wasmtime-c-api
+cargo build --release -p wasmtime-c-api
 ```
 
 This will create static and dynamic libraries called `libwasmtime` in the `target/release` directory.
diff --git a/crates/c-api/include/wasmtime/engine.h b/crates/c-api/include/wasmtime/engine.h
index d68cf4e8f..6b666bf5f 100644
--- a/crates/c-api/include/wasmtime/engine.h
+++ b/crates/c-api/include/wasmtime/engine.h
@@ -13,6 +13,14 @@
 extern "C" {
 #endif
 
+/**
+ * \brief Create a new reference to the same underlying engine.
+ *
+ * This function clones the reference-counted pointer to the internal object,
+ * and must be freed using #wasm_engine_delete.
+ */
+WASM_API_EXTERN wasm_engine_t *wasmtime_engine_clone(wasm_engine_t *engine);
+
 /**
  * \brief Increments the engine-local epoch variable.
  *
diff --git a/crates/c-api/src/engine.rs b/crates/c-api/src/engine.rs
index 06a2b1b3f..e8cea19e7 100644
--- a/crates/c-api/src/engine.rs
+++ b/crates/c-api/src/engine.rs
@@ -29,12 +29,20 @@ pub extern "C" fn wasm_engine_new() -> Box<wasm_engine_t> {
 
 #[no_mangle]
 pub extern "C" fn wasm_engine_new_with_config(c: Box<wasm_config_t>) -> Box<wasm_engine_t> {
+    #[cfg(feature = "logging")]
+    drop(env_logger::try_init());
+
     let config = c.config;
     Box::new(wasm_engine_t {
         engine: Engine::new(&config).unwrap(),
     })
 }
 
+#[no_mangle]
+pub extern "C" fn wasmtime_engine_clone(engine: &wasm_engine_t) -> Box<wasm_engine_t> {
+    Box::new(engine.clone())
+}
+
 #[no_mangle]
 pub extern "C" fn wasmtime_engine_increment_epoch(engine: &wasm_engine_t) {
     engine.engine.increment_epoch();
diff --git a/crates/wasmtime/src/runtime/gc/disabled/rooting.rs b/crates/wasmtime/src/runtime/gc/disabled/rooting.rs
index a63a65126..7c3065265 100644
--- a/crates/wasmtime/src/runtime/gc/disabled/rooting.rs
+++ b/crates/wasmtime/src/runtime/gc/disabled/rooting.rs
@@ -45,7 +45,7 @@ impl RootSet {
         usize::MAX
     }
 
-    pub(crate) fn exit_lifo_scope(&mut self, _gc_store: &mut GcStore, _scope: usize) {}
+    pub(crate) fn exit_lifo_scope(&mut self, _gc_store: Option<&mut GcStore>, _scope: usize) {}
 
     pub(crate) fn with_lifo_scope<T>(
         store: &mut StoreOpaque,
diff --git a/crates/wasmtime/src/runtime/gc/enabled/rooting.rs b/crates/wasmtime/src/runtime/gc/enabled/rooting.rs
index 74fe5be87..632ee02f2 100644
--- a/crates/wasmtime/src/runtime/gc/enabled/rooting.rs
+++ b/crates/wasmtime/src/runtime/gc/enabled/rooting.rs
@@ -417,7 +417,7 @@ impl RootSet {
     ///
     /// Calls to `{enter,exit}_lifo_scope` must happen in a strict LIFO order.
     #[inline]
-    pub(crate) fn exit_lifo_scope(&mut self, gc_store: &mut GcStore, scope: usize) {
+    pub(crate) fn exit_lifo_scope(&mut self, gc_store: Option<&mut GcStore>, scope: usize) {
         debug_assert!(self.lifo_roots.len() >= scope);
 
         // If we actually have roots to unroot, call an out-of-line slow path.
@@ -428,7 +428,7 @@ impl RootSet {
 
     #[inline(never)]
     #[cold]
-    fn exit_lifo_scope_slow(&mut self, gc_store: &mut GcStore, scope: usize) {
+    fn exit_lifo_scope_slow(&mut self, mut gc_store: Option<&mut GcStore>, scope: usize) {
         self.lifo_generation += 1;
 
         // TODO: In the case where we have a tracing GC that doesn't need to
@@ -438,7 +438,13 @@ impl RootSet {
 
         let mut lifo_roots = mem::take(&mut self.lifo_roots);
         for root in lifo_roots.drain(scope..) {
-            gc_store.drop_gc_ref(root.gc_ref);
+            // Only drop the GC reference if we actually have a GC store. How
+            // can we have a GC reference but not a GC store? If we've only
+            // create `i31refs`, we never force a GC store's allocation. This is
+            // fine because `i31ref`s never need drop barriers.
+            if let Some(gc_store) = &mut gc_store {
+                gc_store.drop_gc_ref(root.gc_ref);
+            }
         }
         self.lifo_roots = lifo_roots;
     }
diff --git a/crates/wasmtime/src/runtime/store.rs b/crates/wasmtime/src/runtime/store.rs
index 16eb424d2..944cca644 100644
--- a/crates/wasmtime/src/runtime/store.rs
+++ b/crates/wasmtime/src/runtime/store.rs
@@ -1584,9 +1584,7 @@ impl StoreOpaque {
 
     #[inline]
     pub(crate) fn exit_gc_lifo_scope(&mut self, scope: usize) {
-        if let Some(gc_store) = self.gc_store.as_mut() {
-            self.gc_roots.exit_lifo_scope(gc_store, scope);
-        }
+        self.gc_roots.exit_lifo_scope(self.gc_store.as_mut(), scope);
     }
 
     #[cfg(feature = "gc")]


Test patch:
diff --git a/tests/all/i31ref.rs b/tests/all/i31ref.rs
new file mode 100644
index 000000000..de5308f24
--- /dev/null
+++ b/tests/all/i31ref.rs
@@ -0,0 +1,21 @@
+use wasmtime::*;
+
+#[test]
+fn always_pop_i31ref_lifo_roots() -> Result<()> {
+    let mut config = Config::new();
+    config.wasm_function_references(true);
+    config.wasm_gc(true);
+
+    let engine = Engine::new(&config)?;
+    let mut store = Store::new(&engine, ());
+
+    let anyref = {
+        let mut scope = RootScope::new(&mut store);
+        AnyRef::from_i31(&mut scope, I31::wrapping_u32(42))
+    };
+
+    // The anyref has left its rooting scope and been unrooted.
+    assert!(anyref.is_i31(&store).is_err());
+
+    Ok(())
+}
diff --git a/tests/all/main.rs b/tests/all/main.rs
index 85eb3ea84..b9f2c4b66 100644
--- a/tests/all/main.rs
+++ b/tests/all/main.rs
@@ -16,6 +16,7 @@ mod funcref;
 mod gc;
 mod globals;
 mod host_funcs;
+mod i31ref;
 mod iloop;
 mod import_calling_export;
 mod import_indexes;