8000 n-api: back up env before finalize · nodejs/node@abd9fd6 · GitHub
[go: up one dir, main page]
Skip to content

Commit abd9fd6

Browse files
Gabriel SchulhofMylesBorins
Gabriel Schulhof
authored and
MylesBorins
committed
n-api: back up env before finalize
Heed the comment to not use fields of a Reference after calling its finalize callback, because such a call may destroy the Reference. Fixes: #19673 Backport-PR-URL: #19447 PR-URL: #19718 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
1 parent e6ccdfb commit abd9fd6

File tree

4 files changed

+33
-6
lines changed

4 files changed

+33
-6
lines changed

‎src/node_api.cc‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -481,9 +481,10 @@ class Reference : private Finalizer {
481481
// Check before calling the finalize callback, because the callback might
482482
// delete it.
483483
bool delete_self = reference->_delete_self;
484+
napi_env env = reference->_env;
484485

485486
if (reference->_finalize_callback != nullptr) {
486-
NAPI_CALL_INTO_MODULE_THROW(reference->_env,
487+
NAPI_CALL_INTO_MODULE_THROW(env,
487488
reference->_finalize_callback(
488489
reference->_env,
489490
reference->_finalize_data,

‎test/addons-napi/8_passing_wrapped/binding.cc‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
#include "myobject.h"
22
#include "../common.h"
33

4-
napi_value CreateObject(napi_env env, napi_callback_info info) {
4+
extern size_t finalize_count;
5+
6+
static napi_value CreateObject(napi_env env, napi_callback_info info) {
57
size_t argc = 1;
68
napi_value args[1];
79
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) {
1214
return instance;
1315
}
1416

15-
napi_value Add(napi_env env, napi_callback_info info) {
17+
static napi_value Add(napi_env env, napi_callback_info info) {
1618
size_t argc = 2;
1719
napi_value args[2];
1820
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) {
2931
return sum;
3032
}
3133

32-
napi_value Init(napi_env env, napi_value exports) {
34+
static napi_value FinalizeCount(napi_env env, napi_callback_info info) {
35+
napi_value return_value;
36+
NAPI_CALL(env, napi_create_uint32(env, finalize_count, &return_value));
37+
return return_value;
38+
}
39+
40+
static napi_value Init(napi_env env, napi_value exports) {
3341
MyObject::Init(env);
3442

3543
napi_property_descriptor desc[] = {
3644
DECLARE_NAPI_PROPERTY("createObject", CreateObject),
3745
DECLARE_NAPI_PROPERTY("add", Add),
46+
DECLARE_NAPI_PROPERTY("finalizeCount", FinalizeCount),
3847
};
3948

4049
NAPI_CALL(env,

‎test/addons-napi/8_passing_wrapped/myobject.cc‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,14 @@
11
#include "myobject.h"
22
#include "../common.h"
33

4+
size_t finalize_count = 0;
5+
46
MyObject::MyObject() : env_(nullptr), wrapper_(nullptr) {}
57

6-
MyObject::~MyObject() { napi_delete_reference(env_, wrapper_); }
8+
MyObject::~MyObject() {
9+
finalize_count++;
10+
napi_delete_reference(env_, wrapper_);
11+
}
712

813
void MyObject::Destructor(
914
napi_env env, void* nativeObject, void* /*finalize_hint*/) {
@@ -45,6 +50,11 @@ napi_value MyObject::New(napi_env env, napi_callback_info info) {
4550
}
4651

4752
obj->env_ = env;
53+
54+
// It is important that the below call to napi_wrap() be such that we request
55+
// a reference to the wrapped object via the out-parameter, because this
56+
// ensures that we test the code path that deals with a reference that is
57+
// destroyed from its own finalizer.
4858
NAPI_CALL(env, napi_wrap(env,
4959
_this,
5060
obj,

‎test/addons-napi/8_passing_wrapped/test.js‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,16 @@
11
'use strict';
2+
// Flags: --expose-gc
3+
24
const common = require('../../common');
35
const assert = require('assert');
46
const addon = require(`./build/${common.buildType}/binding`);
57

6-
const obj1 = addon.createObject(10);
8+
let obj1 = addon.createObject(10);
79
const obj2 = addon.createObject(20);
810
const result = addon.add(obj1, obj2);
911
assert.strictEqual(result, 30);
12+
13+
// Make sure the native destructor gets called.
14+
obj1 = null;
15+
global.gc();
16+
assert.strictEqual(addon.finalizeCount(), 1);

0 commit comments

Comments
 (0)
0