Calling a callback from a native addon asynchronously #1247

Activity

1. Fishrock123

   added

   c++Issues and PRs that require attention from people who are familiar with C++.Issues and PRs that require attention from people who are familiar with C++.

   on Mar 24, 2015
2. justinmchase

   changed the title [-]Calling a callback from a native module asynchronously[/-] [+]Calling a callback from a native addon asynchronously[/+] on Mar 24, 2015
3. trevnorris commented on Mar 24, 2015

   

   trevnorris

   on Mar 24, 2015

   Contributor

   More actions

   Have a small test case we can help you with?
4. justinmchase commented on Mar 25, 2015

   

   justinmchase

   on Mar 25, 2015

   Author

   More actions

   Thanks for the reply. I have been working on getting the repro down to the simplest form and I believe it is not actually an iojs bug :)

   It turns out we are actually using http://nwjs.io, which runs with a particular build of iojs. When I run my repro from the commandline iojs it works fine, from inside of nwjs it crashes in the async callback after a few milliseconds. Meaning it works for a little while but it seems like the Isolate or Context is being garbage collected out from under us.

   But anyway, I do have another question. How are people actually building native modules for iojs? In node you would use node-gyp, but that doesn't appear to actually build iojs compatible binaries. I had to fork and alter node-gyp to get it to work with iojs so it leaves me wondering how others are actually building their modules.

   Also, I'm wondering if I'm doing it wrong and it just happens to work from the command line version accidentally? Here is my code I would appreciate any feedback:

   #include <node.h>
   #include "async.h"
   
   using namespace v8;
   
   static Persistent<Function, CopyablePersistentTraits<Function>> _cb;
   Async* _async;
   
   void AsyncCallback(void* data)
   {
       int x = *((int*)data);
       auto isolate = Isolate::GetCurrent();
       auto context = isolate->GetCurrentContext(); // crashes nwjs here
       auto global = context->Global();
   
       const int argc = 1;
       Handle<Value> argv[argc];
       argv[0] = Number::New(isolate, (double)x);
   
       auto fn = Local<Function>::New(isolate, _cb);
       fn->Call(global, argc, argv);
   }
   
   DWORD Worker(LPVOID lparam)
   {
       int x = 0;
       while(true) 
       {
           _async->send(&x);
           x++;
           Sleep(1000);
       }
       return S_OK;
   }
   
   void Start(const FunctionCallbackInfo<Value>& args)
   {
       auto isolate = Isolate::GetCurrent();
   
       Handle<Function> arg0 = Handle<Function>::Cast(args[0]);
       Persistent<Function> cb(isolate, arg0);
       _cb = cb;
   
       _async = new Async(&AsyncCallback);
       CreateThread(nullptr, 0, Worker, nullptr, 0, 0);
   }
   
   void init(Handle<Object> exports, Handle<Object> module)
   {
       NODE_SET_METHOD(exports, "start", Start);
   }
   
   NODE_MODULE(evolve_overlay, init)

   The Async class is very similar to this one:
   https://github.com/mapbox/node-sqlite3/blob/master/src/async.h

   Then to call it from iojs:

   $ iojs
   > require('./build/Debug/test').start(function (x) {
   >    console.log(x)
   > }
   

   This works as expected, but the exact same module compiled with nw-gyp running in nwjs fails in the function AsyncCallback after a few milliseconds. I will follow up with them there but I'm especially wondering about the call to isolate-GetCurrentContext(). Is this the right way to get the isolate and context when coming from an async callback?

   Thanks.
5. justinmchase commented on Mar 25, 2015

   

   justinmchase

   on Mar 25, 2015

   Author

   More actions

   I managed to figure it out. After adding a fatal error handler:

   isolate->SetFatalErrorHandler(&OnFatalError);
   

   I managed to get this error out of it:

   Cannot create a handle without a HandleScope
   

   Which lead me to realize I needed to create a HandleScope in my async callback and that is what fixed it.

   int x = *((int*)data);
   auto isolate = Isolate::GetCurrent();
   auto context = isolate->GetCurrentContext(); // crashes nwjs here
   auto global = context->Global();
   

   Should actually be:

   int x = *((int*)data);
   auto isolate = Isolate::GetCurrent();
   HandleScope scope(isolate);
   auto context = isolate->GetCurrentContext(); // no longer crashes
   auto global = context->Global();
   

   Thanks.
6. justinmchase

   closed this as completedon Mar 25, 2015
7. trevnorris commented on Mar 25, 2015

   

   trevnorris

   on Mar 25, 2015

   Contributor

   More actions

   Ah yes. That'll get you every time. Can't forget those HandleScopes :)

   👍React with 👍1Reacted by Nathan Moin Vaziri