goingnative icon indicating copy to clipboard operation
goingnative copied to clipboard

Running "Offloading The Work" fails w/ anonymous function

Open johnnyman727 opened this issue 10 years ago • 12 comments

I'm working on the final tutorial, "Offloading The Work".

My solution fails if my delay is invoked as follows with provided code:

addon.delay(process.argv[2], function() {
  clearInterval(interval)
  console.log('Done!')
})

By making the callback non-anonymous, the solution passes:

addon.delay(process.argv[2], function callback() {
  clearInterval(interval)
  console.log('Done!')
})

johnnyman727 avatar Jul 05 '14 19:07 johnnyman727

Actually, I lied. It seems to be a race condition (surprise)!

It will inconsistently pass or fail regardless of whether the callback is named.

johnnyman727 avatar Jul 05 '14 19:07 johnnyman727

ok, so can you paste your native addon code here and are you using NanCallback to wrap your Function object when passing it to async?

rvagg avatar Jul 05 '14 20:07 rvagg

Hi @rvagg,

Yes, I'm wrapping my callback in a NanCallback to keep it from being gc'ed.

Here is a copy of my addon.cc

#include <nan.h>

// at the top of your file
#ifndef _WIN32
#include <unistd.h>
#endif

using namespace v8;
class MyWorker : public NanAsyncWorker {
 public:
  // Constructor
  MyWorker(NanCallback *callback, int delay)
    : NanAsyncWorker(callback), delay(delay) {}
  // Destructor
  ~MyWorker() {}

  // Executed inside the worker-thread.
  // It is not safe to access V8, or V8 data structures
  // here, so everything we need for input and output
  // should go on `this`.
  void Execute () {
    // Asynchronous, non-V8 work goes here
    #ifdef _WIN32
    Sleep(delay);
    #else
    usleep(delay * 1000);
    #endif
  }

  // Executed when the async work is complete
  // this function will be run inside the main event loop
  // so it is safe to use V8 again
  void HandleOKCallback () {
    NanScope();

    // NanCallback#Call() does a NanMakeCallback() for us
    callback->Call(0, NULL);
  }

 private:
  int delay;
};

NAN_METHOD(Delay) {
  NanScope();

  // get delay and callback
  Local<Number> num = args[0].As<Number>();
  Local<Function> callback = args[1].As<Function>();

  // create NanCallback instance wrapping the callback
  NanCallback *protectedCallback = new NanCallback(callback);

  // create a MyWorker instance, passing the callback and delay
  // MyWorker
  MyWorker *worker = new MyWorker(protectedCallback, num->IntegerValue());

  // queue the worker instance onto the thread-pool
  NanAsyncQueueWorker(worker);

  NanReturnUndefined();
}

NAN_METHOD(Sleep) {

  Local<Number> num = args[0].As<Number>();
  Local<Function> callback = args[1].As<Function>();

  #ifdef _WIN32
  Sleep(num->IntegerValue());
  #else
  usleep(num->IntegerValue() * 1000);
  #endif

  NanMakeCallback(NanGetCurrentContext()->Global(), callback, 0, NULL);

  NanReturnUndefined();
}

NAN_METHOD(Length) {
  NanScope();
  Local<String> str = args[0].As<String>();
  int len = strlen(*String::Utf8Value(str));
  Local<Number> num = NanNew<Number>(len);
  NanReturnValue(num);
}

NAN_METHOD(Print) {
  Local<String> str = args[0].As<String>();
  printf("%s\n", *String::Utf8Value(str));
  NanReturnUndefined();
}

void Init(Handle<Object> exports) {
  exports->Set(NanNew("print"), NanNew<FunctionTemplate>(Print)->GetFunction());
  exports->Set(NanNew("length"), NanNew<FunctionTemplate>(Length)->GetFunction());
  exports->Set(NanNew("sleep"), NanNew<FunctionTemplate>(Sleep)->GetFunction());
  exports->Set(NanNew("delay"), NanNew<FunctionTemplate>(Delay)->GetFunction());

}

NODE_MODULE(myaddon, Init);

johnnyman727 avatar Jul 05 '14 23:07 johnnyman727

Strange, I don't see any errors, apart from NODE_MODULE(myaddon, Init); having a semicolon, it should not have that -- but I don't think this is impacting the runtime behavior.

kkoopa avatar Jul 09 '14 09:07 kkoopa

@kkoopa, are you able to repro?

johnnyman727 avatar Jul 10 '14 00:07 johnnyman727

@johnnyman727 what version of Node is this? node -v

rvagg avatar Jul 10 '14 04:07 rvagg

v0.10.26

johnnyman727 avatar Jul 10 '14 04:07 johnnyman727

@johnnyman727 try this and see if it changes anything:

addon.delay(process.argv[2], function() {
  clearInterval(interval)
  console.log('Done!')
  addon.foo = 'bar'
})

rvagg avatar Jul 10 '14 04:07 rvagg

@rvagg unfortunately, it still passes inconsistently.

johnnyman727 avatar Jul 10 '14 04:07 johnnyman727

@johnnyman727 as in, it sometimes passes and sometimes fails?

rvagg avatar Jul 10 '14 05:07 rvagg

Yeah, it will fail somewhere around 9 times out of 10.

johnnyman727 avatar Jul 10 '14 05:07 johnnyman727

Similar problem here, running under MSYS:

exercise.js(89) expect = /FAUX 1\nFAUX 2\nWaiting._FAUX 3\n.._FAUX 4\nDone!\n/m My output (and that of the original solution) is: FAUX 1\nFAUX 2\nFAUX 3\nWaiting..FAUX 4\nDone!

Giving the NanReturn a little more time to resume javascript execution, fixed this for me: goingnative\exercises\offloading_the_work\faux\myaddon.cc(21) : Sleep(100);

I also had to go full kirk on the kobayashi maru/"AmIReady?"-test by adding: exercise.js(163): , 2010: 'VS100COMNTOOLS'

Thanks for the great tutorial! This way I also learned a lot about the global npm- and node-module directory ;-)

dsewtz avatar Jan 30 '15 12:01 dsewtz