1berry/nodejs
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

n-api: back up env before finalize

Browse Source 
Heed the comment to not use fields of a Reference after calling its
finalize callback, because such a call may destroy the Reference.

Fixes: https://github.com/nodejs/node/issues/19673
PR-URL: https://github.com/nodejs/node/pull/19718
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
This commit is contained in:
Gabriel Schulhof 2018-03-31 21:37:17 -04:00
parent ed86cc570e
commit 1a5a19d6d4
4 changed files with 33 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
src/node_api.cc
View File

@ -457,9 +457,10 @@ class Reference : private Finalizer {
// Check before calling the finalize callback, because the callback might // Check before calling the finalize callback, because the callback might
// delete it. // delete it.
bool delete_self = reference->_delete_self; bool delete_self = reference->_delete_self;
napi_env env = reference->_env;
if (reference->_finalize_callback != nullptr) { if (reference->_finalize_callback != nullptr) {
NAPI_CALL_INTO_MODULE_THROW(reference->_env, NAPI_CALL_INTO_MODULE_THROW(env,
reference->_finalize_callback( reference->_finalize_callback(
reference->_env, reference->_env,
reference->_finalize_data, reference->_finalize_data,

15
test/addons-napi/8_passing_wrapped/binding.cc
View File

@ -1,7 +1,9 @@
#include "myobject.h" #include "myobject.h"
#include "../common.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; size_t argc = 1;
napi_value args[1]; napi_value args[1];
NAPI_CALL(env, napi_get_cb_info(env, info, &argc, args, nullptr, nullptr)); NAPI_CALL(env, napi_get_cb_info(env, info, &argc, args, nullptr, nullptr));
@ -12,7 +14,7 @@ napi_value CreateObject(napi_env env, napi_callback_info info) {
return instance; 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; size_t argc = 2;
napi_value args[2]; napi_value args[2];
NAPI_CALL(env, napi_get_cb_info(env, info, &argc, args, nullptr, nullptr)); NAPI_CALL(env, napi_get_cb_info(env, info, &argc, args, nullptr, nullptr));
@ -29,12 +31,19 @@ napi_value Add(napi_env env, napi_callback_info info) {
return sum; 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); MyObject::Init(env);
napi_property_descriptor desc[] = { napi_property_descriptor desc[] = {
DECLARE_NAPI_PROPERTY("createObject", CreateObject), DECLARE_NAPI_PROPERTY("createObject", CreateObject),
DECLARE_NAPI_PROPERTY("add", Add), DECLARE_NAPI_PROPERTY("add", Add),
DECLARE_NAPI_PROPERTY("finalizeCount", FinalizeCount),
}; };
NAPI_CALL(env, NAPI_CALL(env,

12
test/addons-napi/8_passing_wrapped/myobject.cc
View File

@ -1,9 +1,14 @@
#include "myobject.h" #include "myobject.h"
#include "../common.h" #include "../common.h"
size_t finalize_count = 0;
MyObject::MyObject() : env_(nullptr), wrapper_(nullptr) {} 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( void MyObject::Destructor(
napi_env env, void* nativeObject, void* /*finalize_hint*/) { napi_env env, void* nativeObject, void* /*finalize_hint*/) {
@ -45,6 +50,11 @@ napi_value MyObject::New(napi_env env, napi_callback_info info) {
} }
obj->env_ = env; 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, NAPI_CALL(env, napi_wrap(env,
_this, _this,
obj, obj,

9
test/addons-napi/8_passing_wrapped/test.js
View File

@ -1,9 +1,16 @@
'use strict'; 'use strict';
// Flags: --expose-gc
const common = require('../../common'); const common = require('../../common');
const assert = require('assert'); const assert = require('assert');
const addon = require(`./build/${common.buildType}/binding`); const addon = require(`./build/${common.buildType}/binding`);
const obj1 = addon.createObject(10); let obj1 = addon.createObject(10);
const obj2 = addon.createObject(20); const obj2 = addon.createObject(20);
const result = addon.add(obj1, obj2); const result = addon.add(obj1, obj2);
assert.strictEqual(result, 30); assert.strictEqual(result, 30);
// Make sure the native destructor gets called.
obj1 = null;
global.gc();
assert.strictEqual(addon.finalizeCount(), 1);