Skip to content

Commit

Permalink
n-api: ensure scope present for finalization
Browse files Browse the repository at this point in the history
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>

PR-URL: #33508
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
mhdawson authored and codebytere committed Jun 18, 2020
1 parent e3a5332 commit 524daf8
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 0 deletions.
1 change: 1 addition & 0 deletions src/js_native_api_v8.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
8 changes: 8 additions & 0 deletions test/node-api/test_worker_terminate_finalization/binding.gyp
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"targets": [
{
"target_name": "test_worker_terminate_finalization",
"sources": [ "test_worker_terminate_finalization.c" ]
}
]
}
18 changes: 18 additions & 0 deletions test/node-api/test_worker_terminate_finalization/test.js
Original file line number Diff line number Diff line change
@@ -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());
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
#include <stdio.h>
#include <node_api.h>
#include <assert.h>
#include <stdlib.h>
#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)

0 comments on commit 524daf8

Please sign in to comment.