Question about ThreadSafeFunction

[marco-ms]

Hi,
I am doing some c++ native code that returns some values via a callback to the add-on on a separate thread. I am using ThreadSafeFunction to go back to the main thread and post result to the JavaScript, however to do that I am using Promises, so I don't really need a function callback.
https://github.com/nodejs/node-addon-api/blob/master/doc/threadsafe_function.md#new

But in all Napi::ThreadSafeFunction::New() there is no way to NOT pass a Function, which I don't really need. Am I using the right tool here to go from a worker thread back to the main thread or is there an alternative that doesn't require a Function?
I think this likely should be called ThreadSafeCallback and accept either a Function or a Promise.

[KevinEady]

Hi @marco-ms,

I think an AsyncWorker might work for you? Take a look at the example.

[marco-ms]

Hi @marco-ms,

I think an AsyncWorker might work for you? Take a look at the example.

AsyncWorker doesn't seem to fit, as I need to call my c++ code in the main thread and then it'll callback to the worker thread.
AsyncWorker calls native code in the worker thread, which is not what I want.
I need to call c++ call in the main thread, I receive callback in the worker thread and need to go back to the main thread to post result to JS layer.

[marco-ms]

Actually seems my request to have it optional has been implemented already on N-API, but looks like made it only to Node 12.6.0 and yet to be released 10.17.0? Too bad it'll never get to Node 8.x I guess.
nodejs/node#27791

Anyhow, is there a way to support this directly under node-addon-api and create a bogus callback? I tried passing Napi::Function() to ThreadSafe, but it errors out. Suggestions welcome.

cc @mhdawson

[mhdawson]

@marco-ms can you show the code and the resulting error. You should be able to define dummy function that you can pass in. Likely, a global static one so that you can use the same one in all cases as well.

[marco-ms]

Code looks like this, from the JS this function is called directly passing a Promise and getting back some result.

Napi::Value MyFunction(const Napi::CallbackInfo &info) {
    Napi::Env env = info.Env();
    Napi::HandleScope scope(info.Env());
    tsfn = Napi::ThreadSafeFunction::New(
      info.Env(),
      Napi::Function(), // bogus function
      "Resource Name",
      0,
      1
    );
    const Napi::Promise::Deferred deferred = Napi::Promise::Deferred::New(env);
    GetObject()->DoSomethingNativeCode([deferred](Result &result) {
        // DoSomethingNativeCode() will call this callback on a worker thread
        auto callback = [deferred, result](Napi::Env env, Napi::Function jsCallback) {
            // Generate a jsResult object
            deferred.Resolve(jsResult);
        };
        // Wait for the callback in the JS thread to be done.
        napi_status status = tsfn.BlockingCall(callback);
        if (status != napi_ok) {
            // Handle error
        }
        // Release the thread-safe function
        tsfn.Release();
    });
    return deferred.Promise();
}

It throws:

Uncaught Exception: Error: Invalid Argument

If instead I modify Napi::Function() to info[1].As<Napi::Function>() and pass from JavaScript an empty function, then it works fine.
And anyway even if I can get a bogus function, the code is not as reusable, since with a lambda callback I am not sure how to encapsulate the code really.
I tried a functor, but didn't succeed, anyway that is a separate issue.

[KevinEady]

Calling Napi::Function() will create an empty Napi Function wrapper and not actually point to anything. You need to create a function by either a lambda:

Function::New(env, [](const CallbackInfo& cbinfo){ return cbinfo.Env().Undefined(); } ) 

or a global static

Value Noop( const CallbackInfo& info ) {
  return info.Env().Undefined();
}

Function::New(env, Noop) // pass this to ThreadSafeFunction::New

If you will be creating many TSFNs, you could create one function, store it in a reference, and pass that reference's value:

Napi::FunctionReference ref = Napi::Persistent(Function::New(.../* from above */));

ref.Value() // pass this to ThreadSafeFunction::New

The optional Function argument will eventually make its way into node-addon-api.

[CraigMason]

Adding a question here as I have been following this thread.

I'm doing something similar, and using ThreadSafeFunction. I'm a little lost as to how I can get a reference to the JS this inside of the callback.

Function::Call works inside of the c++ callback, but I cannot get anything that requires this to work.

[KevinEady]

You want the Function that's passed to ThreadSafeFunction::New to have a specific this (when accessing callbackInfo.This() for C++ native functions or the this object in a JS function)?

Well, I guess you could pass a function that was bound to a specific this via calling bind on the Function?

[CraigMason]

@KevinEady I'm just extending EventEmitter, inherits(MyObject, EventEmitter);

I want to call MyObject.emit inside async thread, and need to have correct Napi::CallbackInfo::This() available to call it I believe.

[KevinEady]

@CraigMason sooo I would think, you are passing either the complete MyObject or MyObject.emit to your native module?

• If you are passing the complete object, you should have a reference to the object somewhere, such that when you use Call, you can pass that object as the receiver
• If you are passing MyObject.emit directly, then could you pass MyObject.emit.bind(MyObject) instead?

What are you expecting to call MyObject.emit? As the ThreadSafeFunction's callback parameter, finalizeCallback parameter, or within a [Non]BlockingCall's callback parameter?

[CraigMason]

I should probably explain the goal in a more abstract sense, rather than querying a specific implementation - as I'm new to native modules and I could be taking the wrong approach here.

I have a device which has an SDK, which dispatches events via a callback on new threads. I ultimately want to encapsulate that object as a JS Object that emits events, so I can deal with remaining logic in Node.

So having read through some example and the docs here, my conclusion was to extend EventEmitter (inherits(DeviceModel, EventEmitter);), grab a function reference to emit in the constructor:

Napi::Function emit = info.This().As<Napi::Object>().Get("emit").As<Napi::Function>();
this->_emitRef = Napi::Reference<Napi::Function>::New(emit, 1);

Then create a ThreadSafeFunction to eventually call the emit function, which accesses EventEmitter class vars. Thus, I figure I need to ensure this is correct - which is where I'm a bit lost.

Thank you,

[KevinEady]

@CraigMason I see...

One thing is that if you pass emit as the TSFN callback function, it can only be called with at most one argument: that value which is returned through the [Non]BlockingCall()'s optional Callback parameter. This means that you'll only be able to pass the string eventName to this emit function and never any additional data args... is that okay?

I'm not 100% sure if it would work, but try binding the emit function like so:

Napi::Function emit = info.This().As<Napi::Object>().Get("emit").As<Napi::Function>();
Napi::Function bound = emit.Get("bind").As<Function>().Call(emit, { info.This() }).As<Function>();
this->_emitRef = Napi::Reference<Napi::Function>::New(bound, 1);

[CraigMason]

@KevinEady Thank you :) Calling bind did indeed work. I hadn't even thought of doing it that way.

Regarding single parameter.. hmm, I'm not sure yet, I need to consider the remaining architecture first.

[marco-ms]

The optional Function argument will eventually make its way into node-addon-api.

Thanks, your fix works for me. I wonder if node-addon-api can anticipate this optional argument by creating a noop function itself. That way would work also on older Node 8.x that won't probably get the fix backported. But I understand this may go out of scope.
You can keep this item open to track whatever work you are going to plan to make optional the function or feel free to close this.

[mhdawson]

@marco-ms I think given that 8.x only has < 4 months before it goes EOL and there is a work around we'll probably just close this as we have lots of other work to do. Please let me know if you think that is not the right thing to do and we'll reopen.