Skip to content

Commit c5f3e91

Browse files
committed
n-api: ensure scope present for finalization
Refs: nodejs/node-addon-api#722 Ensure a scope is on stack during finalization as finalization functions can create JS Objects Signed-off-by: Michael Dawson <michael_dawson@ca.ibm.com>
1 parent 6a1df3b commit c5f3e91

File tree

4 files changed

+68
-0
lines changed

4 files changed

+68
-0
lines changed

src/js_native_api_v8.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,7 @@ class RefBase : protected Finalizer, RefTracker {
267267
protected:
268268
inline void Finalize(bool is_env_teardown = false) override {
269269
if (_finalize_callback != nullptr) {
270+
v8::HandleScope handle_scope(_env->isolate);
270271
_env->CallIntoModule([&](napi_env env) {
271272
_finalize_callback(
272273
env,
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{
2+
"targets": [
3+
{
4+
"target_name": "test_worker_terminate_finalization",
5+
"sources": [ "test_worker_terminate_finalization.c" ]
6+
}
7+
]
8+
}
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
'use strict';
2+
const common = require('../../common');
3+
const { Worker, isMainThread } = require('worker_threads');
4+
5+
if (isMainThread) {
6+
const worker = new Worker(__filename);
7+
worker.on('error', common.mustNotCall());
8+
} else {
9+
const { Test } =
10+
require(`./build/${common.buildType}/test_worker_terminate_finalization`);
11+
12+
// Spin up thread and call add-on create the right sequence
13+
// of rerences to hit the case reported in
14+
// https://github.com/nodejs/node-addon-api/issues/722
15+
// will crash if run under debug and its not possible to
16+
// create object in the specific finalizer
17+
Test(new Object());
18+
}
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
#include <stdio.h>
2+
#include <node_api.h>
3+
#include <assert.h>
4+
#include <malloc.h>
5+
#include "../../js-native-api/common.h"
6+
7+
int wrappedNativeData;
8+
napi_ref ref;
9+
void WrapFinalizer(napi_env env, void* data, void* hint) {
10+
uint32_t count;
11+
NAPI_CALL_RETURN_VOID(env, napi_reference_unref(env, ref, &count));
12+
}
13+
14+
napi_value Test(napi_env env, napi_callback_info info) {
15+
size_t argc = 1;
16+
napi_value argv[1];
17+
napi_value result;
18+
napi_ref ref;
19+
napi_ref wrapResult;
20+
21+
NAPI_CALL(env, napi_get_cb_info(env, info, &argc, argv, NULL, NULL));
22+
NAPI_CALL(env, napi_create_external_buffer(env, 4, (void*) malloc(4), NULL, NULL, &result));
23+
NAPI_CALL(env, napi_create_reference(env, result, 1, &ref));
24+
NAPI_CALL(env, napi_wrap(env, argv[0], (void*) &wrappedNativeData, WrapFinalizer, NULL, &wrapResult));
25+
return NULL;
26+
}
27+
28+
napi_value Init(napi_env env, napi_value exports) {
29+
napi_property_descriptor properties[] = {
30+
DECLARE_NAPI_PROPERTY("Test", Test)
31+
};
32+
33+
NAPI_CALL(env, napi_define_properties(
34+
env, exports, sizeof(properties) / sizeof(*properties), properties));
35+
36+
return exports;
37+
}
38+
39+
// Do not start using NAPI_MODULE_INIT() here, so that we can test
40+
// compatibility of Workers with NAPI_MODULE().
41+
NAPI_MODULE(NODE_GYP_MODULE_NAME, Init)

0 commit comments

Comments
 (0)