charto / nbind

:sparkles: Magical headers that make your C++ library accessible from JavaScript :rocket:
MIT License
1.98k stars 119 forks source link

Callbacks cannot be copied on the v8 codepath #52

Open arcanis opened 7 years ago

arcanis commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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

jjrv commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

@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 commented 7 years ago

@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!'));