Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

n-api: back up env before finalize #19718

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/node_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -457,9 +457,10 @@ class Reference : private Finalizer {
// Check before calling the finalize callback, because the callback might
// delete it.
bool delete_self = reference->_delete_self;
napi_env env = reference->_env;

if (reference->_finalize_callback != nullptr) {
NAPI_CALL_INTO_MODULE_THROW(reference->_env,
NAPI_CALL_INTO_MODULE_THROW(env,
reference->_finalize_callback(
reference->_env,
reference->_finalize_data,
Expand Down
15 changes: 12 additions & 3 deletions test/addons-napi/8_passing_wrapped/binding.cc
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
#include "myobject.h"
#include "../common.h"

napi_value CreateObject(napi_env env, napi_callback_info info) {
extern size_t finalize_count;

static napi_value CreateObject(napi_env env, napi_callback_info info) {
size_t argc = 1;
napi_value args[1];
NAPI_CALL(env, napi_get_cb_info(env, info, &argc, args, nullptr, nullptr));
Expand All @@ -12,7 +14,7 @@ napi_value CreateObject(napi_env env, napi_callback_info info) {
return instance;
}

napi_value Add(napi_env env, napi_callback_info info) {
static napi_value Add(napi_env env, napi_callback_info info) {
size_t argc = 2;
napi_value args[2];
NAPI_CALL(env, napi_get_cb_info(env, info, &argc, args, nullptr, nullptr));
Expand All @@ -29,12 +31,19 @@ napi_value Add(napi_env env, napi_callback_info info) {
return sum;
}

napi_value Init(napi_env env, napi_value exports) {
static napi_value FinalizeCount(napi_env env, napi_callback_info info) {
napi_value return_value;
NAPI_CALL(env, napi_create_uint32(env, finalize_count, &return_value));
return return_value;
}

static napi_value Init(napi_env env, napi_value exports) {
MyObject::Init(env);

napi_property_descriptor desc[] = {
DECLARE_NAPI_PROPERTY("createObject", CreateObject),
DECLARE_NAPI_PROPERTY("add", Add),
DECLARE_NAPI_PROPERTY("finalizeCount", FinalizeCount),
};

NAPI_CALL(env,
Expand Down
12 changes: 11 additions & 1 deletion test/addons-napi/8_passing_wrapped/myobject.cc
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
#include "myobject.h"
#include "../common.h"

size_t finalize_count = 0;

MyObject::MyObject() : env_(nullptr), wrapper_(nullptr) {}

MyObject::~MyObject() { napi_delete_reference(env_, wrapper_); }
MyObject::~MyObject() {
finalize_count++;
napi_delete_reference(env_, wrapper_);
}

void MyObject::Destructor(
napi_env env, void* nativeObject, void* /*finalize_hint*/) {
Expand Down Expand Up @@ -45,6 +50,11 @@ napi_value MyObject::New(napi_env env, napi_callback_info info) {
}

obj->env_ = env;

// It is important that the below call to napi_wrap() be such that we request
// a reference to the wrapped object via the out-parameter, because this
// ensures that we test the code path that deals with a reference that is
// destroyed from its own finalizer.
NAPI_CALL(env, napi_wrap(env,
_this,
obj,
Expand Down
9 changes: 8 additions & 1 deletion test/addons-napi/8_passing_wrapped/test.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,16 @@
'use strict';
// Flags: --expose-gc

const common = require('../../common');
const assert = require('assert');
const addon = require(`./build/${common.buildType}/binding`);

const obj1 = addon.createObject(10);
let obj1 = addon.createObject(10);
const obj2 = addon.createObject(20);
const result = addon.add(obj1, obj2);
assert.strictEqual(result, 30);

// Make sure the native destructor gets called.
obj1 = null;
global.gc();
assert.strictEqual(addon.finalizeCount(), 1);