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

Callbacks cannot be copied on the v8 codepath #52

Open
arcanis opened this issue Dec 18, 2016 · 6 comments
Open

Callbacks cannot be copied on the v8 codepath #52

arcanis opened this issue Dec 18, 2016 · 6 comments

Comments

@arcanis
Copy link
Contributor

arcanis commented Dec 18, 2016

The following lines do not compile with the v8 codepath:

nbind::cbFunction fn; // no matching function for call to ‘nbind::cbFunction::cbFunction()’
fn = otherFnFunction; // use of deleted function ‘nbind::cbFunction& nbind::cbFunction::operator=(const nbind::cbFunction&)’

I've found this line in the documentation that seems to imply that this is a bug (plus, it work fine on the emscripten codepath):

Warning: while callbacks are currently passed by reference, they're freed after the called C++ function returns! That's intended for synchronous functions like Array.map which calls a callback zero or more times and then returns. For asynchronous functions like setTimeout which calls the callback after it has returned, you need to copy the argument to a new nbind::cbFunction and store it somewhere.

@arcanis
Copy link
Contributor Author

arcanis commented Dec 18, 2016

A workaround is to use pointers, as mentioned in #15. However, a better alternative is to use std::unique_ptr, so that those pointers will be automatically disposed.

std::unique_ptr<nbind::cbFunction> fnPtr;
fnPtr = std::make_unique<nbind::cbFunction>(otherFnFunction);

@jjrv
Copy link
Member

jjrv commented Dec 18, 2016

This sounds like a bug. I'll look into it.

@jjrv
Copy link
Member

jjrv commented Dec 18, 2016

This should work:

nbind::cbFunction fn(otherFnFunction);

While that should be enough for basic use, I guess the move constructor and assignment operators should also be implemented...

@arcanis
Copy link
Contributor Author

arcanis commented Dec 18, 2016

Hm, the move constructor isn't really necessary, but the assignment operator is interesting because it's the only way to reassign the callbacks attributes of a class instance (otherwise, using pointers will still be required).

@samuelnj226
Copy link

@arcanis once I declare a unique_ptr to a callback function, how do I go about callling that callback function? I seem to be having some issues with the syntax...

@jjrv
Copy link
Member

jjrv commented Oct 13, 2017

@samuelnj226 try: (*fn)();

Full working example:

#include "nbind/api.h"

/* If using C++11 instead of C++14, uncomment this:
namespace std {
  template<typename T, typename... Args>
  std::unique_ptr<T> make_unique(Args&&... args) {
    return std::unique_ptr<T>(new T(std::forward<Args>(args)...));
  }
}
*/

void callMe(nbind::cbFunction cb) {
  std::unique_ptr<nbind::cbFunction> fn;
  fn = make_unique<nbind::cbFunction>(cb);
  (*fn)();
}

#include "nbind/nbind.h"

NBIND_GLOBAL() {
  function(callMe);
}

JS:

var nbind = require('nbind');
var lib = nbind.init().lib;

lib.callMe(() => console.log('Hello, World!'));

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