Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Persistant callback not working #854

Open
FadyQJ opened this issue May 6, 2019 · 6 comments
Open

Persistant callback not working #854

FadyQJ opened this issue May 6, 2019 · 6 comments

Comments

@FadyQJ
Copy link

FadyQJ commented May 6, 2019

In my c++ library I have the following:

typedef void (update_callback)(LibClass *data);
void setCallback(update_callback *callback) {
  this->callback = callback;
}

Using nan, I am assigning the persistent function to be called from js using a setUpdateCallback function e.g.:

myClass.setUpdateCallback(function() { console.log("Hello"); })

setUpdateCallback calls this nan function:

void MyClass::SetUpdateCallback(const Nan::FunctionCallbackInfo<v8::Value>& info) {

  // get the object
  MyClass *obj = ObjectWrap::Unwrap<MyClass>(info.Holder());

  // I have tried a million combinations, this one is the latest try
  v8::Isolate *isolate = v8::Isolate::GetCurrent();
  v8::Handle<v8::Function> callback = v8::Handle<v8::Function>::Cast(info[0]);
  v8::Persistent<v8::Function> callback_persistent(isolate, callback);
  obj->cb = callback_persistent;

  // assign the callback, lib here is the custom class that the Nan::ObjectWrap contains as a variable
  obj->lib->setCallback(MyClass::Update);

  // return the class, this can be ommited
  info.GetReturnValue().Set(info.This());

}

And the MyClass::Update does the followng:

void MyClass::Update(LibClass *libClass) {

  // get the obj related to the current LibClass
  // libs here is a map with pair<LibClass *, MyClass *>
  MyClass *parent = libs.at(libClass);

  // create callback
  v8::Isolate *isolate = v8::Isolate::GetCurrent();
  v8::HandleScope scope(isolate);
  v8::Local<v8::Function> callback = v8::Local<v8::Function>::New(isolate, parent->cb);

  Nan::Call(callback, Nan::GetCurrentContext()->Global(), 0, NULL);
  // calling the callback twice works successfully
  Nan::Call(callback, Nan::GetCurrentContext()->Global(), 0, NULL);

}

The libClass does not produce a callback, MyClass should set the callback function to call, and throughout the program's execution, libClass will call the update function at random times based on some data.

So what I want to do is to set that callback function in javascript and be able to call it whenever I want from the libClass but it never works. It works on initialization and after that it seems that it is being called by the garbage collector even though it's a persistent function.

I am new to both C++ and Nan and none of the solutions found on the internet were helpful, they were all implementations of callbacks, so am I missing something ?

@bnoordhuis
Copy link
Member

You're using v8::Persistent directly so this isn't really a nan issue. With that in mind, note here:

v8::Persistent<v8::Function> callback_persistent(isolate, callback);
obj->cb = callback_persistent;

...that v8::Persistent has ill-defined copy semantics - ill-defined in the sense that different V8 versions behave differently.

A good first step would be to start using Nan::Callback.

@FadyQJ
Copy link
Author

FadyQJ commented May 6, 2019

Yes sorry for the confusion, I tried Nan::Callback but that too didn't work.
Replacing this code:

// I have tried a million combinations, this one is the latest try
v8::Isolate *isolate = v8::Isolate::GetCurrent();
v8::Handle<v8::Function> callback = v8::Handle<v8::Function>::Cast(info[0]);
v8::Persistent<v8::Function> callback_persistent(isolate, callback);
obj->cb = callback_persistent;

With this:

obj->cb = new Nan::Callback(info[0].As<v8::Function>());

And in the update function, replacing this chunk:

// create callback
v8::Isolate *isolate = v8::Isolate::GetCurrent();
v8::HandleScope scope(isolate);
v8::Local<v8::Function> callback = v8::Local<v8::Function>::New(isolate, parent->cb);

Nan::Call(callback, Nan::GetCurrentContext()->Global(), 0, NULL);
// calling the callback twice works successfully
Nan::Call(callback, Nan::GetCurrentContext()->Global(), 0, NULL);

With this:

Nan::Call(*parent->cb, Nan::GetCurrentContext()->Global(), 0, NULL);
Nan::Call(*parent->cb, Nan::GetCurrentContext()->Global(), 0, NULL);

Yielded the same results. If it is of any help, I am using electron and at the first call of the callback, in the console I see that console.log("Hello"); has been called twice. But after almost 6 seconds, on the second call for MyClass::Update the electron application hangs (I get that the devTool has been disconnected) which implies that the addon has crashed or overloaded the app. The expected behavior is seeing console.log("Hello"); called twice again (total of 4 calls).

I have no idea yet how to debug nan with electron using sublime, but if this problem is not related to Nan, then sorry for wasting your time. But the callback worked at first so it is most probably caused by the garbage collector. And if the problem is not clear at a first glance I will research on how to debug nan using electron and post back some logs.

@bnoordhuis
Copy link
Member

Electron is a big and complex program. Is there any chance you can test with plain Node.js to see if it reproduces there?

@FadyQJ
Copy link
Author

FadyQJ commented May 7, 2019

I will try debugging this using Node.js, I really admire the work you guys put in on open source projects that everyone can benefit from, but if it will take some time I'll stop.

I'm at a certain time schedule so I can't devote a lot of time to debug this issue, but since it is a deeper problem than I initially expected, I will use a workaround and call the update function on each electron tick. That way it behaves the same way and without any issues.

@sergivillar
Copy link

hi @FadyQJ did you solve this issue? I'm facing something very similar

@FadyQJ
Copy link
Author

FadyQJ commented Nov 30, 2019

I solved it by adding the code to an update function that runs every 100ms.
Instead of making the callback, I used multiple boolean variables that would turn true every time a callback is needed.
Once the update function is called, it checks for these booleans, if true it makes the call and resets it to false.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants