3

I am writing C++ addon using nbind - GitHub link for most thing and Nan - GitHub link for calling callbacks asynchronous. When I invoke callback only once, it works perfect. But When I invoke callback twice it gives Segmentation fault (core dumped). Couldn't find error using gdb. Here is JS and C++ codes(compiling using node-gyp configure build):

//main.js code
var nbind = require('nbind');
var lib = nbind.init().lib;

lib.HeaderExample.callJS(function(a) {
console.log("result" + a);
});

lib.HeaderExample.startThread();
lib.HeaderExample.startThread(); 

C++ addon's code

//c++ code
class CallbackRunner : public Nan::AsyncWorker {
public:
    CallbackRunner(Nan::Callback *callback)
            : AsyncWorker(callback) {}
    void Execute () {}
    void HandleOKCallback () {
        std::cout << "running HandleOKCallback in thread " << std::this_thread::get_id() << std::endl;
        Nan::HandleScope scope;
        v8::Local<v8::Value> argv[] = {
                Nan::New<v8::Number>(10)
        };
        callback->Call(1, argv);
    }
};

class HeaderExample {
public:
    static void callJS(nbind::cbFunction &callback) {
        std::cout << "running callJS in thread " << std::this_thread::get_id() << std::endl;
        m_onInitialisationStarted = new nbind::cbFunction(callback);
        Nan::Callback *callbackNan = new Nan::Callback(m_onInitialisationStarted->getJsFunction());
        runner = new CallbackRunner(callbackNan);
    }
    static void startThread() {
        std::cout << "it is here";
        std::thread threadS(some);
        threadS.join();
    }
    static void some() {
        std::cout << "running some in thread: " << std::this_thread::get_id() << std::endl;
        if(runner){
        AsyncQueueWorker(runner);
        }
    }
    inline static nbind::cbFunction *m_onInitialisationStarted = 0;
    inline static CallbackRunner *runner;
};
simon-p-r
  • 3,623
  • 2
  • 20
  • 35
Jasurbek Nabijonov
  • 1,607
  • 3
  • 24
  • 37

2 Answers2

4

Your class uses AsyncQueueWorker to invoke the CallbackRunner, but AsyncQueueWorker calls AsyncExecuteComplete after the callback is done, which in turn calls worker->Destroy(). See the AsyncQueueWorker code from nan.h:

inline void AsyncExecute (uv_work_t* req) {
  AsyncWorker *worker = static_cast<AsyncWorker*>(req->data);
  worker->Execute();
}

inline void AsyncExecuteComplete (uv_work_t* req) {
  AsyncWorker* worker = static_cast<AsyncWorker*>(req->data);
  worker->WorkComplete();
  worker->Destroy();
}

inline void AsyncQueueWorker (AsyncWorker* worker) {
  uv_queue_work(
      uv_default_loop()
    , &worker->request
    , AsyncExecute
    , reinterpret_cast<uv_after_work_cb>(AsyncExecuteComplete)
  );
}

worker->Destroy() will delete the CallbackRunner class, along with the Nan::Callback that you fed to its constructor. That is the reason why you get a segmentation fault when attempting to call this callback a second time.

You might be better off basing your class on Nan::AsyncProgressQueueWorker instead of Nan::AsyncWorker. AsyncProgressQueueWorker inherits AsyncWorker and it allows you to schedule work from the main thread just as AsyncWorker does, but it provides you with an ExecutionProgress class that allows you to use any thread to call back into the main thread any number of times while the original scheduled job is running.

Nan::AsyncProgressQueueWorker was added to NAN in version 2.8.0

mkrufky
  • 3,268
  • 2
  • 17
  • 37
1

You can't have two threads calling into the same V8 instance concurrently. You'll need careful locking to make sure that only one thread interacts with V8 at any point in time.

jmrk
  • 34,271
  • 7
  • 59
  • 74
  • Hi! Should I add lockers? – Jasurbek Nabijonov Aug 09 '17 at 17:44
  • Yes, Lockers are one way to achieve this. (Another question is whether having several threads is helpful for your use case at all; or whether having all these threads do their own callbacks into V8 is the most elegant or performant solution. Since you have provided no further details I cannot give any advice there.) – jmrk Aug 09 '17 at 17:51
  • Thank you for you advice :) This example is only simulation of what my real program does. It is (`in c++`) call callbacks in its own thread during execution that should be invoked in JavaScript. What kind of details will be needed for clarification? I will be happy to share it ;) – Jasurbek Nabijonov Aug 09 '17 at 18:01
  • Well, depending on what you do, you might not need any threading at all. Or it might be easiest to structure the C++ side of your program such that you have one "main" thread that interacts with V8 and manages a pool of worker threads. Or, of course you can use Lockers, there's nothing wrong with that. – jmrk Aug 09 '17 at 18:07
  • Yeah, it would be nice. Very good idea. Is there any example of it? It would great to see some examples. – Jasurbek Nabijonov Aug 09 '17 at 18:12
  • If I remove `std::thread threadS(some); threadS.join();` there is `Segmentation fault (core dumped)`. I think I do something wrong :) – Jasurbek Nabijonov Aug 09 '17 at 18:17
  • What if c++ created and it uses `AsyncQueueWorker`, than it leads to `Segmentation fault (core dumped)`. Should I use something to avoid such situation? :) – Jasurbek Nabijonov Aug 10 '17 at 13:23