nodejs / nodejs/node-addon-api

`napi_adjust_external_memory` crashes when `NAPI_EXPERIMENTAL` is defined

Open
#1,724 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

stale
Dominant language
C++
Stars
2.4k
Forks
499
Avg merge
2d 11h
Merged PRs (30d)
2

Description

Background

In node-canvas we've had a lot of people report memory leaks due to destructors getting called asynchronously, so I want to use the new "nogc" APIs. But when I define NAPI_EXPERIMENTAL, our test suite quickly crashes during GC. It's trying to execute some JS, which I know is illegal, but I can't figure out what is scheduling the JS, and I didn't think that should be possible if I make an API call with napi_basic_env.

Questions

I should be able to call any API that takes basic_env both inside and outside of GC, right?

If I add the Finalizer(Napi::Env) method, node-addon-api switches to calling the destructor asynchronously with node_api_post_finalizer, which does work. That makes me think that it's crashing in the synchronous delete instance of a previous canvas instance. But I can't find anything we're calling with the extended env.

To repro, just run node repro.js from the sync-gc branch. More context here.

LLDB stack trace 
[I] caleb@2024-MacBook-Pro ~/C/node-canvas (sync-gc) [SIGSEGV]> lldb node repro.js
(lldb) target create "node"
Current executable set to '/opt/homebrew/bin/node' (arm64).
(lldb) settings set -- target.run-args  "repro.js"
(lldb) run
Process 98580 launched: '/opt/homebrew/bin/node' (arm64)
Process 98580 stopped
* thread #1, name = 'MainThread', queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x8e4803f0bdb0)
    frame #0: 0x0000000103fd7668 libnode.141.dylib`v8impl::Reference::WeakCallback(v8::WeakCallbackInfo<v8impl::Reference> const&) + 36
libnode.141.dylib`v8impl::Reference::WeakCallback:
->  0x103fd7668 <+36>: ldr    x1, [x8, #0x30]
    0x103fd766c <+40>: mov    x0, x19
    0x103fd7670 <+44>: ldp    x29, x30, [sp, #0x10]
    0x103fd7674 <+48>: ldp    x20, x19, [sp], #0x20
Target 0: (node) stopped.
(lldb) bt
* thread #1, name = 'MainThread', queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x8e4803f0bdb0)
  * frame #0: 0x0000000103fd7668 libnode.141.dylib`v8impl::Reference::WeakCallback(v8::WeakCallbackInfo<v8impl::Reference> const&) + 36
    frame #1: 0x000000010434db80 libnode.141.dylib`v8::internal::GlobalHandles::InvokeFirstPassWeakCallbacks() + 384
    frame #2: 0x00000001043b0da8 libnode.141.dylib`v8::internal::Heap::PerformGarbageCollection(v8::internal::GarbageCollector, v8::internal::GarbageCollectionReason, char const*) + 996
    frame #3: 0x00000001043bf610 libnode.141.dylib`v8::internal::Heap::CollectGarbage(v8::internal::AllocationSpace, v8::internal::GarbageCollectionReason, v8::GCCallbackFlags, v8::internal::PerformHeapLimitCheck)::$_1::operator()() const + 512
    frame #4: 0x00000001043bf3f8 libnode.141.dylib`void heap::base::Stack::SetMarkerAndCallbackImpl<v8::internal::Heap::CollectGarbage(v8::internal::AllocationSpace, v8::internal::GarbageCollectionReason, v8::GCCallbackFlags, v8::internal::PerformHeapLimitCheck)::$_1>(heap::base::Stack*, void*, void const*) + 40
    frame #5: 0x00000001049e31d8 libnode.141.dylib`PushAllRegistersAndIterateStack + 40
    frame #6: 0x00000001043addc4 libnode.141.dylib`v8::internal::Heap::CollectGarbage(v8::internal::AllocationSpace, v8::internal::GarbageCollectionReason, v8::GCCallbackFlags, v8::internal::PerformHeapLimitCheck) + 436
    frame #7: 0x00000001043af1bc libnode.141.dylib`v8::internal::Heap::HandleExternalMemoryInterrupt() + 364
    frame #8: 0x00000001042172bc libnode.141.dylib`v8::Isolate::AdjustAmountOfExternalAllocatedMemoryImpl(long long) + 104
    frame #9: 0x0000000103fe0a24 libnode.141.dylib`napi_adjust_external_memory + 36
    frame #10: 0x0000000101349f80 canvas.node`Canvas::ensureSurface() [inlined] Napi::MemoryManagement::AdjustExternalMemory(env=<unavailable>, change_in_bytes=<unavailable>) at napi-inl.h:6977:7 [opt]
    frame #11: 0x0000000101349f78 canvas.node`Canvas::ensureSurface(this=0x0000600003d90b40) at Canvas.cc:988:5 [opt]
    frame #12: 0x00000001013494f4 canvas.node`Canvas::Canvas(this=0x0000600003d90b40, info=0x000000016fdfd5e8) at Canvas.cc:133:48 [opt]
    frame #13: 0x0000000101354dbc canvas.node`Napi::ObjectWrap<Canvas>::ConstructorCallbackWrapper(napi_env__*, napi_callback_info__*)::'lambda0'()::operator()() const [inlined] Canvas::Canvas(this=0x0000600003d90b40, info=<unavailable>) at Canvas.cc:85:98 [opt]
    frame #14: 0x0000000101354db4 canvas.node`Napi::ObjectWrap<Canvas>::ConstructorCallbackWrapper(this=<unavailable>)::'lambda0'()::operator()() const at napi-inl.h:5237:23 [opt]
    frame #15: 0x0000000101354830 canvas.node`Napi::ObjectWrap<Canvas>::ConstructorCallbackWrapper(napi_env__*, napi_callback_info__*) [inlined] napi_value__* Napi::details::WrapCallback<Napi::ObjectWrap<Canvas>::ConstructorCallbackWrapper(napi_env__*, napi_callback_info__*)::'lambda0'()>((null)=<unavailable>, callback=(unnamed class) @ 0x000000016fdfd6b0) at napi-inl.h:113:10 [opt]
    frame #16: 0x0000000101354828 canvas.node`Napi::ObjectWrap<Canvas>::ConstructorCallbackWrapper(env=<unavailable>, info=0x000000016fdfd740) at napi-inl.h:5235:24 [opt]
    frame #17: 0x0000000103fe0d8c libnode.141.dylib`v8impl::(anonymous namespace)::FunctionCallbackWrapper::Invoke(v8::FunctionCallbackInfo<v8::Value> const&) + 80
    frame #18: 0x000000010423c664 libnode.141.dylib`v8::internal::FunctionCallbackArguments::CallOrConstruct(v8::internal::Tagged<v8::internal::FunctionTemplateInfo>, bool) + 276
    frame #19: 0x000000010423c23c libnode.141.dylib`v8::internal::MaybeHandle<v8::internal::Object> v8::internal::(anonymous namespace)::HandleApiCallHelper<true>(v8::internal::Isolate*, v8::internal::DirectHandle<v8::internal::HeapObject>, v8::internal::DirectHandle<v8::internal::FunctionTemplateInfo>, v8::internal::DirectHandle<v8::internal::Object>, unsigned long*, int) + 392
    frame #20: 0x000000010423bc88 libnode.141.dylib`v8::internal::Builtin_HandleApiConstruct(int, unsigned long*, v8::internal::Isolate*) + 140
    frame #21: 0x0000000103e29394 libnode.141.dylib`Builtins_CEntry_Return1_ArgvOnStack_BuiltinExit + 84
    frame #22: 0x0000000103d89f90 libnode.141.dylib`Builtins_InterpreterPushArgsThenFastConstructFunction + 752
    frame #23: 0x0000000103f19020 libnode.141.dylib`Builtins_ConstructHandler + 864
    frame #24: 0x0000000103d895ec libnode.141.dylib`Builtins_InterpreterEntryTrampoline + 268
    frame #25: 0x0000000103d895ec libnode.141.dylib`Builtins_InterpreterEntryTrampoline + 268
    frame #26: 0x0000000103e40fa4 libnode.141.dylib`Builtins_ArrayForEach + 804
    frame #27: 0x0000000103d895ec libnode.141.dylib`Builtins_InterpreterEntryTrampoline + 268
    frame #28: 0x0000000103d895ec libnode.141.dylib`Builtins_InterpreterEntryTrampoline + 268
    frame #29: 0x0000000103d895ec libnode.141.dylib`Builtins_InterpreterEntryTrampoline + 268
    frame #30: 0x0000000103d895ec libnode.141.dylib`Builtins_InterpreterEntryTrampoline + 268
    frame #31: 0x0000000103d895ec libnode.141.dylib`Builtins_InterpreterEntryTrampoline + 268
    frame #32: 0x0000000103d895ec libnode.141.dylib`Builtins_InterpreterEntryTrampoline + 268
    frame #33: 0x0000000103d895ec libnode.141.dylib`Builtins_InterpreterEntryTrampoline + 268
    frame #34: 0x0000000103d895ec libnode.141.dylib`Builtins_InterpreterEntryTrampoline + 268
    frame #35: 0x0000000103d895ec libnode.141.dylib`Builtins_InterpreterEntryTrampoline + 268
    frame #36: 0x0000000103d895ec libnode.141.dylib`Builtins_InterpreterEntryTrampoline + 268
    frame #37: 0x0000000103d8696c libnode.141.dylib`Builtins_JSEntryTrampoline + 172
    frame #38: 0x0000000103d86610 libnode.141.dylib`Builtins_JSEntry + 176
    frame #39: 0x0000000104317df0 libnode.141.dylib`v8::internal::(anonymous namespace)::Invoke(v8::internal::Isolate*, v8::internal::(anonymous namespace)::InvokeParams const&) + 1544
    frame #40: 0x00000001043177d4 libnode.141.dylib`v8::internal::Execution::Call(v8::internal::Isolate*, v8::internal::DirectHandle<v8::internal::Object>, v8::internal::DirectHandle<v8::internal::Object>, v8::base::Vector<v8::internal::DirectHandle<v8::internal::Object> const>) + 88
    frame #41: 0x0000000104f2272c libnode.141.dylib`v8::Function::Call(v8::Isolate*, v8::Local<v8::Context>, v8::Local<v8::Value>, int, v8::Local<v8::Value>*) + 172
    frame #42: 0x0000000103fff560 libnode.141.dylib`node::builtins::BuiltinLoader::CompileAndCall(v8::Local<v8::Context>, char const*, node::Realm*) + 460
    frame #43: 0x0000000104eb83c4 libnode.141.dylib`node::Realm::ExecuteBootstrapper(char const*) + 76
    frame #44: 0x0000000104e7bc84 libnode.141.dylib`node::StartExecution(node::Environment*, char const*) (.cold.1) + 28
    frame #45: 0x0000000103fea608 libnode.141.dylib`node::StartExecution(node::Environment*, char const*) + 60
    frame #46: 0x0000000103fea5a0 libnode.141.dylib`node::StartExecution(node::Environment*, std::__1::function<v8::MaybeLocal<v8::Value> (node::StartExecutionCallbackInfo const&)>) + 1492
    frame #47: 0x0000000103f66c4c libnode.141.dylib`node::LoadEnvironment(node::Environment*, std::__1::function<v8::MaybeLocal<v8::Value> (node::StartExecutionCallbackInfo const&)>, std::__1::function<void (node::Environment*, v8::Local<v8::Value>, v8::Local<v8::Value>)>) + 204
    frame #48: 0x0000000104048950 libnode.141.dylib`node::NodeMainInstance::Run(node::ExitCode*, node::Environment*) + 92
    frame #49: 0x00000001040486e0 libnode.141.dylib`node::NodeMainInstance::Run() + 152
    frame #50: 0x0000000103fed5c8 libnode.141.dylib`node::Start(int, char**) + 612
    frame #51: 0x0000000198ebeb98 dyld`start + 6076

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Run node repro.js from the sync-gc branch and compare the failure with napi-inl.h:6977, napi_adjust_external_memory, and the LLDB stack trace. Trace whether the basic_env call reaches JavaScript during GC, then document or test the expected behavior so the reproducer no longer crashes.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp, node.js
Domain
api
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.