EXC_BAD_ACCESS with Node 8.3, works with Node 6.11

I've ported https://github.com/mcollina/native-hdr-histogram/tree/napi to node-addon-api. The tests passes with Node 6.11, but they fail with the following error on Node 8.3.0-rc.0:

* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0xffffffff00000000)
    frame #0: 0x0000000100a58df0 node`(anonymous namespace)::v8impl::FunctionCallbackWrapper::SetReturnValue(napi_value__*) + 16
node`(anonymous namespace)::v8impl::FunctionCallbackWrapper::SetReturnValue:
->  0x100a58df0 <+16>: movq   (%rsi), %rcx
    0x100a58df3 <+19>: movq   %rcx, 0x18(%rax)
    0x100a58df7 <+23>: popq   %rbp
    0x100a58df8 <+24>: retq
(llnode) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0xffffffff00000000)
  * frame #0: 0x0000000100a58df0 node`(anonymous namespace)::v8impl::FunctionCallbackWrapper::SetReturnValue(napi_value__*) + 16
    frame #1: 0x0000000100a5227f node`(anonymous namespace)::v8impl::FunctionCallbackWrapper::Invoke(v8::FunctionCallbackInfo<v8::Value> const&) + 307
    frame #2: 0x0000000100216122 node`v8::internal::FunctionCallbackArguments::Call(void (*)(v8::FunctionCallbackInfo<v8::Value> const&)) + 466
    frame #3: 0x000000010027761c node`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) + 572
    frame #4: 0x0000000100277028 node`v8::internal::Builtin_Impl_HandleApiCall(v8::internal::BuiltinArguments, v8::internal::Isolate*) + 232
    frame #5: 0x00003308176840dd
    frame #6: 0x0000330817689154
    frame #7: 0x000033081777596e
    frame #8: 0x0000330817692f12
    frame #9: 0x0000330817773fa1
    frame #10: 0x0000330817692f12
    frame #11: 0x00003308177738aa
    frame #12: 0x0000330817692f12
    frame #13: 0x0000330817686fbb
    frame #14: 0x000033081777373c
    frame #15: 0x0000330817692f12
    frame #16: 0x00003308177740f2
    frame #17: 0x0000330817692f12
    frame #18: 0x0000330817772fdb
    frame #19: 0x0000330817692f12
    frame #20: 0x0000330817773fa1
    frame #21: 0x0000330817692f12
    frame #22: 0x0000330817686fbb
    frame #23: 0x0000330817773a25
    frame #24: 0x0000330817692f12
    frame #25: 0x00003308177738aa
    frame #26: 0x0000330817692f12
    frame #27: 0x0000330817686fbb
    frame #28: 0x0000330817774242
    frame #29: 0x0000330817692f12
    frame #30: 0x0000330817772cc8
    frame #31: 0x0000330817692f12
    frame #32: 0x00003308177738aa
    frame #33: 0x0000330817692f12
    frame #34: 0x00003308177738aa
    frame #35: 0x0000330817692f12
    frame #36: 0x000033081777373c
    frame #37: 0x0000330817692f12
    frame #38: 0x0000330817774242
    frame #39: 0x0000330817692f12
    frame #40: 0x0000330817772cc8
    frame #41: 0x0000330817692f12
    frame #42: 0x0000330817793c29
    frame #43: 0x0000330817692f12
    frame #44: 0x0000330817773fa1
    frame #45: 0x0000330817692f12
    frame #46: 0x0000330817690650
    frame #47: 0x0000330817764ead
    frame #48: 0x00000001005c5dfd node`v8::internal::(anonymous namespace)::Invoke(v8::internal::Isolate*, bool, v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>, int, v8::internal::Handle<v8::internal::Object>*, v8::internal::Handle<v8::internal::Object>, v8::internal::Execution::MessageHandling) + 701
    frame #49: 0x00000001005c5a9b node`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>*) + 171
    frame #50: 0x00000001001faf29 node`v8::Function::Call(v8::Local<v8::Context>, v8::Local<v8::Value>, int, v8::Local<v8::Value>*) + 441
    frame #51: 0x0000000100a4a7cf node`node::LoadEnvironment(node::Environment*) + 591
    frame #52: 0x0000000100a51832 node`node::Start(v8::Isolate*, node::IsolateData*, int, char const* const*, int, char const* const*) + 426
    frame #53: 0x0000000100a4c88f node`node::Start(uv_loop_s*, int, char const* const*, int, char const* const*) + 462
    frame #54: 0x0000000100a4be83 node`node::Start(int, char**) + 333
    frame #55: 0x0000000100001134 node`start + 52

to reproduce, run node --napi-modules test.js after downloading and building the linked branch: https://github.com/mcollina/native-hdr-histogram/tree/napi

@mcollina

Thanks @mcollina for the report. I tried Windows and Ubuntu, but I'm not seeing that crash. I don't have a Mac handy right now to verify with, but I'll try to track one down.

I did this:

nvm install v8.3.0
npm install
node --napi-modules test.js

So far, it's stuck on "# Subtest: create an histogram arguments checks\nok 1 - expected to throw"

This is on "macOS Sierra Version 10.12.2"

Still stuck ... I'm going to interrupt it.

Yes, and if you run that with lldb, you will get the error above.

Aaah, OK. Will do.

Reproduced.

Built 8.3.0 with debug symbols and it runs successfully LOL

That is, inside lldb.

So it only happens with the official version.

@jasongin

@jasongin the instability may arise from the fact that the constructor for the HdrHistogramWrap class throws an error during its construction. I've noticed that the C++ wrapper doesn't have any tests for ObjectWrap. We should add a test that implements an object wrapping such that the constructor throws a JS error.

I've noticed that the C++ wrapper doesn't have any tests for ObjectWrap.

Yes, I agree test coverage is needed. It just hasn't been a top priority for me so far because ObjectWrap was tested fairly well already via the ported modules that use it (like canvas).

@gabrielschulhof

@gabrielschulhof What you’re saying makes a lot of sense. I think this would be the fix for it:

diff --git a/napi-inl.h b/napi-inl.h
index 4573963422d9..bf64376072c6 100644
--- a/napi-inl.h
+++ b/napi-inl.h
@@ -2428,12 +2428,10 @@ inline napi_value ObjectWrap<T>::ConstructorCallbackWrapper(
   }
 
   T* instance;
-  napi_value wrapper;
-  details::WrapCallback([&] {
+  napi_value wrapper = details::WrapCallback([&] {
     CallbackInfo callbackInfo(env, info);
     instance = new T(callbackInfo);
-    wrapper = callbackInfo.This();
-    return nullptr;
+    return callbackInfo.This();
   });
 
   napi_ref ref;

The code should look like that anyway, I think. In its current state, wrapper would always be unitialized if the constructor threw.

Fix ObjectWrap constructor

Navigation Menu

Repository navigation

EXC_BAD_ACCESS with Node 8.3, works with Node 6.11 #101

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

EXC_BAD_ACCESS with Node 8.3, works with Node 6.11 #101

Description

Activity

kfarnung commented on Aug 8, 2017

gabrielschulhof commented on Aug 12, 2017

gabrielschulhof commented on Aug 12, 2017

gabrielschulhof commented on Aug 12, 2017

mcollina commented on Aug 12, 2017

gabrielschulhof commented on Aug 12, 2017

gabrielschulhof commented on Aug 12, 2017

gabrielschulhof commented on Aug 12, 2017

gabrielschulhof commented on Aug 12, 2017

gabrielschulhof commented on Aug 12, 2017

gabrielschulhof commented on Aug 12, 2017

jasongin commented on Aug 12, 2017

addaleax commented on Aug 12, 2017

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions