Skip to content

Commit 87a0d87

Browse files
bartlomiejuclaude
andcommitted
fix: use shared_ptr for resolution callback lifetime in WasmModuleCompilation
Replace RustCallable class with shared_ptr<void> to properly handle std::function copy semantics in V8's Finish method. The previous approach caused heap-use-after-free because V8 copies the std::function internally, and the RustCallable copy/destroy semantics couldn't coordinate ownership of the raw pointer across copies. Now the Rust closure data is reference-counted through shared_ptr copies, and the Rust side uses Option<Box<dyn FnOnce>> so the trampoline can .take() the closure without freeing the outer allocation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent e3654e8 commit 87a0d87

File tree

2 files changed

+36
-61
lines changed

2 files changed

+36
-61
lines changed

src/binding.cc

Lines changed: 17 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -3809,69 +3809,31 @@ void v8__WasmModuleCompilation__OnBytesReceived(v8::WasmModuleCompilation* self,
38093809
self->OnBytesReceived(bytes, size);
38103810
}
38113811

3812-
// A callable wrapper that properly tracks a Rust Box pointer lifetime through
3813-
// std::function's copy/move/destroy operations (rule of five). This ensures
3814-
// the boxed Rust closure is freed when the std::function is destroyed, even
3815-
// if the callback is never invoked.
3816-
class RustCallable {
3817-
public:
3818-
using Callback = void (*)(void* data, const v8::WasmModuleObject* module,
3819-
const v8::Value* error);
3820-
3821-
RustCallable(Callback callback, void* data, void (*drop)(void*))
3822-
: callback_(callback), data_(data), drop_(drop), alive_(true) {}
3823-
3824-
~RustCallable() {
3825-
if (alive_) drop_(data_);
3826-
}
3827-
3828-
RustCallable(const RustCallable& other)
3829-
: callback_(other.callback_),
3830-
data_(other.data_),
3831-
drop_(other.drop_),
3832-
alive_(false) {}
3833-
3834-
RustCallable(RustCallable&& other)
3835-
: callback_(other.callback_),
3836-
data_(other.data_),
3837-
drop_(other.drop_),
3838-
alive_(other.alive_) {
3839-
other.alive_ = false;
3840-
}
3841-
3842-
RustCallable& operator=(const RustCallable&) = delete;
3843-
RustCallable& operator=(RustCallable&&) = delete;
3844-
3845-
void operator()(
3846-
std::variant<v8::Local<v8::WasmModuleObject>, v8::Local<v8::Value>>
3847-
result) {
3848-
if (auto* module = std::get_if<v8::Local<v8::WasmModuleObject>>(&result)) {
3849-
callback_(data_, local_to_ptr(*module), nullptr);
3850-
} else {
3851-
callback_(data_, nullptr,
3852-
local_to_ptr(std::get<v8::Local<v8::Value>>(result)));
3853-
}
3854-
// After invoking a FnOnce, the drop is handled by Rust inside the
3855-
// callback. Prevent double-free when the std::function is destroyed.
3856-
alive_ = false;
3857-
}
3858-
3859-
private:
3860-
Callback callback_;
3861-
void* data_;
3862-
void (*drop_)(void*);
3863-
bool alive_;
3864-
};
3865-
38663812
void v8__WasmModuleCompilation__Finish(
38673813
v8::WasmModuleCompilation* self, v8::Isolate* isolate,
38683814
void (*caching_callback)(v8::WasmStreaming::ModuleCachingInterface&),
38693815
void (*resolution_callback)(void* data, const v8::WasmModuleObject* module,
38703816
const v8::Value* error),
38713817
void* resolution_data, void (*drop_resolution_data)(void* data)) {
3818+
// Use shared_ptr to reference-count the Rust closure data through
3819+
// std::function's copy semantics. The custom deleter calls back into Rust
3820+
// to drop the boxed closure when the last copy is destroyed.
3821+
auto shared_data = std::shared_ptr<void>(
3822+
resolution_data,
3823+
[drop_resolution_data](void* p) { drop_resolution_data(p); });
38723824
self->Finish(
38733825
isolate, caching_callback,
3874-
RustCallable(resolution_callback, resolution_data, drop_resolution_data));
3826+
[resolution_callback, shared_data](auto result) {
3827+
if (auto* module =
3828+
std::get_if<v8::Local<v8::WasmModuleObject>>(&result)) {
3829+
resolution_callback(shared_data.get(), local_to_ptr(*module),
3830+
nullptr);
3831+
} else {
3832+
resolution_callback(
3833+
shared_data.get(), nullptr,
3834+
local_to_ptr(std::get<v8::Local<v8::Value>>(result)));
3835+
}
3836+
});
38753837
}
38763838

38773839
void v8__WasmModuleCompilation__Abort(v8::WasmModuleCompilation* self) {

src/wasm.rs

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -352,9 +352,13 @@ impl WasmModuleCompilation {
352352
}
353353
};
354354

355-
// Double-box: the outer Box gives us a thin pointer suitable for void*.
356-
let boxed: Box<Box<dyn FnOnce(*const WasmModuleObject, *const Value)>> =
357-
Box::new(Box::new(wrapped));
355+
// Double-box with Option: the outer Box gives us a thin pointer suitable
356+
// for void*. The Option allows the trampoline to .take() the closure
357+
// (FnOnce semantics) without freeing the outer allocation, which is
358+
// ref-counted by shared_ptr on the C++ side.
359+
let boxed: Box<
360+
Option<Box<dyn FnOnce(*const WasmModuleObject, *const Value)>>,
361+
> = Box::new(Some(Box::new(wrapped)));
358362
let data = Box::into_raw(boxed) as *mut c_void;
359363

360364
unsafe {
@@ -441,15 +445,24 @@ unsafe extern "C" fn resolution_trampoline(
441445
module: *const WasmModuleObject,
442446
error: *const Value,
443447
) {
444-
let callback: Box<Box<dyn FnOnce(*const WasmModuleObject, *const Value)>> =
445-
unsafe { Box::from_raw(data as *mut _) };
448+
// Take the closure out of the Option without freeing the outer Box.
449+
// The outer Box is ref-counted by shared_ptr on the C++ side and will
450+
// be freed via drop_resolution_data when the last copy is destroyed.
451+
let slot = unsafe {
452+
&mut *(data
453+
as *mut Option<Box<dyn FnOnce(*const WasmModuleObject, *const Value)>>)
454+
};
455+
let callback = slot.take().unwrap();
446456
callback(module, error);
447457
}
448458

449459
unsafe extern "C" fn drop_resolution_data(data: *mut c_void) {
450460
let _ = unsafe {
451461
Box::from_raw(
452-
data as *mut Box<dyn FnOnce(*const WasmModuleObject, *const Value)>,
462+
data
463+
as *mut Option<
464+
Box<dyn FnOnce(*const WasmModuleObject, *const Value)>,
465+
>,
453466
)
454467
};
455468
}

0 commit comments

Comments
 (0)