Skip to content

ObjectWrap leak introduced in 8.2.0 #1602

Description

@arsnyder16

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;

Activity

  1. arsnyder16 commented on Oct 25, 2024

    @arsnyder16
    Author

    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.

  2. mhdawson commented on Oct 31, 2024

    @mhdawson
    Member

    @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?
  3. legendecas commented on Oct 31, 2024

    @legendecas
    Member

    @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.

  4. mhdawson commented on Nov 1, 2024

    @mhdawson
    Member

    @legendecas thanks for the quick answer and fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions