From 524daf89a1509ec6e1ba45fa92e21d051a7c783a Mon Sep 17 00:00:00 2001 From: Michael Dawson Date: Thu, 21 May 2020 22:05:16 -0400 Subject: [PATCH] n-api: ensure scope present for finalization Refs: https://github.com/nodejs/node-addon-api/issues/722 Ensure a scope is on stack during finalization as finalization functions can create JS Objects Signed-off-by: Michael Dawson PR-URL: https://github.com/nodejs/node/pull/33508 Reviewed-By: Gabriel Schulhof Reviewed-By: James M Snell --- src/js_native_api_v8.cc | 1 + .../binding.gyp | 8 ++++ .../test.js | 18 +++++++ .../test_worker_terminate_finalization.c | 48 +++++++++++++++++++ 4 files changed, 75 insertions(+) create mode 100644 test/node-api/test_worker_terminate_finalization/binding.gyp create mode 100644 test/node-api/test_worker_terminate_finalization/test.js create mode 100644 test/node-api/test_worker_terminate_finalization/test_worker_terminate_finalization.c diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc index 4f6d54ad475fd2..daad1907349d63 100644 --- a/src/js_native_api_v8.cc +++ b/src/js_native_api_v8.cc @@ -267,6 +267,7 @@ class RefBase : protected Finalizer, RefTracker { protected: inline void Finalize(bool is_env_teardown = false) override { if (_finalize_callback != nullptr) { + v8::HandleScope handle_scope(_env->isolate); _env->CallIntoModule([&](napi_env env) { _finalize_callback( env, 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)