Wrapped Napi-extracted functions into C++ shared_ptr<>-s for easier lifetime management.

[dkorolev]

Hi @gengjiawen,

Was hoping for some feedback -- after experimenting with node-addon-api for a few days looks like I've managed to wrap the extracted functions into C++ objects that do not require explicit lifetime management. Would be grateful for your time, and about as grateful if you could point me to someone else who would be better suited and/or would have more time.

A one-liner to run the code:

git clone -b cpp_thread_safe_calls https://github.com/dkorolev/node-addon-examples && (cd node-addon-examples/thread_safe_function_counting_wrapped/node-addon-api; npm i && node addon.js)

Results in:

RunAsyncWork(): calling the C++ function.
RunAsyncWork(): the promise is returned from C++.
Callback from C++: odd one=1.
Callback from C++: even two=2.
Callback from C++: odd three=3.
Callback from C++: even four=4.
Callback from C++: odd five=5.
RunAsyncWork(): the promise resolved, from C++, to true.

The relevant JavaScript piece:

const { runAsyncWork } = require('bindings')('addon');

console.log('RunAsyncWork(): calling the C++ function.');
const promise = runAsyncWork(
  (i, s) => { console.log(`Callback from C++: even ${s}=${i}.`); },
  (i, s) => { console.log(`Callback from C++: odd ${s}=${i}.`); }
);
console.log('RunAsyncWork(): the promise is returned from C++.');
promise.then((value) => { console.log(`RunAsyncWork(): the promise resolved, from C++, to ${value}.`); });

And the respective C++ one (the real code has comments, just pasting the cleaned up version here):

Napi::Value RunAsyncWork(const Napi::CallbackInfo& cbinfo) {
  NodeJSContext ctx(cbinfo);
  NodeJSFunction f_even = ctx.ExtractJSFunction(cbinfo[0]);
  NodeJSFunction f_odd = ctx.ExtractJSFunction(cbinfo[1]);
  ctx.RunAsync([f_even, f_odd]() {
    struct IntString final { int i; std::string s; };
    for (IntString& value : std::vector<IntString>({{1, "one"}, {2, "two"}, {3, "three"}, {4, "four"}, {5 ,"five"}})) {
      ((value.i % 2 == 0) ? f_even : f_odd)(value.i, value.s);
      std::this_thread::sleep_for(std::chrono::milliseconds(500));
    }
  });
  return ctx.GetPromise();
}

[mhdawson]

@KevinEady, @gabrielschulhof would probably make sense for one or both of you to take a look at this one.

[mhdawson]

@KevinEady, @gabrielschulhof ping

[KevinEady]

I think this is a really neat PR! I think as-is, it is a good example and starting point for people to work with. @dkorolev Please see review comments and let me know if you plan on making any changes

Some nitpicking though... perhaps the naming of the entities could be something different, as it is a little misleading in their current names.

NodeJSContext -> AsyncCallContext
NodeJSFunction -> AsyncCall
NodeJSContext::ExtractJSFunction -> AsyncCallContext::CreateCall

...? I dunno, just a thought.

[KevinEady]

"the right way" ... perhaps something that uses Value::From:

  template <typename ValueType>
  static void PopulateArg(Napi::Env env, ValueType input, napi_value& output) { output = Napi::Value::From(env, input); }

[KevinEady]

Very neat implementation of "automagically" casting the C++ types to respective Napi::Values.

You mention "process the return value". Currently the operator() returns void, so yes, the caller (eg. f_odd(1,"one") in your example) has no way of processing the return value of the JavaScript side.

The problem is that this is done asynchronously, so we can't return the immediate value. What about using an std::promise<Napi::Value> from the return value of jsf.Call()...? Just throwing some ideas out.

[dkorolev]

Hi Kevin!

Thanks for chiming in! I've refactored the code since, you may find this and this more complete.

On a grander scheme of things, I'm looking into tighter C++ <=> V8 coupling these days, both in my day job and as a pet project, and would be glad to be of help if we could join our efforts. There's an open problem we have right now (can't create a JavaScript object with several wrapped C++ functions as the fields of this JSON object without these fuctions being garbage-collected), and I'm looking forward to finding the time to take a deeper look.

Thanks,
Dima

[KevinEady]

We really don't recommend storing the Napi::Env. You can access it from promise_.Env()

[dkorolev]

Good point, thanks, C5T/Current#904.

[mhdawson]

@KevinEady wondering what the next step on this is?