From 9391196eeffa79e175fc1cb92834a450d1fdcbe0 Mon Sep 17 00:00:00 2001 From: Louis Date: Tue, 20 Feb 2024 13:36:21 +0100 Subject: [PATCH] fix(napi): prevent memory leak when Custom GC is used (#1963) There is a piece of custom logic that has been added a while back to ensure that Buffers can be sent across threads, and be dropped properly. This involves a custom GC that runs on NodeJS's current thread (per my understanding). The logic to drop the buffer on that custom GC differed from the one in the Drop impl. This meant that everytime Node sent a buffer back to a napi-rs function, the reference wouldn't be cleaned up properly, and it would leak (96 bytes per Reference on an ARM MacOS machine). This commit updates the logic in the custom GC so that it matches the one in the Drop impl. This worked locally, and fixed any occurence of the leak I could find. --- crates/napi/src/bindgen_runtime/module_register.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/crates/napi/src/bindgen_runtime/module_register.rs b/crates/napi/src/bindgen_runtime/module_register.rs index d3bcffeb..c7a1ece2 100644 --- a/crates/napi/src/bindgen_runtime/module_register.rs +++ b/crates/napi/src/bindgen_runtime/module_register.rs @@ -619,10 +619,19 @@ extern "C" fn custom_gc( if THREADS_CAN_ACCESS_ENV.borrow_mut(|m| m.get(&std::thread::current().id()) == Some(&false)) { return; } - let mut reference = 0; + let mut ref_count = 0; check_status_or_throw!( env, - unsafe { sys::napi_reference_unref(env, data.cast(), &mut reference) }, + unsafe { sys::napi_reference_unref(env, data.cast(), &mut ref_count) }, + "Failed to unref Buffer reference in Custom GC" + ); + debug_assert!( + ref_count == 0, + "Buffer reference count in Custom GC is not 0" + ); + check_status_or_throw!( + env, + unsafe { sys::napi_delete_reference(env, data.cast()) }, "Failed to delete Buffer reference in Custom GC" ); }