2

I'm making a NodeJS addon using the Nan library, and I'm running into an issue where calling a callback (created on the javascript side and passed to the addon to be executed asyncronously) will cause a segfault - but only about once every 10 thousand or so runs.

There's quite a bit of complexity involved in how everything operates, but I'm hoping that someone will see something I missed or be able to figure out what's going on.

The C++ callback function is created from the javascript callback like this:

   auto nodeFunc = val.As<v8::Function>();
   auto nodeCb   = std::make_shared<Nan::Callback>(nodeFunc);

   auto callback = [nodeCb] (std::string err, std::string val) -> void {
        Nan::HandleScope     scope;
        v8::Local<v8::Value> argv[2];

       if (err.length() == 0) {
            auto isolate = v8::Isolate::GetCurrent();
            auto json    = v8::JSON::Parse(isolate, Nan::New(val).ToLocalChecked());
            auto object  = json.ToLocalChecked();
            argv[0] = Nan::Null();
            argv[1] = object;
        } else {
            argv[0] = Nan::Error(err.c_str());
            argv[1] = Nan::Null();
        }

        try {
            nodeCb->Call(2, argv);
        } catch (std::exception& ex) {
            std::cout << ex.what() << std::endl;
            Nan::ThrowReferenceError(ex.what());
        }
    };

After creation, it is passed to a separate thread which eventually sends the callback to the main libuv thread using uv_async_send() and executes it. This works fine the vast majority of the time, but will very rarely segfault on the nodeCb->Call(2, argv) line.

If anyone has any insight into what's happening here, I'd really appreciate it.

Also, here's the call stack from gdb in case that's any help:

#0  0x00000000009870e0 in v8::Function::Call(v8::Local<v8::Value>, int, v8::Local<v8::Value>*) ()
#1  0x00000000010a562c in node::MakeCallback(node::Environment*, v8::Local<v8::Value>, v8::Local<v8::Function>, int, v8::Local<v8::Value>*) ()
#2  0x00000000010a5a98 in node::MakeCallback(v8::Isolate*, v8::Local<v8::Object>, v8::Local<v8::Function>, int, v8::Local<v8::Value>*) ()
#3  0x00007ffff47b4b30 in Nan::Callback::Call_ (this=0x20c3500, isolate=0x1ded750, target=..., argc=2,
    argv=0x7fffffffa430) at ../node_modules/nan/nan.h:1477
#4  0x00007ffff47b4a93 in Nan::Callback::Call (this=0x20c3500, argc=2, argv=0x7fffffffa430)
    at ../node_modules/nan/nan.h:1443
#5  0x00007ffff47b194b in detail::info::__lambda1::operator() (__closure=0x1e40710, err="",
    val="{\"index\":1,\"status\":1}") at ../node/utils.hpp:125
#6  0x00007ffff47b37f2 in std::_Function_handler<void(std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::basic_string<char, std::char_traits<char>, std::allocator<char> >), detail::info::setElementValue(T&, v8::Local<v8::Value>, size_t) [with T = std::function<void(std::basic_string<char>, std::basic_string<char>)>; size_t = long unsigned int]::__lambda1>::_M_invoke(const std::_Any_data &, std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::basic_string<char, std::char_traits<char>, std::allocator<char> >) (__functor=..., __args#0="", __args#1="")
    at /usr/include/c++/4.8.2/functional:2071
#7  0x00007ffff44cd339 in std::function<void (std::string, std::string)>::operator()(std::string, std::string) const (this=0x1e29c80, __args#0="", __args#1="")
    at /opt/rh/devtoolset-4/root/usr/include/c++/5.2.1/functional:2271
#8  0x00007ffff44e172c in std::_Bind<std::function<void (std::string, std::string)> (char const*, std::string)>::__call<void, , 0ul, 1ul>(std::tuple<>&&, std::_Index_tuple<0ul, 1ul>) (this=0x1e29c80,
    __args=<unknown type in /usr/local/lib/libSCPlay.so, CU 0x0, DIE 0x83e21>)
    at /opt/rh/devtoolset-4/root/usr/include/c++/5.2.1/functional:1074
#9  0x00007ffff44daec8 in std::_Bind<std::function<void (std::string, std::string)> (char const*, std::string)>::operator()<, void>() (this=0x1e29c80)
    at /opt/rh/devtoolset-4/root/usr/include/c++/5.2.1/functional:1133
#10 0x00007ffff44d3b58 in std::_Function_handler<void (), std::_Bind<std::function<void (std::string, std::string)> (char const*, std::string)> >::_M_invoke(std::_Any_data const&) (__functor=...)
    at /opt/rh/devtoolset-4/root/usr/include/c++/5.2.1/functional:1871
#11 0x00007ffff44fab0a in std::function<void ()>::operator()() const (this=0x7fffffffa650)
    at /opt/rh/devtoolset-4/root/usr/include/c++/5.2.1/functional:2271
#12 0x00007ffff44f890c in DeviceThread::asyncListener (handle=0x1efb9f0)
    at /home/scl37510/Projects/SCPlay2/lib/device_thread.cpp:124
#13 0x0000000001316b0b in uv__async_event (loop=0x1de7fe0 <default_loop_struct>, w=<optimized out>,
    nevents=<optimized out>) at ../deps/uv/src/unix/async.c:98
#14 0x0000000001316be3 in uv__async_io (loop=0x1de7fe0 <default_loop_struct>,
    w=0x1de81a8 <default_loop_struct+456>, events=<optimized out>) at ../deps/uv/src/unix/async.c:138
#15 0x00000000013271b0 in uv__io_poll (loop=loop@entry=0x1de7fe0 <default_loop_struct>, timeout=0)
    at ../deps/uv/src/unix/linux-core.c:380
#16 0x00000000013176c6 in uv_run (loop=0x1de7fe0 <default_loop_struct>, mode=UV_RUN_ONCE)
    at ../deps/uv/src/unix/core.c:354
#17 0x00000000010aabe0 in node::Start(int, char**) ()
#18 0x00007ffff6bf5b35 in __libc_start_main () from /lib64/libc.so.6
#19 0x00000000007b1f1d in _start ()

Edit: I've created a much smaller test program to see if I could pinpoint the source of the bug, and I've discovered that I can prevent it by changing the Callback shared_ptr to a regular pointer, deleting it immediately after the nodeCb->Call(2,argv) line.

Is there some semantic difference between the two that could cause this?

simon-p-r
  • 3,623
  • 2
  • 20
  • 35
ChaseC
  • 121
  • 5

1 Answers1

1

The usage of shared_ptr to wrap a Callback is suspicious:

std::make_shared<Nan::Callback>(nodeFunc);

I don't think V8 would like references being handled that way.

Nan::Callback itself contains a Persistent that it uses to store the function's value, so you need not worry about persistence.

...I just noticed your edit after I already wrote out this answer. The shared_ptr is potentially dangerous to mix with V8 internal handles and references.

If your intention is to store the callback and delete it immediately, maybe you could benefit by refactoring your code to use Nan::AsyncWorker instead.

mkrufky
  • 3,268
  • 2
  • 17
  • 37