Address PR feedback

- Pass caching_callback directly to Finish instead of wrapping in a lambda
- Capture isolate pointer in the Rust closure instead of threading it
  through the C++ resolution callback
- Use a rule-of-five RustCallable class to track Box lifetime through
  std::function, preventing leaks if the callback is never invoked

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Author: bartlomieju

src/binding.cc

@@ -3808,34 +3808,72 @@ void v8__WasmModuleCompilation__OnBytesReceived(
   self->OnBytesReceived(bytes, size);
 }
 
+// A callable wrapper that properly tracks a Rust Box pointer lifetime through
+// std::function's copy/move/destroy operations (rule of five). This ensures
+// the boxed Rust closure is freed when the std::function is destroyed, even
+// if the callback is never invoked.
+class RustCallable {
+ public:
+  using Callback = void (*)(void* data, const v8::WasmModuleObject* module,
+                            const v8::Value* error);
+
+  RustCallable(Callback callback, void* data, void (*drop)(void*))
+      : callback_(callback), data_(data), drop_(drop), alive_(true) {}
+
+  ~RustCallable() {
+    if (alive_) drop_(data_);
+  }
+
+  RustCallable(const RustCallable& other)
+      : callback_(other.callback_),
+        data_(other.data_),
+        drop_(other.drop_),
+        alive_(false) {}
+
+  RustCallable(RustCallable&& other)
+      : callback_(other.callback_),
+        data_(other.data_),
+        drop_(other.drop_),
+        alive_(other.alive_) {
+    other.alive_ = false;
+  }
+
+  RustCallable& operator=(const RustCallable&) = delete;
+  RustCallable& operator=(RustCallable&&) = delete;
+
+  void operator()(
+      std::variant<v8::Local<v8::WasmModuleObject>, v8::Local<v8::Value>>
+          result) {
+    if (auto* module =
+            std::get_if<v8::Local<v8::WasmModuleObject>>(&result)) {
+      callback_(data_, local_to_ptr(*module), nullptr);
+    } else {
+      callback_(data_, nullptr,
+                local_to_ptr(std::get<v8::Local<v8::Value>>(result)));
+    }
+    // After invoking a FnOnce, the drop is handled by Rust inside the
+    // callback. Prevent double-free when the std::function is destroyed.
+    alive_ = false;
+  }
+
+ private:
+  Callback callback_;
+  void* data_;
+  void (*drop_)(void*);
+  bool alive_;
+};
+
 void v8__WasmModuleCompilation__Finish(
     v8::WasmModuleCompilation* self, v8::Isolate* isolate,
     void (*caching_callback)(v8::WasmStreaming::ModuleCachingInterface&),
-    void (*resolution_callback)(void* data, v8::Isolate* isolate,
+    void (*resolution_callback)(void* data,
                                 const v8::WasmModuleObject* module,
                                 const v8::Value* error),
-    void* resolution_data) {
-  v8::WasmModuleCompilation::ModuleCachingCallback cc;
-  if (caching_callback) {
-    cc = [caching_callback](v8::WasmStreaming::ModuleCachingInterface& mci) {
-      caching_callback(mci);
-    };
-  }
-  self->Finish(
-      isolate, cc,
-      [resolution_callback, resolution_data, isolate](
-          std::variant<v8::Local<v8::WasmModuleObject>, v8::Local<v8::Value>>
-              result) {
-        if (auto* module =
-                std::get_if<v8::Local<v8::WasmModuleObject>>(&result)) {
-          resolution_callback(resolution_data, isolate, local_to_ptr(*module),
-                              nullptr);
-        } else {
-          resolution_callback(
-              resolution_data, isolate, nullptr,
-              local_to_ptr(std::get<v8::Local<v8::Value>>(result)));
-        }
-      });
+    void* resolution_data,
+    void (*drop_resolution_data)(void* data)) {
+  self->Finish(isolate, caching_callback,
+               RustCallable(resolution_callback, resolution_data,
+                            drop_resolution_data));
 }
 
 void v8__WasmModuleCompilation__Abort(v8::WasmModuleCompilation* self) {

src/wasm.rs

@@ -333,15 +333,27 @@ impl WasmModuleCompilation {
         Result<Local<'_, WasmModuleObject>, Local<'_, Value>>,
       ) + 'static,
   ) {
+    // Capture the isolate pointer in the closure so it doesn't need to be
+    // threaded through C++.
+    let isolate_ptr = scope.get_isolate_ptr();
+    let wrapped = move |module: *const WasmModuleObject, error: *const Value| {
+      let isolate = unsafe { Isolate::from_raw_ptr(isolate_ptr) };
+      if !module.is_null() {
+        resolution_callback(
+          &isolate,
+          Ok(unsafe { Local::from_raw(module) }.unwrap()),
+        );
+      } else {
+        resolution_callback(
+          &isolate,
+          Err(unsafe { Local::from_raw(error) }.unwrap()),
+        );
+      }
+    };
+
     // Double-box: the outer Box gives us a thin pointer suitable for void*.
-    let boxed: Box<
-      Box<
-        dyn FnOnce(
-          &Isolate,
-          Result<Local<'_, WasmModuleObject>, Local<'_, Value>>,
-        ),
-      >,
-    > = Box::new(Box::new(resolution_callback));
+    let boxed: Box<Box<dyn FnOnce(*const WasmModuleObject, *const Value)>> =
+      Box::new(Box::new(wrapped));
     let data = Box::into_raw(boxed) as *mut c_void;
 
     unsafe {
@@ -351,6 +363,7 @@ impl WasmModuleCompilation {
         caching_callback,
         resolution_trampoline,
         data,
+        drop_resolution_data,
       );
     }
   }
@@ -424,30 +437,20 @@ impl Drop for WasmModuleCompilation {
 
 unsafe extern "C" fn resolution_trampoline(
   data: *mut c_void,
-  isolate: *mut RealIsolate,
   module: *const WasmModuleObject,
   error: *const Value,
 ) {
-  let callback: Box<
-    Box<
-      dyn FnOnce(
-        &Isolate,
-        Result<Local<'_, WasmModuleObject>, Local<'_, Value>>,
-      ),
-    >,
-  > = unsafe { Box::from_raw(data as *mut _) };
-  let isolate = unsafe { Isolate::from_raw_ptr(isolate) };
-  if !module.is_null() {
-    callback(
-      &isolate,
-      Ok(unsafe { Local::from_raw(module) }.unwrap()),
-    );
-  } else {
-    callback(
-      &isolate,
-      Err(unsafe { Local::from_raw(error) }.unwrap()),
-    );
-  }
+  let callback: Box<Box<dyn FnOnce(*const WasmModuleObject, *const Value)>> =
+    unsafe { Box::from_raw(data as *mut _) };
+  callback(module, error);
+}
+
+unsafe extern "C" fn drop_resolution_data(data: *mut c_void) {
+  let _ = unsafe {
+    Box::from_raw(
+      data as *mut Box<dyn FnOnce(*const WasmModuleObject, *const Value)>,
+    )
+  };
 }
 
 unsafe extern "C" fn serialization_trampoline(
@@ -587,11 +590,11 @@ unsafe extern "C" {
     caching_callback: Option<ModuleCachingCallback>,
     resolution_callback: unsafe extern "C" fn(
       *mut c_void,
-      *mut RealIsolate,
       *const WasmModuleObject,
       *const Value,
     ),
     resolution_data: *mut c_void,
+    drop_resolution_data: unsafe extern "C" fn(*mut c_void),
   );
   fn v8__WasmModuleCompilation__Abort(
     this: *mut InternalWasmModuleCompilation,