nodejs/node-addon-api

ObjectWrap leak introduced in 8.2.0

arsnyder16 opened this issue · 4 comments

arsnyder16 commented

When using ObjectWrap that does not override a finalizer a memory leak is occurring.

I traced this back to #1514

Which introduced the following code which seems to trigger the leak. Removing this set eliminated the leak, but its not clear to me how this affects a basic or extended finalizer.

// Prevent ~ObjectWrap from calling napi_remove_wrap
instance->_ref = nullptr;
arsnyder16 commented

Repro steps

CFLAGS=-fsanitize=address CXXFLAGS=-fsanitize=address npm run pretest
LD_PRELOAD=`clang -print-file-name=libclang_rt.asan-x86_64.so` NODE_API_BUILD_CONFIG=Release node --expose-gc test/objectwrap.js

you will see the very first leak reported comes from a ObjectWrap<> construction

Direct leak of 72 byte(s) in 1 object(s) allocated from:
    #0 0x7fdf194265fd in operator new(unsigned long) (/usr/lib/llvm-19/lib/clang/19/lib/linux/libclang_rt.asan-x86_64.so+0x10e5fd) (BuildId: 897c70dd2d121c287b9879aab2db7e64324572a9)
    #1 0x000000c5dd9e in v8impl::ReferenceWithFinalizer::New(napi_env__*, v8::Local<v8::Value>, unsigned int, v8impl::ReferenceOwnership, void (*)(napi_env__*, void*, void*), void*, void*) (/root/.nvm/versions/node/v20.18.0/bin/node+0xc5dd9e) (BuildId: fbe09eeb702823fc4c9cc18fd5d44c32225e2ab6)
    #2 0x000000c6632f in napi_wrap (/root/.nvm/versions/node/v20.18.0/bin/node+0xc6632f) (BuildId: fbe09eeb702823fc4c9cc18fd5d44c32225e2ab6)
    #3 0x7fdeeaaa946c in Napi::cstm::ObjectWrap<Test>::ObjectWrap(Napi::cstm::CallbackInfo const&) (/root/node-addon-api/test/build/Release/binding_custom_namespace.node+0x15646c) (BuildId: 68f9f6c62e70d2753bc17857e6be2f35924283c2)
    #4 0x7fdeeaaa84bb in Test::Test(Napi::cstm::CallbackInfo const&) (/root/node-addon-api/test/build/Release/binding_custom_namespace.node+0x1554bb) (BuildId: 68f9f6c62e70d2753bc17857e6be2f35924283c2)
    #5 0x7fdeeaaa800f in Napi::cstm::ObjectWrap<Test>::ConstructorCallbackWrapper(napi_env__*, napi_callback_info__*)::'lambda0'()::operator()() const (/root/node-addon-api/test/build/Release/binding_custom_namespace.node+0x15500f) (BuildId: 68f9f6c62e70d2753bc17857e6be2f35924283c2)
    #6 0x7fdeeaaa7511 in Napi::cstm::ObjectWrap<Test>::ConstructorCallbackWrapper(napi_env__*, napi_callback_info__*) (/root/node-addon-api/test/build/Release/binding_custom_namespace.node+0x154511) (BuildId: 68f9f6c62e70d2753bc17857e6be2f35924283c2)
    #7 0x000000c5c524 in v8impl::(anonymous namespace)::FunctionCallbackWrapper::Invoke(v8::FunctionCallbackInfo<v8::Value> const&) js_native_api_v8.cc
    #8 0x000000f6e88e in v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) (/root/.nvm/versions/node/v20.18.0/bin/node+0xf6e88e) (BuildId: fbe09eeb702823fc4c9cc18fd5d44c32225e2ab6)
    #9 0x000000f6ee44 in v8::internal::MaybeHandle<v8::internal::Object> v8::internal::(anonymous namespace)::HandleApiCallHelper<true>(v8::internal::Isolate*, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, unsigned long*, int) builtins-api.cc
    #10 0x000000f6f562 in v8::internal::Builtin_HandleApiCall(int, unsigned long*, v8::internal::Isolate*) (/root/.nvm/versions/node/v20.18.0/bin/node+0xf6f562) (BuildId: fbe09eeb702823fc4c9cc18fd5d44c32225e2ab6)
    #11 0x000001979df5 in Builtins_CEntry_Return1_ArgvOnStack_BuiltinExit /home/iojs/build/ws/out/Release/obj.target/v8_snapshot/geni/embedded.o
    #12 0x0000018e958e in Builtins_JSBuiltinsConstructStub /home/iojs/build/ws/out/Release/obj.target/v8_snapshot/geni/embedded.o
    #13 0x000001a35c01 in Builtins_ConstructHandler /home/iojs/build/ws/out/Release/obj.target/v8_snapshot/geni/embedded.o
    #14 0x0000018ebd1b in Builtins_InterpreterEntryTrampoline /home/iojs/build/ws/out/Release/obj.target/v8_snapshot/geni/embedded.o
    #15 0x0000018ebd1b in Builtins_InterpreterEntryTrampoline /home/iojs/build/ws/out/Release/obj.target/v8_snapshot/geni/embedded.o
    #16 0x000001923d42 in Builtins_AsyncFunctionAwaitResolveClosure /home/iojs/build/ws/out/Release/obj.target/v8_snapshot/geni/embedded.o
    #17 0x0000019dbbf0 in Builtins_PromiseFulfillReactionJob /home/iojs/build/ws/out/Release/obj.target/v8_snapshot/geni/embedded.o
    #18 0x000001913a33 in Builtins_RunMicrotasks /home/iojs/build/ws/out/Release/obj.target/v8_snapshot/geni/embedded.o
    #19 0x0000018ea002 in Builtins_JSRunMicrotasksEntry /home/iojs/build/ws/out/Release/obj.target/v8_snapshot/geni/embedded.o
    #20 0x000001066f9c in v8::internal::(anonymous namespace)::Invoke(v8::internal::Isolate*, v8::internal::(anonymous namespace)::InvokeParams const&) execution.cc
    #21 0x0000010681ce in v8::internal::Execution::TryRunMicrotasks(v8::internal::Isolate*, v8::internal::MicrotaskQueue*) (/root/.nvm/versions/node/v20.18.0/bin/node+0x10681ce) (BuildId: fbe09eeb702823fc4c9cc18fd5d44c32225e2ab6)
    #22 0x00000109bb45 in v8::internal::MicrotaskQueue::RunMicrotasks(v8::internal::Isolate*) (/root/.nvm/versions/node/v20.18.0/bin/node+0x109bb45) (BuildId: fbe09eeb702823fc4c9cc18fd5d44c32225e2ab6)
    #23 0x00000109bedc in v8::internal::MicrotaskQueue::PerformCheckpoint(v8::Isolate*) (/root/.nvm/versions/node/v20.18.0/bin/node+0x109bedc) (BuildId: fbe09eeb702823fc4c9cc18fd5d44c32225e2ab6)
    #24 0x000000bd2c1a in node::InternalCallbackScope::Close() (/root/.nvm/versions/node/v20.18.0/bin/node+0xbd2c1a) (BuildId: fbe09eeb702823fc4c9cc18fd5d44c32225e2ab6)
    #25 0x000000bd312a in node::InternalMakeCallback(node::Environment*, v8::Local<v8::Object>, v8::Local<v8::Object>, v8::Local<v8::Function>, int, v8::Local<v8::Value>*, node::async_context) (/root/.nvm/versions/node/v20.18.0/bin/node+0xbd312a) (BuildId: fbe09eeb702823fc4c9cc18fd5d44c32225e2ab6)
    #26 0x000000bd325d in node::MakeCallback(v8::Isolate*, v8::Local<v8::Object>, v8::Local<v8::Function>, int, v8::Local<v8::Value>*, node::async_context) (/root/.nvm/versions/node/v20.18.0/bin/node+0xbd325d) (BuildId: fbe09eeb702823fc4c9cc18fd5d44c32225e2ab6)
    #27 0x000000c363a1 in node::Environment::CheckImmediate(uv_check_s*) (/root/.nvm/versions/node/v20.18.0/bin/node+0xc363a1) (BuildId: fbe09eeb702823fc4c9cc18fd5d44c32225e2ab6)
    #28 0x0000018d0028 in uv__run_check /home/iojs/build/ws/out/../deps/uv/src/unix/loop-watcher.c:67:1
    #29 0x0000018c864b in uv_run /home/iojs/build/ws/out/../deps/uv/src/unix/core.c:461:5

If you run this same sequence in on the tag node-addon-api-v8.1.0 the leak won't exist.

mhdawson commented

@legendecas I see you have a PR which is related. A couple of questions:

  1. Does that fix this issue, or will additional changes be needed in node-addon-api, I'm assuming the later
  2. Is there a short term work around that people can use in their code, or possibly a change we could add to node-addon-api that would fix it transparently as the change in node itself works its way through to the LTS versions?
legendecas commented

@mhdawson #1607 should be sufficient to fix the issue, without any changes in Node.js core.

nodejs/node#55620 allows a Napi::Reference to reclmain napi_reference memory earlier, without a post_finalizer call.

mhdawson commented

@legendecas thanks for the quick answer and fix!