N-API reference management leaks memory

[addaleax]

After making the node-api/test_worker_terminate_finalization complete run as expected in #34726, it starts failing under ASAN/valgrind because it leaks memory (the reference from napi_create_reference() in the test is never freed).

I’ve tried to debug this for an hour, but got nowhere, so I’ll open a PR to just skip the test (shouldn’t exactly make things worse since the test hasn’t been working until today anyway).

/cc The only people who understand N-API reference lifetime management, aka @gabrielschulhof and maybe @mhdawson

[gengjiawen]

Stacktrace

2020-08-11T23:46:40.5991523Z ##[error]--- stderr ---
=================================================================
==107347==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 80 byte(s) in 1 object(s) allocated from:
    #0 0xa1d80d in operator new(unsigned long) (/home/runner/work/node/node/out/Release/node+0xa1d80d)
    #1 0xb5bf63 in napi_create_reference (/home/runner/work/node/node/out/Release/node+0xb5bf63)
    #2 0x7fdf6fac9878  (<unknown module>)
    #3 0xb47492 in v8impl::(anonymous namespace)::FunctionCallbackWrapper::Invoke(v8::FunctionCallbackInfo<v8::Value> const&) (/home/runner/work/node/node/out/Release/node+0xb47492)
    #4 0x1268a73 in v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) (/home/runner/work/node/node/out/Release/node+0x1268a73)
    #5 0x12668a4 in v8::internal::MaybeHandle<v8::internal::Object> v8::internal::(anonymous namespace)::HandleApiCallHelper<false>(v8::internal::Isolate*, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) (/home/runner/work/node/node/out/Release/node+0x12668a4)
    #6 0x1264932 in v8::internal::Builtin_Impl_HandleApiCall(v8::internal::BuiltinArguments, v8::internal::Isolate*) (/home/runner/work/node/node/out/Release/node+0x1264932)
    #7 0x2aeb098 in Builtins_CEntry_Return1_DontSaveFPRegs_ArgvOnStack_BuiltinExit (/home/runner/work/node/node/out/Release/node+0x2aeb098)
    #8 0x2a84361 in Builtins_InterpreterEntryTrampoline (/home/runner/work/node/node/out/Release/node+0x2a84361)
    #9 0x2a84361 in Builtins_InterpreterEntryTrampoline (/home/runner/work/node/node/out/Release/node+0x2a84361)
    #10 0x2a84361 in Builtins_InterpreterEntryTrampoline (/home/runner/work/node/node/out/Release/node+0x2a84361)
    #11 0x2a84361 in Builtins_InterpreterEntryTrampoline (/home/runner/work/node/node/out/Release/node+0x2a84361)
    #12 0x2a84361 in Builtins_InterpreterEntryTrampoline (/home/runner/work/node/node/out/Release/node+0x2a84361)
    #13 0x2a84361 in Builtins_InterpreterEntryTrampoline (/home/runner/work/node/node/out/Release/node+0x2a84361)
    #14 0x2a84361 in Builtins_InterpreterEntryTrampoline (/home/runner/work/node/node/out/Release/node+0x2a84361)
    #15 0x2a84361 in Builtins_InterpreterEntryTrampoline (/home/runner/work/node/node/out/Release/node+0x2a84361)
    #16 0x2a84361 in Builtins_InterpreterEntryTrampoline (/home/runner/work/node/node/out/Release/node+0x2a84361)
    #17 0x2a82079 in Builtins_JSEntryTrampoline (/home/runner/work/node/node/out/Release/node+0x2a82079)
    #18 0x2a81e57 in Builtins_JSEntry (/home/runner/work/node/node/out/Release/node+0x2a81e57)
    #19 0x14f5d60 in v8::internal::(anonymous namespace)::Invoke(v8::internal::Isolate*, v8::internal::(anonymous namespace)::InvokeParams const&) (/home/runner/work/node/node/out/Release/node+0x14f5d60)
    #20 0x14f3b44 in v8::internal::Execution::Call(v8::internal::Isolate*, v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>, int, v8::internal::Handle<v8::internal::Object>*) (/home/runner/work/node/node/out/Release/node+0x14f3b44)
    #21 0x112c210 in v8::Function::Call(v8::Local<v8::Context>, v8::Local<v8::Value>, int, v8::Local<v8::Value>*) (/home/runner/work/node/node/out/Release/node+0x112c210)
    #22 0xa24b32 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) (/home/runner/work/node/node/out/Release/node+0xa24b32)
    #23 0xa6cc97 in node::AsyncWrap::MakeCallback(v8::Local<v8::Function>, int, v8::Local<v8::Value>*) (/home/runner/work/node/node/out/Release/node+0xa6cc97)
    #24 0xd107f4 in node::worker::MessagePort::OnMessage() (/home/runner/work/node/node/out/Release/node+0xd107f4)
    #25 0x2a3ea98 in uv__async_io /home/runner/work/node/node/out/../deps/uv/src/unix/async.c:163:5
    #26 0x2a723d2 in uv__io_poll /home/runner/work/node/node/out/../deps/uv/src/unix/linux-core.c:431:11
    #27 0x2a3faa7 in uv_run /home/runner/work/node/node/out/../deps/uv/src/unix/core.c:385:5
    #28 0xe937c5 in node::worker::Worker::Run() (/home/runner/work/node/node/out/Release/node+0xe937c5)
    #29 0xe9d770 in node::worker::Worker::StartThread(v8::FunctionCallbackInfo<v8::Value> const&)::$_3::__invoke(void*) (/home/runner/work/node/node/out/Release/node+0xe9d770

[gengjiawen]

Is there a way I can directly run this single test ? python3 tools/test.py --mode=debug test/node-api/test_worker_terminate_finalization/test.js looks like not working.

[richardlau]

@gengjiawen Did you build the addon (make build-node-api-tests) first? It's possible you'll need to build the addon manually for debug mode as I think the makefile assumes release in a couple of places.

[gengjiawen]

@gengjiawen Did you build the addon (make build-node-api-tests) first? It's possible you'll need to build the addon manually for debug mode as I think the makefile assumes release in a couple of places.

Thanks, this do work, although the script has force me to build a release build 😭

Debug stacktrace

=== debug test ===                   
Path: node-api/test_worker_terminate_finalization/test
=================================================================
==19561==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 80 byte(s) in 1 object(s) allocated from:
    #0 0xf0932d in operator new(unsigned long) (/root/node/out/Debug/node+0xf0932d)
    #1 0x1290ff8 in v8impl::(anonymous namespace)::Reference::New(napi_env__*, v8::Local<v8::Value>, unsigned int, bool, void (*)(napi_env__*, void*, void*), void*, void*) /root/node/out/Debug/../../src/js_native_api_v8.cc:313:12
    #2 0x12936fa in napi_create_reference /root/node/out/Debug/../../src/js_native_api_v8.cc:2462:7
    #3 0x7f86ceefdf72  (<unknown module>)
    #4 0x12a1612 in v8impl::(anonymous namespace)::CallbackWrapperBase<v8::FunctionCallbackInfo<v8::Value>, &(v8impl::(anonymous namespace)::CallbackBundle::function_or_getter)>::InvokeCallback()::'lambda'(napi_env__*)::operator()(napi_env__*) const /root/node/out/Debug/../../src/js_native_api_v8.cc:477:16
    #5 0x12a12bf in void napi_env__::CallIntoModule<v8impl::(anonymous namespace)::CallbackWrapperBase<v8::FunctionCallbackInfo<v8::Value>, &(v8impl::(anonymous namespace)::CallbackBundle::function_or_getter)>::InvokeCallback()::'lambda'(napi_env__*), void (napi_env__*, v8::Local<v8::Value>)>(v8::FunctionCallbackInfo<v8::Value>&&, &(v8impl::(anonymous namespace)::CallbackBundle::function_or_getter)&&) /root/node/out/Debug/../../src/js_native_api_v8.h:95:5
    #6 0x129ff67 in v8impl::(anonymous namespace)::CallbackWrapperBase<v8::FunctionCallbackInfo<v8::Value>, &(v8impl::(anonymous namespace)::CallbackBundle::function_or_getter)>::InvokeCallback() /root/node/out/Debug/../../src/js_native_api_v8.cc:476:10
    #7 0x12706c2 in v8impl::(anonymous namespace)::FunctionCallbackWrapper::Invoke(v8::FunctionCallbackInfo<v8::Value> const&) /root/node/out/Debug/../../src/js_native_api_v8.cc:495:15
    #8 0x1fb3db2 in v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) /root/node/out/Debug/../../deps/v8/src/api/api-arguments-inl.h:158:3
    #9 0x1fb071f in v8::internal::MaybeHandle<v8::internal::Object> v8::internal::(anonymous namespace)::HandleApiCallHelper<false>(v8::internal::Isolate*, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) /root/node/out/Debug/../../deps/v8/src/builtins/builtins-api.cc:111:36
    #10 0x1fad6fa in v8::internal::Builtin_Impl_HandleApiCall(v8::internal::BuiltinArguments, v8::internal::Isolate*) /root/node/out/Debug/../../deps/v8/src/builtins/builtins-api.cc:141:5
    #11 0x1fac950 in v8::internal::Builtin_HandleApiCall(int, unsigned long*, v8::internal::Isolate*) /root/node/out/Debug/../../deps/v8/src/builtins/builtins-api.cc:129:1
    #12 0x401a13f in Builtins_CEntry_Return1_DontSaveFPRegs_ArgvOnStack_BuiltinExit (/root/node/out/Debug/node+0x401a13f)
    #13 0x3e16be1 in Builtins_InterpreterEntryTrampoline (/root/node/out/Debug/node+0x3e16be1)
    #14 0x3e16be1 in Builtins_InterpreterEntryTrampoline (/root/node/out/Debug/node+0x3e16be1)
    #15 0x3e16be1 in Builtins_InterpreterEntryTrampoline (/root/node/out/Debug/node+0x3e16be1)
    #16 0x3e16be1 in Builtins_InterpreterEntryTrampoline (/root/node/out/Debug/node+0x3e16be1)
    #17 0x3e16be1 in Builtins_InterpreterEntryTrampoline (/root/node/out/Debug/node+0x3e16be1)
    #18 0x3e16be1 in Builtins_InterpreterEntryTrampoline (/root/node/out/Debug/node+0x3e16be1)
    #19 0x3e16be1 in Builtins_InterpreterEntryTrampoline (/root/node/out/Debug/node+0x3e16be1)
    #20 0x3e16be1 in Builtins_InterpreterEntryTrampoline (/root/node/out/Debug/node+0x3e16be1)
    #21 0x3e16be1 in Builtins_InterpreterEntryTrampoline (/root/node/out/Debug/node+0x3e16be1)
    #22 0x3e0d839 in Builtins_JSEntryTrampoline (/root/node/out/Debug/node+0x3e0d839)
    #23 0x3e0d617 in Builtins_JSEntry (/root/node/out/Debug/node+0x3e0d617)
    #24 0x23a0a4b in v8::internal::GeneratedCode<unsigned long, unsigned long, unsigned long, unsigned long, unsigned long, long, unsigned long**>::Call(unsigned long, unsigned long, unsigned long, unsigned long, long, unsigned long**) /root/node/out/Debug/../../deps/v8/src/execution/simulator.h:142:12
    #25 0x23a0a4b in v8::internal::(anonymous namespace)::Invoke(v8::internal::Isolate*, v8::internal::(anonymous namespace)::InvokeParams const&) /root/node/out/Debug/../../deps/v8/src/execution/execution.cc:367:33
    #26 0x239f6f2 in v8::internal::Execution::Call(v8::internal::Isolate*, v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>, int, v8::internal::Handle<v8::internal::Object>*) /root/node/out/Debug/../../deps/v8/src/execution/execution.cc:461:10
    #27 0x1da8c33 in v8::Function::Call(v8::Local<v8::Context>, v8::Local<v8::Value>, int, v8::Local<v8::Value>*) /root/node/out/Debug/../../deps/v8/src/api/api.cc:4876:7
    #28 0xff6503 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/node/out/Debug/../../src/api/callback.cc:191:21
    #29 0x109d902 in node::AsyncWrap::MakeCallback(v8::Local<v8::Function>, int, v8::Local<v8::Value>*) /root/node/out/Debug/../../src/async_wrap.cc:974:27
    #30 0x15dc8b7 in node::worker::MessagePort::OnMessage() /root/node/out/Debug/../../src/node_messaging.cc:772:9

SUMMARY: AddressSanitizer: 80 byte(s) leaked in 1 allocation(s).