node icon indicating copy to clipboard operation
node copied to clipboard

node-api: generalize finalizer second pass callback

Open legendecas opened this issue 3 years ago • 5 comments

Generalize the finalizer's second pass callback to make it cancellable and simplify the code around the second pass callback.

With this change, it is determined that Reference::Finalize/RefBase::Finalize are called once, either from the env's shutdown, or from the env's second pass callback.

All existing node-api js tests should pass without a touch. The js_native_api cctest is no longer applicable with this change, just removing it.

Refs: https://github.com/nodejs/node/issues/44071

legendecas avatar Aug 05 '22 07:08 legendecas

Review requested:

  • [ ] @nodejs/gyp
  • [ ] @nodejs/node-api

nodejs-github-bot avatar Aug 05 '22 07:08 nodejs-github-bot

CI: https://ci.nodejs.org/job/node-test-pull-request/45853/

nodejs-github-bot avatar Aug 05 '22 07:08 nodejs-github-bot

CI: https://ci.nodejs.org/job/node-test-pull-request/45890/

nodejs-github-bot avatar Aug 07 '22 06:08 nodejs-github-bot

CI: https://ci.nodejs.org/job/node-test-pull-request/45940/

Jenkins failure: https://ci.nodejs.org/job/node-test-commit-freebsd/45349/nodes=freebsd12-x64/

nodejs-github-bot avatar Aug 09 '22 03:08 nodejs-github-bot

CI: https://ci.nodejs.org/job/node-test-pull-request/45952/

nodejs-github-bot avatar Aug 09 '22 16:08 nodejs-github-bot

We're missing a test case that napi_remove_wrap a wrapped value, while simultaneously deleting the in-out param ref of napi_wrap with napi_delete_reference. I'll submit a PR for that.

legendecas avatar Sep 02 '22 14:09 legendecas

I started looking at this today and figure one thing I should do is run the node-addon-api tests, and in particular run them a lot of times to see if there were any flaky failures.

This seems to fail consistently:

[midawson@drx-hemera node-addon-api]$ node test/objectwrap_constructor_exception.js
Segmentation fault (core dumped)
[midawson@drx-hemera node-addon-api]$ gdb --args node test/objectwrap_constructor_exception.js
GNU gdb (GDB) Red Hat Enterprise Linux 8.2-16.el8
Copyright (C) 2018 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-redhat-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
    <http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from node...done.
(gdb) run
Starting program: /home/midawson/refs/node/node test/objectwrap_constructor_exception.js
Missing separate debuginfos, use: yum debuginfo-install glibc-2.28-164.el8.x86_64
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
[New Thread 0x7ffff6cb7700 (LWP 3223040)]
[New Thread 0x7ffff64b6700 (LWP 3223041)]
[New Thread 0x7ffff5cb5700 (LWP 3223042)]
[New Thread 0x7ffff54b4700 (LWP 3223043)]
[New Thread 0x7ffff4cb3700 (LWP 3223044)]
[New Thread 0x7ffff7ee7700 (LWP 3223045)]

Thread 1 "node" received signal SIGSEGV, Segmentation fault.
0x0000000000b168be in napi_delete_reference ()
Missing separate debuginfos, use: yum debuginfo-install libgcc-8.5.0-4.el8_5.x86_64 libstdc++-8.5.0-4.el8_5.x86_64
(gdb) bt
#0  0x0000000000b168be in napi_delete_reference ()
#1  0x00007ffff4103239 in Napi::ObjectWrap<ConstructorExceptionTest>::~ObjectWrap() ()
   from /home/midawson/refs/node-addon-api/test/build/Release/binding.node
#2  0x00007ffff410506e in Napi::ObjectWrap<ConstructorExceptionTest>::ConstructorCallbackWrapper(napi_env__*, napi_callback_info__*) ()
   from /home/midawson/refs/node-addon-api/test/build/Release/binding.node
#3  0x0000000000b0d8f9 in v8impl::(anonymous namespace)::FunctionCallbackWrapper::Invoke(v8::FunctionCallbackInfo<v8::Value> const&) ()
#4  0x0000000000dab1b0 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::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) ()
#5  0x0000000000dab7af in v8::internal::Builtin_HandleApiCall(int, unsigned long*, v8::internal::Isolate*) ()
#6  0x00000000016edd79 in Builtins_CEntry_Return1_DontSaveFPRegs_ArgvOnStack_BuiltinExit ()
#7  0x000000000166f7ec in Builtins_JSBuiltinsConstructStub ()
#8  0x00001b97e83425a9 in ?? ()
#9  0x00001b97e83425a9 in ?? ()
#10 0x0000000500000000 in ?? ()
#11 0x0000183e84d41689 in ?? ()
#12 0x00000e09efc637d1 in ?? ()
#13 0x0000000100000000 in ?? ()
#14 0x00000e09efc636b9 in ?? ()
#15 0x0000000000000028 in ?? ()
#16 0x00007fffffffd020 in ?? ()
#17 0x000000000178bbff in Builtins_ConstructHandler ()
Backtrace stopped: frame did not save the PC
(gdb) 

It might be related to how I built it, but all the regular node.js tests passed.

mhdawson avatar Sep 26 '22 19:09 mhdawson

objectwrap_remove_wrap.js also seems to crash

[midawson@drx-hemera node-addon-api]$ gdb --args node test/objectwrap_removewrap.js GNU gdb (GDB) Red Hat Enterprise Linux 8.2-16.el8 Copyright (C) 2018 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later http://gnu.org/licenses/gpl.html This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Type "show copying" and "show warranty" for details. This GDB was configured as "x86_64-redhat-linux-gnu". Type "show configuration" for configuration details. For bug reporting instructions, please see: http://www.gnu.org/software/gdb/bugs/. Find the GDB manual and other documentation resources online at: http://www.gnu.org/software/gdb/documentation/.

For help, type "help". Type "apropos word" to search for commands related to "word"... Reading symbols from node...done. (gdb) run Starting program: /home/midawson/refs/node/node test/objectwrap_removewrap.js Missing separate debuginfos, use: yum debuginfo-install glibc-2.28-164.el8.x86_64 [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib64/libthread_db.so.1". [New Thread 0x7ffff6cb7700 (LWP 3227912)] [New Thread 0x7ffff64b6700 (LWP 3227913)] [New Thread 0x7ffff5cb5700 (LWP 3227914)] [New Thread 0x7ffff54b4700 (LWP 3227915)] [New Thread 0x7ffff4cb3700 (LWP 3227916)] [New Thread 0x7ffff7ee7700 (LWP 3227917)]

Thread 1 "node" received signal SIGSEGV, Segmentation fault. 0x0000000000b168be in napi_delete_reference () Missing separate debuginfos, use: yum debuginfo-install libgcc-8.5.0-4.el8_5.x86_64 libstdc++-8.5.0-4.el8_5.x86_64 (gdb)

mhdawson avatar Sep 26 '22 19:09 mhdawson

All of the other node-addon-api tests seem to pass, at least in a run or two. Will recompile with debug to see if that gives any more info on the crashes.

mhdawson avatar Sep 26 '22 19:09 mhdawson

For /home/midawson/refs/node/node test/objectwrap_removewrap.js

Thread 1 "node" received signal SIGSEGV, Segmentation fault.
0x0000000001148d6a in napi_delete_reference (env=0x7ffff7ecc8d0, ref=0x7ffff7e534d0) at ../src/js_native_api_v8.cc:2423
2423	  delete reinterpret_cast<v8impl::Reference*>(ref);
Missing separate debuginfos, use: yum debuginfo-install libgcc-8.5.0-4.el8_5.x86_64 libstdc++-8.5.0-4.el8_5.x86_64
(gdb) bt
#0  0x0000000001148d6a in napi_delete_reference (env=0x7ffff7ecc8d0, ref=0x7ffff7e534d0) at ../src/js_native_api_v8.cc:2423
#1  0x00007ffff416f0a9 in Napi::ObjectWrap<(anonymous namespace)::Test>::~ObjectWrap() ()
   from /home/midawson/refs/node-addon-api/test/build/Debug/binding.node
#2  0x00007ffff40e8ac5 in Napi::ObjectWrap<(anonymous namespace)::Test>::ConstructorCallbackWrapper(napi_env__*, napi_callback_info__*) [clone .cold.114]
    () from /home/midawson/refs/node-addon-api/test/build/Debug/binding.node
#3  0x000000000113edba in v8impl::(anonymous namespace)::CallbackWrapperBase::<lambda(napi_env)>::operator()(napi_env) const (__closure=0x7fffffffcb30, 
    env=0x7ffff7ecc8d0) at ../src/js_native_api_v8.cc:308
#4  0x000000000114c291 in napi_env__::CallIntoModule<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::<lambda(napi_env)>, v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::<lambda(napi_env, v8::Local<v8::Value>)> >(v8impl::(anonymous namespace)::CallbackWrapperBase::<lambda(napi_env)> &&, v8impl::(anonymous namespace)::CallbackWrapperBase::<lambda(napi_env, v8::Local<v8::Value>)> &&) (this=0x7ffff7ecc8d0, call=..., 
    handle_exception=...) at ../src/js_native_api_v8.h:83
#5  0x000000000113ee74 in v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback (this=0x7fffffffcb70) at ../src/js_native_api_v8.cc:308
#6  0x000000000113eed4 in v8impl::(anonymous namespace)::FunctionCallbackWrapper::Invoke (info=...) at ../src/js_native_api_v8.cc:327
#7  0x000000000151be59 in v8::internal::FunctionCallbackArguments::Call (this=this@entry=0x7fffffffccc0, handler=handler@entry=...)
    at ../deps/v8/src/api/api-arguments-inl.h:147
#8  0x000000000151cdb5 in v8::internal::(anonymous namespace)::HandleApiCallHelper<true> (isolate=isolate@entry=0x65efdc0, function=..., 
    function@entry=..., new_target=new_target@entry=..., fun_data=..., receiver=..., receiver@entry=..., args=...)
    at ../deps/v8/src/objects/heap-object.h:173
#9  0x000000000151e9e5 in v8::internal::Builtin_Impl_HandleApiCall (isolate=0x65efdc0, args=...) at ../deps/v8/src/handles/handles.h:133
#10 v8::internal::Builtin_HandleApiCall (args_length=<optimized out>, args_object=0x7fffffffce30, isolate=0x65efdc0)
    at ../deps/v8/src/builtins/builtins-api.cc:130
#11 0x000000000228f239 in Builtins_CEntry_Return1_DontSaveFPRegs_ArgvOnStack_BuiltinExit () at ../../deps/v8/src/builtins/torque-internal.tq:101
#12 0x000000000220106c in Builtins_JSBuiltinsConstructStub () at ../../deps/v8/src/builtins/torque-internal.tq:101

mhdawson avatar Sep 26 '22 20:09 mhdawson

And valgrind report

[midawson@drx-hemera node-addon-api]$ valgrind node test/objectwrap_removewrap.js
==3486371== Memcheck, a memory error detector
==3486371== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==3486371== Using Valgrind-3.17.0 and LibVEX; rerun with -h for copyright info
==3486371== Command: node test/objectwrap_removewrap.js
==3486371== 
==3486371== Invalid read of size 8
==3486371==    at 0x1148D63: napi_delete_reference (js_native_api_v8.cc:2423)
==3486371==    by 0x142A30A8: Napi::ObjectWrap<(anonymous namespace)::Test>::~ObjectWrap() (in /home/midawson/refs/node-addon-api/test/build/Debug/binding.node)
==3486371==    by 0x1421CAC4: Napi::ObjectWrap<(anonymous namespace)::Test>::ConstructorCallbackWrapper(napi_env__*, napi_callback_info__*) [clone .cold.114] (in /home/midawson/refs/node-addon-api/test/build/Debug/binding.node)
==3486371==    by 0x113EDB9: v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::operator()(napi_env__*) const (js_native_api_v8.cc:308)
==3486371==    by 0x114C290: void napi_env__::CallIntoModule<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}, v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*, v8::Local<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::Value>)#2}>(v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}&&, v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*, v8::Local<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::Value>)#2}&&) (js_native_api_v8.h:83)
==3486371==    by 0x113EE73: v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback() (js_native_api_v8.cc:308)
==3486371==    by 0x113EED3: v8impl::(anonymous namespace)::FunctionCallbackWrapper::Invoke(v8::FunctionCallbackInfo<v8::Value> const&) (js_native_api_v8.cc:327)
==3486371==    by 0x151BE58: v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) (api-arguments-inl.h:147)
==3486371==    by 0x151CDB4: 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::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) (builtins-api.cc:112)
==3486371==    by 0x151E9E4: Builtin_Impl_HandleApiCall (builtins-api.cc:138)
==3486371==    by 0x151E9E4: v8::internal::Builtin_HandleApiCall(int, unsigned long*, v8::internal::Isolate*) (builtins-api.cc:130)
==3486371==    by 0x228F238: Builtins_CEntry_Return1_DontSaveFPRegs_ArgvOnStack_BuiltinExit (torque-internal.tq:101)
==3486371==    by 0x220106B: Builtins_JSBuiltinsConstructStub (torque-internal.tq:101)
==3486371==  Address 0x1402aa90 is 0 bytes inside a block of size 80 free'd
==3486371==    at 0x6EBE209: operator delete(void*, unsigned long) (vg_replace_malloc.c:814)
==3486371==    by 0x113F5B6: v8impl::Reference::~Reference() (js_native_api_v8.cc:552)
==3486371==    by 0x113EAF7: v8impl::(anonymous namespace)::Unwrap(napi_env__*, napi_value__*, void**, v8impl::(anonymous namespace)::UnwrapAction) (js_native_api_v8.cc:221)
==3486371==    by 0x1147FFB: napi_remove_wrap (js_native_api_v8.cc:2282)
==3486371==    by 0x142A30CA: Napi::ObjectWrap<(anonymous namespace)::Test>::~ObjectWrap() (in /home/midawson/refs/node-addon-api/test/build/Debug/binding.node)
==3486371==    by 0x1421CAC4: Napi::ObjectWrap<(anonymous namespace)::Test>::ConstructorCallbackWrapper(napi_env__*, napi_callback_info__*) [clone .cold.114] (in /home/midawson/refs/node-addon-api/test/build/Debug/binding.node)
==3486371==    by 0x113EDB9: v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::operator()(napi_env__*) const (js_native_api_v8.cc:308)
==3486371==    by 0x114C290: void napi_env__::CallIntoModule<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}, v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*, v8::Local<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::Value>)#2}>(v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}&&, v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*, v8::Local<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::Value>)#2}&&) (js_native_api_v8.h:83)
==3486371==    by 0x113EE73: v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback() (js_native_api_v8.cc:308)
==3486371==    by 0x113EED3: v8impl::(anonymous namespace)::FunctionCallbackWrapper::Invoke(v8::FunctionCallbackInfo<v8::Value> const&) (js_native_api_v8.cc:327)
==3486371==    by 0x151BE58: v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) (api-arguments-inl.h:147)
==3486371==    by 0x151CDB4: 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::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) (builtins-api.cc:112)
==3486371==  Block was alloc'd at
==3486371==    at 0x6EBB833: operator new(unsigned long) (vg_replace_malloc.c:417)
==3486371==    by 0x113F5F2: v8impl::Reference::New(napi_env__*, v8::Local<v8::Value>, unsigned int, bool, void (*)(napi_env__*, void*, void*), void*, void*) (js_native_api_v8.cc:567)
==3486371==    by 0x114C5DA: napi_status v8impl::(anonymous namespace)::Wrap<(v8impl::(anonymous namespace)::WrapType)0>(napi_env__*, napi_value__*, void*, void (*)(napi_env__*, void*, void*), void*, napi_ref__**) (js_native_api_v8.cc:432)
==3486371==    by 0x1147F9D: napi_wrap (js_native_api_v8.cc:2269)
==3486371==    by 0x142A4251: Napi::ObjectWrap<(anonymous namespace)::Test>::ConstructorCallbackWrapper(napi_env__*, napi_callback_info__*) (in /home/midawson/refs/node-addon-api/test/build/Debug/binding.node)
==3486371==    by 0x113EDB9: v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::operator()(napi_env__*) const (js_native_api_v8.cc:308)
==3486371==    by 0x114C290: void napi_env__::CallIntoModule<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}, v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*, v8::Local<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::Value>)#2}>(v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}&&, v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*, v8::Local<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::Value>)#2}&&) (js_native_api_v8.h:83)
==3486371==    by 0x113EE73: v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback() (js_native_api_v8.cc:308)
==3486371==    by 0x113EED3: v8impl::(anonymous namespace)::FunctionCallbackWrapper::Invoke(v8::FunctionCallbackInfo<v8::Value> const&) (js_native_api_v8.cc:327)
==3486371==    by 0x151BE58: v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) (api-arguments-inl.h:147)
==3486371==    by 0x151CDB4: 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::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) (builtins-api.cc:112)
==3486371==    by 0x151E9E4: Builtin_Impl_HandleApiCall (builtins-api.cc:138)
==3486371==    by 0x151E9E4: v8::internal::Builtin_HandleApiCall(int, unsigned long*, v8::internal::Isolate*) (builtins-api.cc:130)
==3486371== 
==3486371== Invalid write of size 8
==3486371==    at 0x114CFE9: v8impl::Finalizer::~Finalizer() (js_native_api_v8.h:305)
==3486371==    by 0x114D007: v8impl::Finalizer::~Finalizer() (js_native_api_v8.h:305)
==3486371==    by 0x1148D75: napi_delete_reference (js_native_api_v8.cc:2423)
==3486371==    by 0x142A30A8: Napi::ObjectWrap<(anonymous namespace)::Test>::~ObjectWrap() (in /home/midawson/refs/node-addon-api/test/build/Debug/binding.node)
==3486371==    by 0x1421CAC4: Napi::ObjectWrap<(anonymous namespace)::Test>::ConstructorCallbackWrapper(napi_env__*, napi_callback_info__*) [clone .cold.114] (in /home/midawson/refs/node-addon-api/test/build/Debug/binding.node)
==3486371==    by 0x113EDB9: v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::operator()(napi_env__*) const (js_native_api_v8.cc:308)
==3486371==    by 0x114C290: void napi_env__::CallIntoModule<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}, v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*, v8::Local<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::Value>)#2}>(v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}&&, v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*, v8::Local<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::Value>)#2}&&) (js_native_api_v8.h:83)
==3486371==    by 0x113EE73: v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback() (js_native_api_v8.cc:308)
==3486371==    by 0x113EED3: v8impl::(anonymous namespace)::FunctionCallbackWrapper::Invoke(v8::FunctionCallbackInfo<v8::Value> const&) (js_native_api_v8.cc:327)
==3486371==    by 0x151BE58: v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) (api-arguments-inl.h:147)
==3486371==    by 0x151CDB4: 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::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) (builtins-api.cc:112)
==3486371==    by 0x151E9E4: Builtin_Impl_HandleApiCall (builtins-api.cc:138)
==3486371==    by 0x151E9E4: v8::internal::Builtin_HandleApiCall(int, unsigned long*, v8::internal::Isolate*) (builtins-api.cc:130)
==3486371==  Address 0x1402aa90 is 0 bytes inside a block of size 80 free'd
==3486371==    at 0x6EBE209: operator delete(void*, unsigned long) (vg_replace_malloc.c:814)
==3486371==    by 0x113F5B6: v8impl::Reference::~Reference() (js_native_api_v8.cc:552)
==3486371==    by 0x113EAF7: v8impl::(anonymous namespace)::Unwrap(napi_env__*, napi_value__*, void**, v8impl::(anonymous namespace)::UnwrapAction) (js_native_api_v8.cc:221)
==3486371==    by 0x1147FFB: napi_remove_wrap (js_native_api_v8.cc:2282)
==3486371==    by 0x142A30CA: Napi::ObjectWrap<(anonymous namespace)::Test>::~ObjectWrap() (in /home/midawson/refs/node-addon-api/test/build/Debug/binding.node)
==3486371==    by 0x1421CAC4: Napi::ObjectWrap<(anonymous namespace)::Test>::ConstructorCallbackWrapper(napi_env__*, napi_callback_info__*) [clone .cold.114] (in /home/midawson/refs/node-addon-api/test/build/Debug/binding.node)
==3486371==    by 0x113EDB9: v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::operator()(napi_env__*) const (js_native_api_v8.cc:308)
==3486371==    by 0x114C290: void napi_env__::CallIntoModule<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}, v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*, v8::Local<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::Value>)#2}>(v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}&&, v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*, v8::Local<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::Value>)#2}&&) (js_native_api_v8.h:83)
==3486371==    by 0x113EE73: v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback() (js_native_api_v8.cc:308)
==3486371==    by 0x113EED3: v8impl::(anonymous namespace)::FunctionCallbackWrapper::Invoke(v8::FunctionCallbackInfo<v8::Value> const&) (js_native_api_v8.cc:327)
==3486371==    by 0x151BE58: v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) (api-arguments-inl.h:147)
==3486371==    by 0x151CDB4: 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::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) (builtins-api.cc:112)
==3486371==  Block was alloc'd at
==3486371==    at 0x6EBB833: operator new(unsigned long) (vg_replace_malloc.c:417)
==3486371==    by 0x113F5F2: v8impl::Reference::New(napi_env__*, v8::Local<v8::Value>, unsigned int, bool, void (*)(napi_env__*, void*, void*), void*, void*) (js_native_api_v8.cc:567)
==3486371==    by 0x114C5DA: napi_status v8impl::(anonymous namespace)::Wrap<(v8impl::(anonymous namespace)::WrapType)0>(napi_env__*, napi_value__*, void*, void (*)(napi_env__*, void*, void*), void*, napi_ref__**) (js_native_api_v8.cc:432)
==3486371==    by 0x1147F9D: napi_wrap (js_native_api_v8.cc:2269)
==3486371==    by 0x142A4251: Napi::ObjectWrap<(anonymous namespace)::Test>::ConstructorCallbackWrapper(napi_env__*, napi_callback_info__*) (in /home/midawson/refs/node-addon-api/test/build/Debug/binding.node)
==3486371==    by 0x113EDB9: v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::operator()(napi_env__*) const (js_native_api_v8.cc:308)
==3486371==    by 0x114C290: void napi_env__::CallIntoModule<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}, v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*, v8::Local<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::Value>)#2}>(v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}&&, v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*, v8::Local<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::Value>)#2}&&) (js_native_api_v8.h:83)
==3486371==    by 0x113EE73: v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback() (js_native_api_v8.cc:308)
==3486371==    by 0x113EED3: v8impl::(anonymous namespace)::FunctionCallbackWrapper::Invoke(v8::FunctionCallbackInfo<v8::Value> const&) (js_native_api_v8.cc:327)
==3486371==    by 0x151BE58: v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) (api-arguments-inl.h:147)
==3486371==    by 0x151CDB4: 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::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) (builtins-api.cc:112)
==3486371==    by 0x151E9E4: Builtin_Impl_HandleApiCall (builtins-api.cc:138)
==3486371==    by 0x151E9E4: v8::internal::Builtin_HandleApiCall(int, unsigned long*, v8::internal::Isolate*) (builtins-api.cc:130)
==3486371== 
==3486371== Invalid free() / delete / delete[] / realloc()
==3486371==    at 0x6EBE209: operator delete(void*, unsigned long) (vg_replace_malloc.c:814)
==3486371==    by 0x114D018: v8impl::Finalizer::~Finalizer() (js_native_api_v8.h:305)
==3486371==    by 0x1148D75: napi_delete_reference (js_native_api_v8.cc:2423)
==3486371==    by 0x142A30A8: Napi::ObjectWrap<(anonymous namespace)::Test>::~ObjectWrap() (in /home/midawson/refs/node-addon-api/test/build/Debug/binding.node)
==3486371==    by 0x1421CAC4: Napi::ObjectWrap<(anonymous namespace)::Test>::ConstructorCallbackWrapper(napi_env__*, napi_callback_info__*) [clone .cold.114] (in /home/midawson/refs/node-addon-api/test/build/Debug/binding.node)
==3486371==    by 0x113EDB9: v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::operator()(napi_env__*) const (js_native_api_v8.cc:308)
==3486371==    by 0x114C290: void napi_env__::CallIntoModule<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}, v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*, v8::Local<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::Value>)#2}>(v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}&&, v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*, v8::Local<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::Value>)#2}&&) (js_native_api_v8.h:83)
==3486371==    by 0x113EE73: v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback() (js_native_api_v8.cc:308)
==3486371==    by 0x113EED3: v8impl::(anonymous namespace)::FunctionCallbackWrapper::Invoke(v8::FunctionCallbackInfo<v8::Value> const&) (js_native_api_v8.cc:327)
==3486371==    by 0x151BE58: v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) (api-arguments-inl.h:147)
==3486371==    by 0x151CDB4: 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::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) (builtins-api.cc:112)
==3486371==    by 0x151E9E4: Builtin_Impl_HandleApiCall (builtins-api.cc:138)
==3486371==    by 0x151E9E4: v8::internal::Builtin_HandleApiCall(int, unsigned long*, v8::internal::Isolate*) (builtins-api.cc:130)
==3486371==  Address 0x1402aa90 is 0 bytes inside a block of size 80 free'd
==3486371==    at 0x6EBE209: operator delete(void*, unsigned long) (vg_replace_malloc.c:814)
==3486371==    by 0x113F5B6: v8impl::Reference::~Reference() (js_native_api_v8.cc:552)
==3486371==    by 0x113EAF7: v8impl::(anonymous namespace)::Unwrap(napi_env__*, napi_value__*, void**, v8impl::(anonymous namespace)::UnwrapAction) (js_native_api_v8.cc:221)
==3486371==    by 0x1147FFB: napi_remove_wrap (js_native_api_v8.cc:2282)
==3486371==    by 0x142A30CA: Napi::ObjectWrap<(anonymous namespace)::Test>::~ObjectWrap() (in /home/midawson/refs/node-addon-api/test/build/Debug/binding.node)
==3486371==    by 0x1421CAC4: Napi::ObjectWrap<(anonymous namespace)::Test>::ConstructorCallbackWrapper(napi_env__*, napi_callback_info__*) [clone .cold.114] (in /home/midawson/refs/node-addon-api/test/build/Debug/binding.node)
==3486371==    by 0x113EDB9: v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::operator()(napi_env__*) const (js_native_api_v8.cc:308)
==3486371==    by 0x114C290: void napi_env__::CallIntoModule<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}, v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*, v8::Local<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::Value>)#2}>(v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}&&, v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*, v8::Local<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::Value>)#2}&&) (js_native_api_v8.h:83)
==3486371==    by 0x113EE73: v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback() (js_native_api_v8.cc:308)
==3486371==    by 0x113EED3: v8impl::(anonymous namespace)::FunctionCallbackWrapper::Invoke(v8::FunctionCallbackInfo<v8::Value> const&) (js_native_api_v8.cc:327)
==3486371==    by 0x151BE58: v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) (api-arguments-inl.h:147)
==3486371==    by 0x151CDB4: 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::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) (builtins-api.cc:112)
==3486371==  Block was alloc'd at
==3486371==    at 0x6EBB833: operator new(unsigned long) (vg_replace_malloc.c:417)
==3486371==    by 0x113F5F2: v8impl::Reference::New(napi_env__*, v8::Local<v8::Value>, unsigned int, bool, void (*)(napi_env__*, void*, void*), void*, void*) (js_native_api_v8.cc:567)
==3486371==    by 0x114C5DA: napi_status v8impl::(anonymous namespace)::Wrap<(v8impl::(anonymous namespace)::WrapType)0>(napi_env__*, napi_value__*, void*, void (*)(napi_env__*, void*, void*), void*, napi_ref__**) (js_native_api_v8.cc:432)
==3486371==    by 0x1147F9D: napi_wrap (js_native_api_v8.cc:2269)
==3486371==    by 0x142A4251: Napi::ObjectWrap<(anonymous namespace)::Test>::ConstructorCallbackWrapper(napi_env__*, napi_callback_info__*) (in /home/midawson/refs/node-addon-api/test/build/Debug/binding.node)
==3486371==    by 0x113EDB9: v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::operator()(napi_env__*) const (js_native_api_v8.cc:308)
==3486371==    by 0x114C290: void napi_env__::CallIntoModule<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}, v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*, v8::Local<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::Value>)#2}>(v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}&&, v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*, v8::Local<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::Value>)#2}&&) (js_native_api_v8.h:83)
==3486371==    by 0x113EE73: v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback() (js_native_api_v8.cc:308)
==3486371==    by 0x113EED3: v8impl::(anonymous namespace)::FunctionCallbackWrapper::Invoke(v8::FunctionCallbackInfo<v8::Value> const&) (js_native_api_v8.cc:327)
==3486371==    by 0x151BE58: v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) (api-arguments-inl.h:147)
==3486371==    by 0x151CDB4: 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::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) (builtins-api.cc:112)
==3486371==    by 0x151E9E4: Builtin_Impl_HandleApiCall (builtins-api.cc:138)
==3486371==    by 0x151E9E4: v8::internal::Builtin_HandleApiCall(int, unsigned long*, v8::internal::Isolate*) (builtins-api.cc:130)
==3486371== 
==3486371== Conditional jump or move depends on uninitialised value(s)
==3486371==    at 0x16E47C7: CheckNEImpl<long unsigned int, long unsigned int> (logging.h:369)
==3486371==    by 0x16E47C7: v8::internal::ExitFrame::GetStateForFramePointer(unsigned long, v8::internal::StackFrame::State*) [clone .part.195] (frames.cc:815)
==3486371==    by 0x16E48E2: GetStateForFramePointer (frames.cc:161)
==3486371==    by 0x16E48E2: v8::internal::StackFrameIterator::Reset(v8::internal::ThreadLocalTop*) (frames.cc:158)
==3486371==    by 0x17078EB: v8::internal::Isolate::UnwindAndFindHandler() (isolate.cc:1916)
==3486371==    by 0x1D2E572: __RT_impl_Runtime_UnwindAndFindExceptionHandler (runtime-internal.cc:189)
==3486371==    by 0x1D2E572: v8::internal::Runtime_UnwindAndFindExceptionHandler(int, unsigned long*, v8::internal::Isolate*) (runtime-internal.cc:186)
==3486371==    by 0x228F2A5: Builtins_CEntry_Return1_DontSaveFPRegs_ArgvOnStack_BuiltinExit (torque-internal.tq:101)
==3486371==    by 0x220106B: Builtins_JSBuiltinsConstructStub (torque-internal.tq:101)
==3486371==    by 0x2357BA8: Builtins_ConstructHandler (torque-internal.tq:101)
==3486371==    by 0x2203D0F: Builtins_InterpreterEntryTrampoline (in /home/midawson/refs/node/out/Debug/node)
==3486371==    by 0x2203D0F: Builtins_InterpreterEntryTrampoline (in /home/midawson/refs/node/out/Debug/node)
==3486371==    by 0x2203D0F: Builtins_InterpreterEntryTrampoline (in /home/midawson/refs/node/out/Debug/node)
==3486371==    by 0x2203D0F: Builtins_InterpreterEntryTrampoline (in /home/midawson/refs/node/out/Debug/node)
==3486371==    by 0x2203D0F: Builtins_InterpreterEntryTrampoline (in /home/midawson/refs/node/out/Debug/node)
==3486371== 

mhdawson avatar Sep 26 '22 20:09 mhdawson

In the test/objectwrap_removewrap.js test, from the doc with the case I think that we are hitting this case:

  • napi_wrap, when the in-out napi_ref parameter is set, deleted with napi_remove_wrap

It looks like maybe the delete it being done in both the napi_remove_wrap and by the runtime automatically?

mhdawson avatar Sep 26 '22 21:09 mhdawson

From the comments in the previous code:

// When it is called from Unwrap or napi_delete_reference we only
// want to do the delete if the finalizer has already run or
// cannot have been queued to run (ie the reference count is > 0),
// otherwise we may crash when the finalizer does run.

I think we may be hitting that case as it looks like we crash in the Finalizer:

 Invalid write of size 8
==3486371==    at 0x114CFE9: v8impl::Finalizer::~Finalizer() (js_native_api_v8.h:305)
==3486371==    by 0x114D007: v8impl::Finalizer::~Finalizer() (js_native_api_v8.h:305)
==3486371==    by 0x1148D75: napi_delete_reference (js_native_api_v8.cc:2423)
==3486371==    by 0x142A30A8: Napi::ObjectWrap<(anonymous namespace)::Test>::~ObjectWrap() (in /home/midawson/refs/node-addon-api/test/build/Debug/binding.node)
==3486371==    by 0x1421CAC4: Napi::ObjectWrap<(anonymous namespace)::Test>::ConstructorCallbackWrapper(napi_env__*, napi_callback_info__*) [clone .cold.114] (in /home/midawson/refs/node-addon-api/test/build/Debug/binding.node)
==3486371==    by 0x113EDB9: v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::operator()(napi_env__*) const (js_native_api_v8.cc:308)
==3486371==    by 0x114C290: void napi_env__::CallIntoModule<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}, v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*, v8::Local<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::Value>)#2}>(v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}&&, v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*, v8::Local<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::Value>)#2}&&) (js_native_api_v8.h:83)
==3486371==    by 0x113EE73: v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback() (js_native_api_v8.cc:308)
==3486371==    by 0x113EED3: v8impl::(anonymous namespace)::FunctionCallbackWrapper::Invoke(v8::FunctionCallbackInfo<v8::Value> const&) (js_native_api_v8.cc:327)
==3486371==    by 0x151BE58: v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) (api-arguments-inl.h:147)
==3486371==    by 0x151CDB4: 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::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) (builtins-api.cc:112)
==3486371==    by 0x151E9E4: Builtin_Impl_HandleApiCall (builtins-api.cc:138)
==3486371==    by 0x151E9E4: v8::internal::Builtin_HandleApiCall(int, unsigned long*, v8::internal::Isolate*) (builtins-api.cc:130)

mhdawson avatar Sep 26 '22 21:09 mhdawson

@legendecas I think we will need something like this back. If there was a finalizer and there was any chance it was scheduled we want to let it complete (I had though we might just unlink, but I think that might result in the finalizer not runing when it is expected that it will).

void RefBase::Delete(RefBase* reference) {
  if ((reference->RefCount() != 0) || (reference->_delete_self) ||
      (reference->_finalize_ran)) {
    delete reference;
  } else {
    // defer until finalizer runs as
    // it may already be queued
    reference->_delete_self = true;
  }
}

mhdawson avatar Sep 26 '22 21:09 mhdawson

Looking at the other failure: Starting program: /home/midawson/refs/node/node test/objectwrap_constructor_exception.j

The stack trace

Thread 1 "node" received signal SIGSEGV, Segmentation fault.
0x0000000001148d6a in napi_delete_reference (env=0x7ffff7ecc8d0, ref=0x7ffff7e54330) at ../src/js_native_api_v8.cc:2423
warning: Source file is more recent than executable.
2423	  delete reinterpret_cast<v8impl::Reference*>(ref);
Missing separate debuginfos, use: yum debuginfo-install libgcc-8.5.0-4.el8_5.x86_64 libstdc++-8.5.0-4.el8_5.x86_64
(gdb) bt
#0  0x0000000001148d6a in napi_delete_reference (env=0x7ffff7ecc8d0, ref=0x7ffff7e54330) at ../src/js_native_api_v8.cc:2423
#1  0x00007ffff416a239 in Napi::ObjectWrap<ConstructorExceptionTest>::~ObjectWrap() ()
   from /home/midawson/refs/node-addon-api/test/build/Debug/binding.node
#2  0x00007ffff416c06e in Napi::ObjectWrap<ConstructorExceptionTest>::ConstructorCallbackWrapper(napi_env__*, napi_callback_info__*) ()
   from /home/midawson/refs/node-addon-api/test/build/Debug/binding.node
#3  0x000000000113edba in v8impl::(anonymous namespace)::CallbackWrapperBase::<lambda(napi_env)>::operator()(napi_env) const (__closure=0x7fffffffcb20, 
    env=0x7ffff7ecc8d0) at ../src/js_native_api_v8.cc:308
#4  0x000000000114c291 in napi_env__::CallIntoModule<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::<lambda(napi_env)>, v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::<lambda(napi_env, v8::Local<v8::Value>)> >(v8impl::(anonymous namespace)::CallbackWrapperBase::<lambda(napi_env)> &&, v8impl::(anonymous namespace)::CallbackWrapperBase::<lambda(napi_env, v8::Local<v8::Value>)> &&) (this=0x7ffff7ecc8d0, call=..., 
    handle_exception=...) at ../src/js_native_api_v8.h:83
#5  0x000000000113ee74 in v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback (this=0x7fffffffcb60) at ../src/js_native_api_v8.cc:308
#6  0x000000000113eed4 in v8impl::(anonymous namespace)::FunctionCallbackWrapper::Invoke (info=...) at ../src/js_native_api_v8.cc:327
#7  0x000000000151be59 in v8::internal::FunctionCallbackArguments::Call (this=this@entry=0x7fffffffccb0, handler=handler@entry=...)
    at ../deps/v8/src/api/api-arguments-inl.h:147
#8  0x000000000151cdb5 in v8::internal::(anonymous namespace)::HandleApiCallHelper<true> (isolate=isolate@entry=0x65efd80, function=..., 
    function@entry=..., new_target=new_target@entry=..., fun_data=..., receiver=..., receiver@entry=..., args=...)
    at ../deps/v8/src/objects/heap-object.h:173
#9  0x000000000151e9e5 in v8::internal::Builtin_Impl_HandleApiCall (isolate=0x65efd80, args=...) at ../deps/v8/src/handles/handles.h:133
#10 v8::internal::Builtin_HandleApiCall (args_length=<optimized out>, args_object=0x7fffffffce20, isolate=0x65efd80)
    at ../deps/v8/src/builtins/builtins-api.cc:130
#11 0x000000000228f239 in Builtins_CEntry_Return1_DontSaveFPRegs_ArgvOnStack_BuiltinExit () at ../../deps/v8/src/builtins/torque-internal.tq:101
#12 0x000000000220106c in Builtins_JSBuiltinsConstructStub () at ../../deps/v8/src/builtins/torque-internal.tq:101
#13 0x0000032520e061a1 in ?? ()
#14 0x0000032520e061a1 in ?? ()
#15 0x0000000500000000 in ?? ()
#16 0x00000984b9ac1689 in ?? ()
#17 0x000034e3bf8637d1 in ?? ()
#18 0x0000000100000000 in ?? ()
#19 0x000034e3bf8636b9 in ?? ()
#20 0x0000000000000028 in ?? ()
#21 0x00007fffffffcea0 in ?? ()
#22 0x0000000002357ba9 in Builtins_ConstructHandler () at ../../deps/v8/src/builtins/torque-internal.tq:101
Backtrace stopped: frame did not save the PC
(gdb) 

mhdawson avatar Sep 26 '22 22:09 mhdawson

And the valgrind output

[midawson@drx-hemera node-addon-api]$ valgrind node test/objectwrap_constructor_exception.js
==3495085== Memcheck, a memory error detector
==3495085== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==3495085== Using Valgrind-3.17.0 and LibVEX; rerun with -h for copyright info
==3495085== Command: node test/objectwrap_constructor_exception.js
==3495085== 
==3495085== Invalid read of size 8
==3495085==    at 0x1148D63: napi_delete_reference (js_native_api_v8.cc:2423)
==3495085==    by 0x1429E238: Napi::ObjectWrap<ConstructorExceptionTest>::~ObjectWrap() (in /home/midawson/refs/node-addon-api/test/build/Debug/binding.node)
==3495085==    by 0x142A006D: Napi::ObjectWrap<ConstructorExceptionTest>::ConstructorCallbackWrapper(napi_env__*, napi_callback_info__*) (in /home/midawson/refs/node-addon-api/test/build/Debug/binding.node)
==3495085==    by 0x113EDB9: v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::operator()(napi_env__*) const (js_native_api_v8.cc:308)
==3495085==    by 0x114C290: void napi_env__::CallIntoModule<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}, v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*, v8::Local<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::Value>)#2}>(v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}&&, v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*, v8::Local<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::Value>)#2}&&) (js_native_api_v8.h:83)
==3495085==    by 0x113EE73: v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback() (js_native_api_v8.cc:308)
==3495085==    by 0x113EED3: v8impl::(anonymous namespace)::FunctionCallbackWrapper::Invoke(v8::FunctionCallbackInfo<v8::Value> const&) (js_native_api_v8.cc:327)
==3495085==    by 0x151BE58: v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) (api-arguments-inl.h:147)
==3495085==    by 0x151CDB4: 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::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) (builtins-api.cc:112)
==3495085==    by 0x151E9E4: Builtin_Impl_HandleApiCall (builtins-api.cc:138)
==3495085==    by 0x151E9E4: v8::internal::Builtin_HandleApiCall(int, unsigned long*, v8::internal::Isolate*) (builtins-api.cc:130)
==3495085==    by 0x228F238: Builtins_CEntry_Return1_DontSaveFPRegs_ArgvOnStack_BuiltinExit (torque-internal.tq:101)
==3495085==    by 0x220106B: Builtins_JSBuiltinsConstructStub (torque-internal.tq:101)
==3495085==  Address 0x14029420 is 0 bytes inside a block of size 80 free'd
==3495085==    at 0x6EBE209: operator delete(void*, unsigned long) (vg_replace_malloc.c:814)
==3495085==    by 0x113F5B6: v8impl::Reference::~Reference() (js_native_api_v8.cc:552)
==3495085==    by 0x113EAF7: v8impl::(anonymous namespace)::Unwrap(napi_env__*, napi_value__*, void**, v8impl::(anonymous namespace)::UnwrapAction) (js_native_api_v8.cc:221)
==3495085==    by 0x1147FFB: napi_remove_wrap (js_native_api_v8.cc:2282)
==3495085==    by 0x1429E25A: Napi::ObjectWrap<ConstructorExceptionTest>::~ObjectWrap() (in /home/midawson/refs/node-addon-api/test/build/Debug/binding.node)
==3495085==    by 0x142A006D: Napi::ObjectWrap<ConstructorExceptionTest>::ConstructorCallbackWrapper(napi_env__*, napi_callback_info__*) (in /home/midawson/refs/node-addon-api/test/build/Debug/binding.node)
==3495085==    by 0x113EDB9: v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::operator()(napi_env__*) const (js_native_api_v8.cc:308)
==3495085==    by 0x114C290: void napi_env__::CallIntoModule<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}, v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*, v8::Local<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::Value>)#2}>(v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}&&, v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*, v8::Local<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::Value>)#2}&&) (js_native_api_v8.h:83)
==3495085==    by 0x113EE73: v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback() (js_native_api_v8.cc:308)
==3495085==    by 0x113EED3: v8impl::(anonymous namespace)::FunctionCallbackWrapper::Invoke(v8::FunctionCallbackInfo<v8::Value> const&) (js_native_api_v8.cc:327)
==3495085==    by 0x151BE58: v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) (api-arguments-inl.h:147)
==3495085==    by 0x151CDB4: 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::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) (builtins-api.cc:112)
==3495085==  Block was alloc'd at
==3495085==    at 0x6EBB833: operator new(unsigned long) (vg_replace_malloc.c:417)
==3495085==    by 0x113F5F2: v8impl::Reference::New(napi_env__*, v8::Local<v8::Value>, unsigned int, bool, void (*)(napi_env__*, void*, void*), void*, void*) (js_native_api_v8.cc:567)
==3495085==    by 0x114C5DA: napi_status v8impl::(anonymous namespace)::Wrap<(v8impl::(anonymous namespace)::WrapType)0>(napi_env__*, napi_value__*, void*, void (*)(napi_env__*, void*, void*), void*, napi_ref__**) (js_native_api_v8.cc:432)
==3495085==    by 0x1147F9D: napi_wrap (js_native_api_v8.cc:2269)
==3495085==    by 0x1429F7F3: Napi::ObjectWrap<ConstructorExceptionTest>::ObjectWrap(Napi::CallbackInfo const&) (in /home/midawson/refs/node-addon-api/test/build/Debug/binding.node)
==3495085==    by 0x1429FC5C: Napi::ObjectWrap<ConstructorExceptionTest>::ConstructorCallbackWrapper(napi_env__*, napi_callback_info__*) (in /home/midawson/refs/node-addon-api/test/build/Debug/binding.node)
==3495085==    by 0x113EDB9: v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::operator()(napi_env__*) const (js_native_api_v8.cc:308)
==3495085==    by 0x114C290: void napi_env__::CallIntoModule<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}, v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*, v8::Local<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::Value>)#2}>(v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}&&, v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*, v8::Local<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::Value>)#2}&&) (js_native_api_v8.h:83)
==3495085==    by 0x113EE73: v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback() (js_native_api_v8.cc:308)
==3495085==    by 0x113EED3: v8impl::(anonymous namespace)::FunctionCallbackWrapper::Invoke(v8::FunctionCallbackInfo<v8::Value> const&) (js_native_api_v8.cc:327)
==3495085==    by 0x151BE58: v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) (api-arguments-inl.h:147)
==3495085==    by 0x151CDB4: 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::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) (builtins-api.cc:112)
==3495085== 
==3495085== Invalid write of size 8
==3495085==    at 0x114CFE9: v8impl::Finalizer::~Finalizer() (js_native_api_v8.h:305)
==3495085==    by 0x114D007: v8impl::Finalizer::~Finalizer() (js_native_api_v8.h:305)
==3495085==    by 0x1148D75: napi_delete_reference (js_native_api_v8.cc:2423)
==3495085==    by 0x1429E238: Napi::ObjectWrap<ConstructorExceptionTest>::~ObjectWrap() (in /home/midawson/refs/node-addon-api/test/build/Debug/binding.node)
==3495085==    by 0x142A006D: Napi::ObjectWrap<ConstructorExceptionTest>::ConstructorCallbackWrapper(napi_env__*, napi_callback_info__*) (in /home/midawson/refs/node-addon-api/test/build/Debug/binding.node)
==3495085==    by 0x113EDB9: v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::operator()(napi_env__*) const (js_native_api_v8.cc:308)
==3495085==    by 0x114C290: void napi_env__::CallIntoModule<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}, v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*, v8::Local<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::Value>)#2}>(v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}&&, v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*, v8::Local<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::Value>)#2}&&) (js_native_api_v8.h:83)
==3495085==    by 0x113EE73: v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback() (js_native_api_v8.cc:308)
==3495085==    by 0x113EED3: v8impl::(anonymous namespace)::FunctionCallbackWrapper::Invoke(v8::FunctionCallbackInfo<v8::Value> const&) (js_native_api_v8.cc:327)
==3495085==    by 0x151BE58: v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) (api-arguments-inl.h:147)
==3495085==    by 0x151CDB4: 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::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) (builtins-api.cc:112)
==3495085==    by 0x151E9E4: Builtin_Impl_HandleApiCall (builtins-api.cc:138)
==3495085==    by 0x151E9E4: v8::internal::Builtin_HandleApiCall(int, unsigned long*, v8::internal::Isolate*) (builtins-api.cc:130)
==3495085==  Address 0x14029420 is 0 bytes inside a block of size 80 free'd
==3495085==    at 0x6EBE209: operator delete(void*, unsigned long) (vg_replace_malloc.c:814)
==3495085==    by 0x113F5B6: v8impl::Reference::~Reference() (js_native_api_v8.cc:552)
==3495085==    by 0x113EAF7: v8impl::(anonymous namespace)::Unwrap(napi_env__*, napi_value__*, void**, v8impl::(anonymous namespace)::UnwrapAction) (js_native_api_v8.cc:221)
==3495085==    by 0x1147FFB: napi_remove_wrap (js_native_api_v8.cc:2282)
==3495085==    by 0x1429E25A: Napi::ObjectWrap<ConstructorExceptionTest>::~ObjectWrap() (in /home/midawson/refs/node-addon-api/test/build/Debug/binding.node)
==3495085==    by 0x142A006D: Napi::ObjectWrap<ConstructorExceptionTest>::ConstructorCallbackWrapper(napi_env__*, napi_callback_info__*) (in /home/midawson/refs/node-addon-api/test/build/Debug/binding.node)
==3495085==    by 0x113EDB9: v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::operator()(napi_env__*) const (js_native_api_v8.cc:308)
==3495085==    by 0x114C290: void napi_env__::CallIntoModule<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}, v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*, v8::Local<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::Value>)#2}>(v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}&&, v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*, v8::Local<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::Value>)#2}&&) (js_native_api_v8.h:83)
==3495085==    by 0x113EE73: v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback() (js_native_api_v8.cc:308)
==3495085==    by 0x113EED3: v8impl::(anonymous namespace)::FunctionCallbackWrapper::Invoke(v8::FunctionCallbackInfo<v8::Value> const&) (js_native_api_v8.cc:327)
==3495085==    by 0x151BE58: v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) (api-arguments-inl.h:147)
==3495085==    by 0x151CDB4: 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::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) (builtins-api.cc:112)
==3495085==  Block was alloc'd at
==3495085==    at 0x6EBB833: operator new(unsigned long) (vg_replace_malloc.c:417)
==3495085==    by 0x113F5F2: v8impl::Reference::New(napi_env__*, v8::Local<v8::Value>, unsigned int, bool, void (*)(napi_env__*, void*, void*), void*, void*) (js_native_api_v8.cc:567)
==3495085==    by 0x114C5DA: napi_status v8impl::(anonymous namespace)::Wrap<(v8impl::(anonymous namespace)::WrapType)0>(napi_env__*, napi_value__*, void*, void (*)(napi_env__*, void*, void*), void*, napi_ref__**) (js_native_api_v8.cc:432)
==3495085==    by 0x1147F9D: napi_wrap (js_native_api_v8.cc:2269)
==3495085==    by 0x1429F7F3: Napi::ObjectWrap<ConstructorExceptionTest>::ObjectWrap(Napi::CallbackInfo const&) (in /home/midawson/refs/node-addon-api/test/build/Debug/binding.node)
==3495085==    by 0x1429FC5C: Napi::ObjectWrap<ConstructorExceptionTest>::ConstructorCallbackWrapper(napi_env__*, napi_callback_info__*) (in /home/midawson/refs/node-addon-api/test/build/Debug/binding.node)
==3495085==    by 0x113EDB9: v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::operator()(napi_env__*) const (js_native_api_v8.cc:308)
==3495085==    by 0x114C290: void napi_env__::CallIntoModule<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}, v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*, v8::Local<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::Value>)#2}>(v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}&&, v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*, v8::Local<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::Value>)#2}&&) (js_native_api_v8.h:83)
==3495085==    by 0x113EE73: v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback() (js_native_api_v8.cc:308)
==3495085==    by 0x113EED3: v8impl::(anonymous namespace)::FunctionCallbackWrapper::Invoke(v8::FunctionCallbackInfo<v8::Value> const&) (js_native_api_v8.cc:327)
==3495085==    by 0x151BE58: v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) (api-arguments-inl.h:147)
==3495085==    by 0x151CDB4: 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::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) (builtins-api.cc:112)
==3495085== 
==3495085== Invalid free() / delete / delete[] / realloc()
==3495085==    at 0x6EBE209: operator delete(void*, unsigned long) (vg_replace_malloc.c:814)
==3495085==    by 0x114D018: v8impl::Finalizer::~Finalizer() (js_native_api_v8.h:305)
==3495085==    by 0x1148D75: napi_delete_reference (js_native_api_v8.cc:2423)
==3495085==    by 0x1429E238: Napi::ObjectWrap<ConstructorExceptionTest>::~ObjectWrap() (in /home/midawson/refs/node-addon-api/test/build/Debug/binding.node)
==3495085==    by 0x142A006D: Napi::ObjectWrap<ConstructorExceptionTest>::ConstructorCallbackWrapper(napi_env__*, napi_callback_info__*) (in /home/midawson/refs/node-addon-api/test/build/Debug/binding.node)
==3495085==    by 0x113EDB9: v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::operator()(napi_env__*) const (js_native_api_v8.cc:308)
==3495085==    by 0x114C290: void napi_env__::CallIntoModule<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}, v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*, v8::Local<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::Value>)#2}>(v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}&&, v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*, v8::Local<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::Value>)#2}&&) (js_native_api_v8.h:83)
==3495085==    by 0x113EE73: v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback() (js_native_api_v8.cc:308)
==3495085==    by 0x113EED3: v8impl::(anonymous namespace)::FunctionCallbackWrapper::Invoke(v8::FunctionCallbackInfo<v8::Value> const&) (js_native_api_v8.cc:327)
==3495085==    by 0x151BE58: v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) (api-arguments-inl.h:147)
==3495085==    by 0x151CDB4: 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::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) (builtins-api.cc:112)
==3495085==    by 0x151E9E4: Builtin_Impl_HandleApiCall (builtins-api.cc:138)
==3495085==    by 0x151E9E4: v8::internal::Builtin_HandleApiCall(int, unsigned long*, v8::internal::Isolate*) (builtins-api.cc:130)
==3495085==  Address 0x14029420 is 0 bytes inside a block of size 80 free'd
==3495085==    at 0x6EBE209: operator delete(void*, unsigned long) (vg_replace_malloc.c:814)
==3495085==    by 0x113F5B6: v8impl::Reference::~Reference() (js_native_api_v8.cc:552)
==3495085==    by 0x113EAF7: v8impl::(anonymous namespace)::Unwrap(napi_env__*, napi_value__*, void**, v8impl::(anonymous namespace)::UnwrapAction) (js_native_api_v8.cc:221)
==3495085==    by 0x1147FFB: napi_remove_wrap (js_native_api_v8.cc:2282)
==3495085==    by 0x1429E25A: Napi::ObjectWrap<ConstructorExceptionTest>::~ObjectWrap() (in /home/midawson/refs/node-addon-api/test/build/Debug/binding.node)
==3495085==    by 0x142A006D: Napi::ObjectWrap<ConstructorExceptionTest>::ConstructorCallbackWrapper(napi_env__*, napi_callback_info__*) (in /home/midawson/refs/node-addon-api/test/build/Debug/binding.node)
==3495085==    by 0x113EDB9: v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::operator()(napi_env__*) const (js_native_api_v8.cc:308)
==3495085==    by 0x114C290: void napi_env__::CallIntoModule<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}, v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*, v8::Local<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::Value>)#2}>(v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}&&, v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*, v8::Local<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::Value>)#2}&&) (js_native_api_v8.h:83)
==3495085==    by 0x113EE73: v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback() (js_native_api_v8.cc:308)
==3495085==    by 0x113EED3: v8impl::(anonymous namespace)::FunctionCallbackWrapper::Invoke(v8::FunctionCallbackInfo<v8::Value> const&) (js_native_api_v8.cc:327)
==3495085==    by 0x151BE58: v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) (api-arguments-inl.h:147)
==3495085==    by 0x151CDB4: 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::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) (builtins-api.cc:112)
==3495085==  Block was alloc'd at
==3495085==    at 0x6EBB833: operator new(unsigned long) (vg_replace_malloc.c:417)
==3495085==    by 0x113F5F2: v8impl::Reference::New(napi_env__*, v8::Local<v8::Value>, unsigned int, bool, void (*)(napi_env__*, void*, void*), void*, void*) (js_native_api_v8.cc:567)
==3495085==    by 0x114C5DA: napi_status v8impl::(anonymous namespace)::Wrap<(v8impl::(anonymous namespace)::WrapType)0>(napi_env__*, napi_value__*, void*, void (*)(napi_env__*, void*, void*), void*, napi_ref__**) (js_native_api_v8.cc:432)
==3495085==    by 0x1147F9D: napi_wrap (js_native_api_v8.cc:2269)
==3495085==    by 0x1429F7F3: Napi::ObjectWrap<ConstructorExceptionTest>::ObjectWrap(Napi::CallbackInfo const&) (in /home/midawson/refs/node-addon-api/test/build/Debug/binding.node)
==3495085==    by 0x1429FC5C: Napi::ObjectWrap<ConstructorExceptionTest>::ConstructorCallbackWrapper(napi_env__*, napi_callback_info__*) (in /home/midawson/refs/node-addon-api/test/build/Debug/binding.node)
==3495085==    by 0x113EDB9: v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::operator()(napi_env__*) const (js_native_api_v8.cc:308)
==3495085==    by 0x114C290: void napi_env__::CallIntoModule<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}, v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*, v8::Local<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::Value>)#2}>(v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}&&, v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*, v8::Local<v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback()::{lambda(napi_env__*)#1}::Value>)#2}&&) (js_native_api_v8.h:83)
==3495085==    by 0x113EE73: v8impl::(anonymous namespace)::CallbackWrapperBase::InvokeCallback() (js_native_api_v8.cc:308)
==3495085==    by 0x113EED3: v8impl::(anonymous namespace)::FunctionCallbackWrapper::Invoke(v8::FunctionCallbackInfo<v8::Value> const&) (js_native_api_v8.cc:327)
==3495085==    by 0x151BE58: v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) (api-arguments-inl.h:147)
==3495085==    by 0x151CDB4: 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::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) (builtins-api.cc:112)
==3495085== 
==3495085== Conditional jump or move depends on uninitialised value(s)
==3495085==    at 0x16E47C7: CheckNEImpl<long unsigned int, long unsigned int> (logging.h:369)
==3495085==    by 0x16E47C7: v8::internal::ExitFrame::GetStateForFramePointer(unsigned long, v8::internal::StackFrame::State*) [clone .part.195] (frames.cc:815)
==3495085==    by 0x16E48E2: GetStateForFramePointer (frames.cc:161)
==3495085==    by 0x16E48E2: v8::internal::StackFrameIterator::Reset(v8::internal::ThreadLocalTop*) (frames.cc:158)
==3495085==    by 0x17078EB: v8::internal::Isolate::UnwindAndFindHandler() (isolate.cc:1916)
==3495085==    by 0x1D2E572: __RT_impl_Runtime_UnwindAndFindExceptionHandler (runtime-internal.cc:189)
==3495085==    by 0x1D2E572: v8::internal::Runtime_UnwindAndFindExceptionHandler(int, unsigned long*, v8::internal::Isolate*) (runtime-internal.cc:186)
==3495085==    by 0x228F2A5: Builtins_CEntry_Return1_DontSaveFPRegs_ArgvOnStack_BuiltinExit (torque-internal.tq:101)
==3495085==    by 0x220106B: Builtins_JSBuiltinsConstructStub (torque-internal.tq:101)
==3495085==    by 0x2357BA8: Builtins_ConstructHandler (torque-internal.tq:101)
==3495085==    by 0x2203D0F: Builtins_InterpreterEntryTrampoline (in /home/midawson/refs/node/out/Debug/node)
==3495085==    by 0x2203D0F: Builtins_InterpreterEntryTrampoline (in /home/midawson/refs/node/out/Debug/node)
==3495085==    by 0x2203D0F: Builtins_InterpreterEntryTrampoline (in /home/midawson/refs/node/out/Debug/node)
==3495085==    by 0x2203D0F: Builtins_InterpreterEntryTrampoline (in /home/midawson/refs/node/out/Debug/node)
==3495085==    by 0x2203D0F: Builtins_InterpreterEntryTrampoline (in /home/midawson/refs/node/out/Debug/node)
==3495085== 

mhdawson avatar Sep 26 '22 22:09 mhdawson

So the second failing test seems to look like the same/similar problem as the stack trace and valgrind output looks very similar.

mhdawson avatar Sep 26 '22 22:09 mhdawson

hmm, got this failure in another run. EDIT I think I did not see it earlier since I was not running the debug build which I am now. It is also possible that is not related to this issue, we'd have to see if we get the same check on the existing code.

Running test 'function_reference'


#
# Fatal error in ../deps/v8/src/objects/tagged-impl.h, line 90
# Debug check failed: kCanBeWeak || (!IsSmi() == HAS_STRONG_HEAP_OBJECT_TAG(ptr_)).
#
#
#
#FailureMessage Object: 0x7fffffffc690
 1: 0x10d80e9 node::DumpBacktrace(_IO_FILE*) [/home/midawson/refs/node/out/Debug/node]
 2: 0x1290171  [/home/midawson/refs/node/out/Debug/node]
 3: 0x1290191  [/home/midawson/refs/node/out/Debug/node]
 4: 0x2a911a7 V8_Fatal(char const*, int, char const*, ...) [/home/midawson/refs/node/out/Debug/node]
 5: 0x2a911d3  [/home/midawson/refs/node/out/Debug/node]
 6: 0x151af24 v8::internal::Object::VerifyApiCallResultType() [/home/midawson/refs/node/out/Debug/node]
 7: 0x151b18d v8::internal::Handle<v8::internal::Object> v8::internal::CustomArguments<v8::FunctionCallbackInfo<v8::Value> >::GetReturnValue<v8::internal::Object>(v8::internal::Isolate*) [/home/midawson/refs/node/out/Debug/node]
 8: 0x151be64 v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) [/home/midawson/refs/node/out/Debug/node]
 9: 0x151de0f  [/home/midawson/refs/node/out/Debug/node]
10: 0x151e8b1 v8::internal::Builtin_HandleApiCall(int, unsigned long*, v8::internal::Isolate*) [/home/midawson/refs/node/out/Debug/node]
11: 0x228f239  [/home/midawson/refs/node/out/Debug/node]
Tests aborted with SIGTRAP

mhdawson avatar Sep 26 '22 22:09 mhdawson

@mhdawson thank you for verifying the patch with node-addon-api test sets.

I believe the problem here is what napi_remove_wrap should do when the out-param napi_ref result is set on napi_wrap. The document said that:

napi_remove_wrap: Retrieves a native instance that was previously wrapped in the JavaScript object js_object using napi_wrap() and removes the wrapping. If a finalizer callback was associated with the wrapping, it will no longer be called when the JavaScript object becomes garbage-collected. https://nodejs.org/api/n-api.html#napi_remove_wrap

However, I found that on the main branch, when the napi_remove_wrap is called but the napi_delete_reference is not called with the out-param napi_ref result, the finalizer can still be invoked. I've added a test case for it: https://github.com/nodejs/node/pull/44141/files#diff-8037a0113a35b91ac2d997501123d0b8be5b341907e9e350fe4f70082afa361f.

So basically, when the napi_wrap is called with non-null out-paramnapi_ref result, the expected behavior would be:

  1. invoke napi_remove_wrap with the wrapped object to invalidate the wrap, after this point no finalizer should be invoked, and napi_get_reference_value can still get the ref-ed value from the out-param napi_ref result.
  2. invoke napi_delete_reference with the out-param napi_ref result, to delete the user-owned napi_ref.

legendecas avatar Oct 14 '22 07:10 legendecas

CI: https://ci.nodejs.org/job/node-test-pull-request/48011/

nodejs-github-bot avatar Nov 18 '22 16:11 nodejs-github-bot

@mhdawson updated, PTAL again, thank you :)

legendecas avatar Nov 21 '22 09:11 legendecas

Landed in f14fa1bb

legendecas avatar Dec 19 '22 17:12 legendecas