What if a thread-safe function execution throws an exception，FATAL ERROR: Error::Error napi_define_properties, I catch exceptions and exception handling calls Env interface GetAndClearPendingException, then submitted to the following error

[Candylong]

[JckXia]

Hi there. Any chance you can provide a minimal reproducible example? (including the node version) I think this might be related to how we wrap primitive type errors thrown related to the changes in PR #1075

[Candylong]

Hi there. Any chance you can provide a minimal reproducible example? (including the node version) I think this might be related to how we wrap primitive type errors thrown related to the changes in PR #1075

[Candylong]

@JckXia ,Thank you for your reply， i use the node-addon-api version 5.0.0 write the addon plugin, and run in electron 18(Indicates the corresponding nodejs version 16.13.2), this isn't a problem that always comes up, but I've encountered it several times using the reload feature on my electron DevTools, what should i do? Here are the crash stack and thread-safe functions when an exception occurs


Execute in a thread-safe function
auto jscallback = [=](Napi::Env env, Napi::Function jsCallback)
{
Napi::Object videoFrame = Napi::Object::New(env);
videoFrame .Set("width", Napi::Number::New(env, 1920)); //crash happens
}

[JckXia]

Could you provide the source code of an addon that would best replicate this issue? Or would that not possible in this case?

But just a shot in the dark...it looks like the real issue here is napi_set_named_property failed to define the property width on the videoFrame object. The macro in the following line simply wraps the error in an Napi::Object, where we do the error wrapping logic but napi_define_properties shouldn't fail.

As you have mentioned it is an intermittent failure. Another option is to compile node 16.33.2 from the source and put a debugger starting at this line: https://github.com/nodejs/node/blob/main/src/js_native_api_v8.cc#L1104 should the error persists.

[Candylong]

The following example can reproduce the crash, Hi, @jack 

…

------------------ 原始邮件 ------------------ 发件人: "nodejs/node-addon-api" ***@***.***>; 发送时间: 2022年12月5日(星期一) 凌晨4:15 ***@***.***>; ***@***.***>;"State ***@***.***>; 主题: Re: [nodejs/node-addon-api] What if a thread-safe function execution throws an exception，FATAL ERROR: Error::Error napi_define_properties, I catch exceptions and exception handling calls Env interface GetAndClearPendingException, then submitted to the following error (Issue #1244) Could you provide the source code of an addon that would best replicate this issue? Or would that not possible in this case? But just a shot in the dark...it looks like the real issue here is napi_set_named_property failed to define the property width on the videoFrame object. The macro in the following line simply wraps the error in an Napi::Object, where we do the error wrapping logic but napi_define_properties shouldn't fail. As you have mentioned it is an intermittent failure. Another option is to compile node 16.33.2 from the source and put a debugger starting at this line: https://github.com/nodejs/node/blob/main/src/js_native_api_v8.cc#L1104 should the error persists. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you modified the open/close state.Message ID: ***@***.***>

[JckXia]

Sorry, the format looked a bit funky to me. Is this a new example are you talking about, or is it the one in your original post?

[Candylong]

Sorry, that message was sent in the wrong format， @JckXia , Here is the addon implementation code I can reproduce for crash:

#include <napi.h>
#include
#include

class MyObjWrap : public Napi::ObjectWrap {

public:
static Napi::FunctionReference Initialize(Napi::Env env) {
printf("init MyObjWrap\n");
Napi::Function fn = DefineClass(env, "MyObjWrap", {
InstanceMethod("print", &MyObjWrap::Print),
});
return Napi::Persistent(fn);
}
MyObjWrap(const Napi::CallbackInfo& info) : Napi::ObjectWrap(info) {
// ...
}
~MyObjWrap() {
// ...
}

private:
void Print(const Napi::CallbackInfo& info){
std::cout << "Print ..." << std::endl;
}

};

class MyObjWrap1 : public Napi::ObjectWrap {

public:
static Napi::FunctionReference Initialize(Napi::Env env) {
Napi::Function fn = DefineClass(env, "MyObjWrap1", {
InstanceMethod("print1", &MyObjWrap1::Print1),
});
return Napi::Persistent(fn);
}
MyObjWrap1(const Napi::CallbackInfo& info) : Napi::ObjectWrap(info) {
// ...
}
~MyObjWrap1() {
// ...
}

private:
void Print1(const Napi::CallbackInfo& info){
std::cout << "Print1 ..." << std::endl;
}

};

Napi::ThreadSafeFunction myThreadFunction;
using callJsfuntion = std::function<void(Napi::Env, Napi::Function)>;
bool jsCallBack(callJsfuntion fucntion)
{
if (napi_status::napi_ok == myThreadFunction.Acquire())
{
bool ok = napi_ok == myThreadFunction.BlockingCall(fucntion);
myThreadFunction.Release();
return ok;
}
else
{
return true;
}
}

bool isStop(true);
void threadLoop()
{
while(!isStop)
{
auto callback = [=](Napi::Env env, Napi::Function function)
{
Napi::Object videoFrame = Napi::Object::New(env);
videoFrame.Set("width", Napi::Number::New(env, 1920)); // exceptions occur
videoFrame.Set("mode2", Napi::Number::New(env, 1920));
videoFrame.Set("mode3", Napi::Number::New(env, 1080));
videoFrame.Set("mode4", Napi::Number::New(env, 1920));
videoFrame.Set("mode5", Napi::Number::New(env, 1080));
for(int i = 0; i<5000; i++) // Compare to internal frequent tasks
{
videoFrame.Set(std::to_string(i).c_str(), Napi::String::New(env, "123456789"));
}

  for(int i = 0; i<7000; i++)
  {
    videoFrame.Set(std::to_string(i).c_str(), Napi::Number::New(env, 1080));
  }
  function.Call({Napi::String::New(env, "hello world!"), videoFrame});
};
jsCallBack(callback);
// sleep

std::this_thread::sleep_for(std::chrono::microseconds(1));
}
}

void threadLoop2()
{
while(!isStop)
{
auto callback = [=](Napi::Env env, Napi::Function function)
{
Napi::Object videoFrame = Napi::Object::New(env);
videoFrame.Set("width", Napi::Number::New(env, 1920)); // exceptions occur
videoFrame.Set("mode2", Napi::Number::New(env, 1920));
videoFrame.Set("mode3", Napi::Number::New(env, 1080));
videoFrame.Set("mode4", Napi::Number::New(env, 1920));
videoFrame.Set("mode5", Napi::Number::New(env, 1080));

  for(int i = 0; i<5000; i++)
  {
    videoFrame.Set(std::to_string(i).c_str(), Napi::Number::New(env, 1080));
  }

  for(int i = 0; i<5000; i++)   // Compare to internal frequent tasks
  {
    videoFrame.Set(std::to_string(i).c_str(), Napi::String::New(env, "123456789"));
  }
  function.Call({Napi::String::New(env, "hello world!"), videoFrame});
};
jsCallBack(callback);
// sleep

std::this_thread::sleep_for(std::chrono::microseconds(1));
}
}

class MyObjWrap2 : public Napi::ObjectWrap {

public:
static Napi::FunctionReference Initialize(Napi::Env env) {
printf("init MyObjWrap2\n");
Napi::Function fn = DefineClass(env, "MyObjWrap2", {
InstanceMethod("print2", &MyObjWrap2::Print2),
});
return Napi::Persistent(fn);
}
MyObjWrap2(const Napi::CallbackInfo& info) : Napi::ObjectWrap(info) {
// ...
isStop = false;
// thread 1
mythread1 = std::thread(std::bind(threadLoop));
// thread 2
mythread2 = std::thread(std::bind(threadLoop2));
}
~MyObjWrap2()
{
isStop = true;
mythread1.join();
mythread2.join();
}

private:
void Print2(const Napi::CallbackInfo& info){
std::cout << "Print2 ..." << std::endl;
}
std::thread mythread1;
std::thread mythread2;

};

class Addon : public Napi::Addon {
public:

Napi::Value CreateFunction(const Napi::CallbackInfo& info)
{
printf("Addon Function \n");
Napi::Env env = info.Env();
Napi::Object param = info[0].AsNapi::Object();
if (!param.Has("tsfn"))
{
Napi::TypeError::New(env, "Wrong arguments").ThrowAsJavaScriptException();
return env.Undefined();
}

Napi::Function tsfn = param.Get("tsfn").As<Napi::Function>();
myThreadFunction = Napi::ThreadSafeFunction::New(
                env,
                tsfn,            // JavaScript function called asynchronously
                "Resource Name", // Name
                0,               // Unlimited queue
                1,               // Only one thread will use this initially
                [](Napi::Env) {  // Finalizer used to clean threads up
                    printf("Addon tsfn finalizer \n");
                });

env.AddCleanupHook([=](){
  printf("execute clean hook\n");
  myThreadFunction.Release();
});
return Napi::Number::New(env, 0);

}

Napi::Value CreateObject(const Napi::CallbackInfo& info)
{
printf("Addon CreateObject \n");
Napi::Env env = info.Env();

Napi::Object jsObject = myObjWrapCtr2.New({});
return jsObject;

}

Addon(Napi::Env env, Napi::Object exports) {
myObjWrapCtr = MyObjWrap::Initialize(env);
myObjWrapCtr1 = MyObjWrap1::Initialize(env);
myObjWrapCtr2 = MyObjWrap2::Initialize(env);
DefineAddon(exports, {
InstanceMethod("CreateFunction", &Addon::CreateFunction),
InstanceMethod("CreateObject", &Addon::CreateObject)}
);
}
private:
Napi::FunctionReference myObjWrapCtr;
Napi::FunctionReference myObjWrapCtr1;
Napi::FunctionReference myObjWrapCtr2;
};

// The macro announces that instances of the class CallasAddon will be
// created for each instance of the add-on that must be loaded into Node.js.
NODE_API_NAMED_ADDON(NODE_GYP_MODULE_NAME, Addon)

Here is the code I can reproduce for electron using addon：(render.js)

//'use strict'
const addon = require('./addon')
let myObjWrap2 = null
const testAPI = document.getElementById('TEST')
function callEmit()
{
console.log(arguments[0]);
}
testAPI.onclick = () =>
{
addon.CreateFunction({tsfn: callEmit})
myObjWrap2 = addon.CreateObject()
}

To repeat the step, click the button on the electron interface and then click reload on Devtools and wait for a while to crash

Judging by the results of my tests：I think when I click on the button in the electron (start threads and run thread-safe function), because the thread-safe function need to perform more tasks (may queue accumulation is too many tasks, not to be able to perform), and then led to a thread-safe function performs the exception, but what I don't really understand the underlying implementation of it @JckXia

[JckXia]

Ok thank you @Candylong. Will try to reproduce this on my end and get back to you asap.

[JckXia]

Hey @Candylong, there were some issues with the code you provided. I made some fixes and stored it under https://github.com/JckXia/napi-bug-repro. Just want to double check we are indeed able to reproduce the bug with this addon. I haven't used electron before, so will do more read-up on that tommorow but just running it with node shows no problems.

Out of curiosity, were you able to replicate this issue outside of the electron environment? Or does the error only show up when run in electron? (For example, just run node driver.js after building the addon from my repo)

[Candylong]

@JckXia ,like you said, normal operation will be fine, the main issue is to highlight the context-aware addon feature, while electron's reload will reload addon(.node) repeatedly，which is what context-aware addon supports(https://nodejs.org/dist/latest-v18.x/docs/api/addons.html#context-aware-addons), so it shouldn't be a problem
,

I have arranged my demo and put it here(https://github.com/Candylong/napi-electron-demo). I mainly run it on windows, and it can be reproduced after about 5 operations. Can you have a try

[JckXia]

Hey @Candylong. I tried your code and can indeed replicate this bug (clicking the button in 5 rapid successions and then attempting to reload). Will take a deeper dive tomorrow.