diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc index 31b394ed570a34..daad1907349d63 100644 --- a/src/js_native_api_v8.cc +++ b/src/js_native_api_v8.cc @@ -267,7 +267,8 @@ class RefBase : protected Finalizer, RefTracker { protected: inline void Finalize(bool is_env_teardown = false) override { if (_finalize_callback != nullptr) { - _env->CallIntoModuleThrow([&](napi_env env) { + v8::HandleScope handle_scope(_env->isolate); + _env->CallIntoModule([&](napi_env env) { _finalize_callback( env, _finalize_data, @@ -475,7 +476,7 @@ class CallbackWrapperBase : public CallbackWrapper { napi_callback cb = _bundle->*FunctionField; napi_value result; - env->CallIntoModuleThrow([&](napi_env env) { + env->CallIntoModule([&](napi_env env) { result = cb(env, cbinfo_wrapper); }); diff --git a/src/js_native_api_v8.h b/src/js_native_api_v8.h index 32fc16f155a7fe..9c737f3c9cc9fc 100644 --- a/src/js_native_api_v8.h +++ b/src/js_native_api_v8.h @@ -82,8 +82,13 @@ struct napi_env__ { return v8::Just(true); } - template - void CallIntoModule(T&& call, U&& handle_exception) { + static inline void + HandleThrow(napi_env env, v8::Local value) { + env->isolate->ThrowException(value); + } + + template + inline void CallIntoModule(T&& call, U&& handle_exception = HandleThrow) { int open_handle_scopes_before = open_handle_scopes; int open_callback_scopes_before = open_callback_scopes; napi_clear_last_error(this); @@ -96,13 +101,6 @@ struct napi_env__ { } } - template - void CallIntoModuleThrow(T&& call) { - CallIntoModule(call, [&](napi_env env, v8::Local value) { - env->isolate->ThrowException(value); - }); - } - v8impl::Persistent last_exception; // We store references in two different lists, depending on whether they have diff --git a/src/node_api.cc b/src/node_api.cc index 098c5e03219396..4d6bf536602243 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -57,7 +57,7 @@ class BufferFinalizer : private Finalizer { v8::HandleScope handle_scope(finalizer->_env->isolate); v8::Context::Scope context_scope(finalizer->_env->context()); - finalizer->_env->CallIntoModuleThrow([&](napi_env env) { + finalizer->_env->CallIntoModule([&](napi_env env) { finalizer->_finalize_callback( env, finalizer->_finalize_data, @@ -331,7 +331,7 @@ class ThreadSafeFunction : public node::AsyncResource { v8::Local::New(env->isolate, ref); js_callback = v8impl::JsValueFromV8LocalValue(js_cb); } - env->CallIntoModuleThrow([&](napi_env env) { + env->CallIntoModule([&](napi_env env) { call_js_cb(env, js_callback, context, data); }); } @@ -341,7 +341,7 @@ class ThreadSafeFunction : public node::AsyncResource { v8::HandleScope scope(env->isolate); if (finalize_cb) { CallbackScope cb_scope(this); - env->CallIntoModuleThrow([&](napi_env env) { + env->CallIntoModule([&](napi_env env) { finalize_cb(env, finalize_data, context); }); } @@ -478,7 +478,7 @@ void napi_module_register_by_symbol(v8::Local exports, napi_env env = v8impl::NewEnv(context); napi_value _exports; - env->CallIntoModuleThrow([&](napi_env env) { + env->CallIntoModule([&](napi_env env) { _exports = init(env, v8impl::JsValueFromV8LocalValue(exports)); }); diff --git a/test/node-api/test_worker_terminate_finalization/binding.gyp b/test/node-api/test_worker_terminate_finalization/binding.gyp new file mode 100644 index 00000000000000..800ed54d568e16 --- /dev/null +++ b/test/node-api/test_worker_terminate_finalization/binding.gyp @@ -0,0 +1,8 @@ +{ + "targets": [ + { + "target_name": "test_worker_terminate_finalization", + "sources": [ "test_worker_terminate_finalization.c" ] + } + ] +} diff --git a/test/node-api/test_worker_terminate_finalization/test.js b/test/node-api/test_worker_terminate_finalization/test.js new file mode 100644 index 00000000000000..76cece7bcf3cc4 --- /dev/null +++ b/test/node-api/test_worker_terminate_finalization/test.js @@ -0,0 +1,18 @@ +'use strict'; +const common = require('../../common'); +const { Worker, isMainThread } = require('worker_threads'); + +if (isMainThread) { + const worker = new Worker(__filename); + worker.on('error', common.mustNotCall()); +} else { + const { Test } = + require(`./build/${common.buildType}/test_worker_terminate_finalization`); + + // Spin up thread and call add-on create the right sequence + // of rerences to hit the case reported in + // https://github.com/nodejs/node-addon-api/issues/722 + // will crash if run under debug and its not possible to + // create object in the specific finalizer + Test(new Object()); +} diff --git a/test/node-api/test_worker_terminate_finalization/test_worker_terminate_finalization.c b/test/node-api/test_worker_terminate_finalization/test_worker_terminate_finalization.c new file mode 100644 index 00000000000000..6171f01e1519eb --- /dev/null +++ b/test/node-api/test_worker_terminate_finalization/test_worker_terminate_finalization.c @@ -0,0 +1,48 @@ +#include +#include +#include +#include +#include "../../js-native-api/common.h" + +#define BUFFER_SIZE 4 + +int wrappedNativeData; +napi_ref ref; +void WrapFinalizer(napi_env env, void* data, void* hint) { + uint32_t count; + NAPI_CALL_RETURN_VOID(env, napi_reference_unref(env, ref, &count)); + NAPI_CALL_RETURN_VOID(env, napi_delete_reference(env, ref)); +} + +void BufferFinalizer(napi_env env, void* data, void* hint) { + free(hint); +} + +napi_value Test(napi_env env, napi_callback_info info) { + size_t argc = 1; + napi_value argv[1]; + napi_value result; + napi_ref ref; + void* bufferData = malloc(BUFFER_SIZE); + + NAPI_CALL(env, napi_get_cb_info(env, info, &argc, argv, NULL, NULL)); + NAPI_CALL(env, napi_create_external_buffer(env, BUFFER_SIZE, bufferData, BufferFinalizer, bufferData, &result)); + NAPI_CALL(env, napi_create_reference(env, result, 1, &ref)); + NAPI_CALL(env, napi_wrap(env, argv[0], (void*) &wrappedNativeData, WrapFinalizer, NULL, NULL)); + return NULL; +} + +napi_value Init(napi_env env, napi_value exports) { + napi_property_descriptor properties[] = { + DECLARE_NAPI_PROPERTY("Test", Test) + }; + + NAPI_CALL(env, napi_define_properties( + env, exports, sizeof(properties) / sizeof(*properties), properties)); + + return exports; +} + +// Do not start using NAPI_MODULE_INIT() here, so that we can test +// compatibility of Workers with NAPI_MODULE(). +NAPI_MODULE(NODE_GYP_MODULE_NAME, Init)