Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

n-api segfault in v8impl::Reference::FinalizeCallback #33283

Closed
codebytere opened this issue May 7, 2020 · 9 comments
Closed

n-api segfault in v8impl::Reference::FinalizeCallback #33283

codebytere opened this issue May 7, 2020 · 9 comments
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. node-api Issues and PRs related to the Node-API.

Comments

@codebytere
Copy link
Member

codebytere commented May 7, 2020

  • Version: v14.2.0
  • Platform: Linux vagrant 4.15.0-99-generic #100-Ubuntu SMP Wed Apr 22 20:32:56 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
  • Subsystem: n-api

What steps will reproduce the bug?

$ git clone https://github.com/codebytere/node-cmark-gfm
$ npm i
$ npm test

It will segfault in the extensions portion of the test suite - this does not occur on v12.x so i'm reasonably sure it's a regression. It also does not occur on macOS.

I dug into it a little and surfaced the following stacktrace:

Thread 1 "node" received signal SIGSEGV, Segmentation fault.
0x00000000009e2e4e in v8impl::(anonymous namespace)::Reference::FinalizeCallback(v8::WeakCallbackInfo<v8impl::(anonymous namespace)::Reference> const&) ()
(gdb) bt
#0  0x00000000009e2e4e in v8impl::(anonymous namespace)::Reference::FinalizeCallback(v8::WeakCallbackInfo<v8impl::(anonymous namespace)::Reference> const&) ()
#1  0x0000000000cfbdd0 in v8::internal::GlobalHandles::InvokeSecondPassPhantomCallbacks() [clone .part.122] ()
#2  0x0000000000cfc968 in v8::internal::GlobalHandles::InvokeSecondPassPhantomCallbacksFromTask() ()
#3  0x0000000000c886bb in non-virtual thunk to v8::internal::CancelableTask::Run() ()
#4  0x0000000000a96fb4 in node::PerIsolatePlatformData::RunForegroundTask(std::unique_ptr<v8::Task, std::default_delete<v8::Task> >) ()
#5  0x0000000000a98e19 in node::PerIsolatePlatformData::FlushForegroundTasksInternal() ()
#6  0x00000000013203a9 in uv.async_io.part ()
    at ../deps/uv/src/unix/async.c:150
#7  0x0000000001332810 in uv.io_poll ()
---Type <return> to continue, or q <return> to quit---
    at ../deps/uv/src/unix/linux-core.c:431
#8  0x0000000001320cb8 in uv_run (
    loop=0x4232ea0 <default_loop_struct>, mode=UV_RUN_DEFAULT)
    at ../deps/uv/src/unix/core.c:381
#9  0x0000000000a6a334 in node::NodeMainInstance::Run() ()
#10 0x00000000009f9651 in node::Start(int, char**) ()
#11 0x00007ffff6ca3b97 in __libc_start_main (main=0x9919f0 <main>, 
    argc=6, argv=0x7fffffffddd8, init=<optimized out>, 
    fini=<optimized out>, rtld_fini=<optimized out>, 
    stack_end=0x7fffffffddc8) at ../csu/libc-start.c:310
#12 0x0000000000992f0c in _start ()

How often does it reproduce? Is there a required condition?

This reproduces about 95% of the time.

What is the expected behavior?

For it not to crash.

What do you see instead?

It segfaults.

Additional information

I believe it may be connected to this PR: #31638. I'm also happy to take a crack at this if i could get some pointers on possible underlying cause.

cc @gabrielschulhof

@codebytere codebytere added node-api Issues and PRs related to the Node-API. c++ Issues and PRs that require attention from people who are familiar with C++. labels May 7, 2020
@mhdawson
Copy link
Member

mhdawson commented May 8, 2020

Was that with node and the addon compiled in debug mode? If not this link provides some info on doing that: https://github.com/nodejs/node/blob/master/doc/guides/investigating_native_memory_leak.md#enabling-debug-symbols-to-get-more-information and I'm hoping we might get more info out of the stack.

@gabrielschulhof
Copy link
Contributor

@codebytere can I run the test by hand? Like, node ./something/something/test.js?

@gabrielschulhof
Copy link
Contributor

I was able to run

/node_modules/.bin/mocha test/helper.js test/extensions.test.js

successfully several times in a row. Nevertheless, it reliably dies when running

./node_modules/.bin/mocha test/helper.js test/*.test.js

@codebytere
Copy link
Member Author

@gabrielschulhof that was a strange thing i discovered - it doesn't die in isolation but reliably does so when part of the suite 🤔

@gabrielschulhof
Copy link
Contributor

@codebytere

diff --git a/binding.gyp b/binding.gyp
index a5f9525..38d6607 100644
--- a/binding.gyp
+++ b/binding.gyp
@@ -79,13 +79,13 @@
         ['OS=="win"', {
           'link_settings': {
             'libraries': [
-              '<(module_root_dir)/build/Release/libcmark-gfm.lib'
+              '<(PRODUCT_DIR)/libcmark-gfm.lib'
             ]
           }
         }, {
           'link_settings': {
             'libraries': [
-              '<(module_root_dir)/build/Release/cmark-gfm.a'
+              '<(PRODUCT_DIR)/cmark-gfm.a'
             ]
           }
         }]

makes npm i --debug possible.

@gabrielschulhof
Copy link
Contributor

@codebytere it won't crash with Node.js 14.2.0 in debug mode with the package built in debug mode 🙁

I'm wondering if it's #33508 ...

@gabrielschulhof
Copy link
Contributor

@codebytere FWIW it doesn't crash as of 5536044.

@gabrielschulhof
Copy link
Contributor

@codebytere I ran a bisect and it looks like 8f10bb2 fixed it.

@codebytere
Copy link
Member Author

Oh rad! thanks @gabrielschulhof 🙇🏻‍♀️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. node-api Issues and PRs related to the Node-API.
Projects
None yet
Development

No branches or pull requests

3 participants