Skip to content

Commit 3b90f34

Browse files
legendecaslegendecas
authored
node-api: add nested object wrap and napi_ref test
Test that an napi_ref can be nested inside another ObjectWrap. The test shows a critical case where a finalizer deletes an `napi_ref` whose finalizer is also scheduled. PR-URL: #57981 Reviewed-By: Vladimir Morozov <vmorozov@microsoft.com> Reviewed-By: Michael Dawson <midawson@redhat.com>
1 parent 25fe802 commit 3b90f34

File tree

8 files changed

+207
-26
lines changed

8 files changed

+207
-26
lines changed

‎test/js-native-api/6_object_wrap/binding.gyp

+16-5
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,28 @@
11
{
22
"targets": [
33
{
4-
"target_name": "6_object_wrap",
4+
"target_name": "myobject",
55
"sources": [
6-
"6_object_wrap.cc"
6+
"myobject.cc",
7+
"myobject.h",
78
]
89
},
910
{
10-
"target_name": "6_object_wrap_basic_finalizer",
11+
"target_name": "myobject_basic_finalizer",
1112
"defines": [ "NAPI_EXPERIMENTAL" ],
1213
"sources": [
13-
"6_object_wrap.cc"
14+
"myobject.cc",
15+
"myobject.h",
1416
]
15-
}
17+
},
18+
{
19+
"target_name": "nested_wrap",
20+
# Test without basic finalizers as it schedules differently.
21+
"defines": [ "NAPI_VERSION=10" ],
22+
"sources": [
23+
"nested_wrap.cc",
24+
"nested_wrap.h",
25+
],
26+
},
1627
]
1728
}

‎test/js-native-api/6_object_wrap/6_object_wrap.cc renamed to ‎test/js-native-api/6_object_wrap/myobject.cc

+36-18
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1+
#include "myobject.h"
12
#include "../common.h"
23
#include "../entry_point.h"
34
#include "assert.h"
4-
#include "myobject.h"
55

66
typedef int32_t FinalizerData;
77

@@ -10,7 +10,9 @@ napi_ref MyObject::constructor;
1010
MyObject::MyObject(double value)
1111
: value_(value), env_(nullptr), wrapper_(nullptr) {}
1212

13-
MyObject::~MyObject() { napi_delete_reference(env_, wrapper_); }
13+
MyObject::~MyObject() {
14+
napi_delete_reference(env_, wrapper_);
15+
}
1416

1517
void MyObject::Destructor(node_api_basic_env env,
1618
void* nativeObject,
@@ -26,24 +28,36 @@ void MyObject::Destructor(node_api_basic_env env,
2628

2729
void MyObject::Init(napi_env env, napi_value exports) {
2830
napi_property_descriptor properties[] = {
29-
{ "value", nullptr, nullptr, GetValue, SetValue, 0, napi_default, 0 },
30-
{ "valueReadonly", nullptr, nullptr, GetValue, nullptr, 0, napi_default,
31-
0 },
32-
DECLARE_NODE_API_PROPERTY("plusOne", PlusOne),
33-
DECLARE_NODE_API_PROPERTY("multiply", Multiply),
31+
{"value", nullptr, nullptr, GetValue, SetValue, 0, napi_default, 0},
32+
{"valueReadonly",
33+
nullptr,
34+
nullptr,
35+
GetValue,
36+
nullptr,
37+
0,
38+
napi_default,
39+
0},
40+
DECLARE_NODE_API_PROPERTY("plusOne", PlusOne),
41+
DECLARE_NODE_API_PROPERTY("multiply", Multiply),
3442
};
3543

3644
napi_value cons;
37-
NODE_API_CALL_RETURN_VOID(env, napi_define_class(
38-
env, "MyObject", -1, New, nullptr,
39-
sizeof(properties) / sizeof(napi_property_descriptor),
40-
properties, &cons));
45+
NODE_API_CALL_RETURN_VOID(
46+
env,
47+
napi_define_class(env,
48+
"MyObject",
49+
-1,
50+
New,
51+
nullptr,
52+
sizeof(properties) / sizeof(napi_property_descriptor),
53+
properties,
54+
&cons));
4155

4256
NODE_API_CALL_RETURN_VOID(env,
43-
napi_create_reference(env, cons, 1, &constructor));
57+
napi_create_reference(env, cons, 1, &constructor));
4458

45-
NODE_API_CALL_RETURN_VOID(env,
46-
napi_set_named_property(env, exports, "MyObject", cons));
59+
NODE_API_CALL_RETURN_VOID(
60+
env, napi_set_named_property(env, exports, "MyObject", cons));
4761
}
4862

4963
napi_value MyObject::New(napi_env env, napi_callback_info info) {
@@ -71,8 +85,12 @@ napi_value MyObject::New(napi_env env, napi_callback_info info) {
7185

7286
obj->env_ = env;
7387
NODE_API_CALL(env,
74-
napi_wrap(env, _this, obj, MyObject::Destructor,
75-
nullptr /* finalize_hint */, &obj->wrapper_));
88+
napi_wrap(env,
89+
_this,
90+
obj,
91+
MyObject::Destructor,
92+
nullptr /* finalize_hint */,
93+
&obj->wrapper_));
7694

7795
return _this;
7896
}
@@ -93,7 +111,7 @@ napi_value MyObject::New(napi_env env, napi_callback_info info) {
93111
napi_value MyObject::GetValue(napi_env env, napi_callback_info info) {
94112
napi_value _this;
95113
NODE_API_CALL(env,
96-
napi_get_cb_info(env, info, nullptr, nullptr, &_this, nullptr));
114+
napi_get_cb_info(env, info, nullptr, nullptr, &_this, nullptr));
97115

98116
MyObject* obj;
99117
NODE_API_CALL(env, napi_unwrap(env, _this, reinterpret_cast<void**>(&obj)));
@@ -121,7 +139,7 @@ napi_value MyObject::SetValue(napi_env env, napi_callback_info info) {
121139
napi_value MyObject::PlusOne(napi_env env, napi_callback_info info) {
122140
napi_value _this;
123141
NODE_API_CALL(env,
124-
napi_get_cb_info(env, info, nullptr, nullptr, &_this, nullptr));
142+
napi_get_cb_info(env, info, nullptr, nullptr, &_this, nullptr));
125143

126144
MyObject* obj;
127145
NODE_API_CALL(env, napi_unwrap(env, _this, reinterpret_cast<void**>(&obj)));

‎test/js-native-api/6_object_wrap/nested_wrap.cc

+99
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
#include "nested_wrap.h"
2+
#include "../common.h"
3+
#include "../entry_point.h"
4+
5+
napi_ref NestedWrap::constructor{};
6+
static int finalization_count = 0;
7+
8+
NestedWrap::NestedWrap() {}
9+
10+
NestedWrap::~NestedWrap() {
11+
napi_delete_reference(env_, wrapper_);
12+
13+
// Delete the nested reference as well.
14+
napi_delete_reference(env_, nested_);
15+
}
16+
17+
void NestedWrap::Destructor(node_api_basic_env env,
18+
void* nativeObject,
19+
void* /*finalize_hint*/) {
20+
// Once this destructor is called, it cancels all pending
21+
// finalizers for the object by deleting the references.
22+
NestedWrap* obj = static_cast<NestedWrap*>(nativeObject);
23+
delete obj;
24+
25+
finalization_count++;
26+
}
27+
28+
void NestedWrap::Init(napi_env env, napi_value exports) {
29+
napi_value cons;
30+
NODE_API_CALL_RETURN_VOID(
31+
env,
32+
napi_define_class(
33+
env, "NestedWrap", -1, New, nullptr, 0, nullptr, &cons));
34+
35+
NODE_API_CALL_RETURN_VOID(env,
36+
napi_create_reference(env, cons, 1, &constructor));
37+
38+
NODE_API_CALL_RETURN_VOID(
39+
env, napi_set_named_property(env, exports, "NestedWrap", cons));
40+
}
41+
42+
napi_value NestedWrap::New(napi_env env, napi_callback_info info) {
43+
napi_value new_target;
44+
NODE_API_CALL(env, napi_get_new_target(env, info, &new_target));
45+
bool is_constructor = (new_target != nullptr);
46+
NODE_API_BASIC_ASSERT_BASE(
47+
is_constructor, "Constructor called without new", nullptr);
48+
49+
napi_value this_val;
50+
NODE_API_CALL(env,
51+
napi_get_cb_info(env, info, 0, nullptr, &this_val, nullptr));
52+
53+
NestedWrap* obj = new NestedWrap();
54+
55+
obj->env_ = env;
56+
NODE_API_CALL(env,
57+
napi_wrap(env,
58+
this_val,
59+
obj,
60+
NestedWrap::Destructor,
61+
nullptr /* finalize_hint */,
62+
&obj->wrapper_));
63+
64+
// Create a second napi_ref to be deleted in the destructor.
65+
NODE_API_CALL(env,
66+
napi_add_finalizer(env,
67+
this_val,
68+
obj,
69+
NestedWrap::Destructor,
70+
nullptr /* finalize_hint */,
71+
&obj->nested_));
72+
73+
return this_val;
74+
}
75+
76+
static napi_value GetFinalizerCallCount(napi_env env, napi_callback_info info) {
77+
napi_value result;
78+
NODE_API_CALL(env, napi_create_int32(env, finalization_count, &result));
79+
return result;
80+
}
81+
82+
EXTERN_C_START
83+
napi_value Init(napi_env env, napi_value exports) {
84+
NestedWrap::Init(env, exports);
85+
86+
napi_property_descriptor descriptors[] = {
87+
DECLARE_NODE_API_PROPERTY("getFinalizerCallCount", GetFinalizerCallCount),
88+
};
89+
90+
NODE_API_CALL(
91+
env,
92+
napi_define_properties(env,
93+
exports,
94+
sizeof(descriptors) / sizeof(*descriptors),
95+
descriptors));
96+
97+
return exports;
98+
}
99+
EXTERN_C_END

‎test/js-native-api/6_object_wrap/nested_wrap.h

+33
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
#ifndef TEST_JS_NATIVE_API_6_OBJECT_WRAP_NESTED_WRAP_H_
2+
#define TEST_JS_NATIVE_API_6_OBJECT_WRAP_NESTED_WRAP_H_
3+
4+
#include <js_native_api.h>
5+
6+
/**
7+
* Test that an napi_ref can be nested inside another ObjectWrap.
8+
*
9+
* This test shows a critical case where a finalizer deletes an napi_ref
10+
* whose finalizer is also scheduled.
11+
*/
12+
13+
class NestedWrap {
14+
public:
15+
static void Init(napi_env env, napi_value exports);
16+
static void Destructor(node_api_basic_env env,
17+
void* nativeObject,
18+
void* finalize_hint);
19+
20+
private:
21+
explicit NestedWrap();
22+
~NestedWrap();
23+
24+
static napi_value New(napi_env env, napi_callback_info info);
25+
26+
static napi_ref constructor;
27+
28+
napi_env env_{};
29+
napi_ref wrapper_{};
30+
napi_ref nested_{};
31+
};
32+
33+
#endif // TEST_JS_NATIVE_API_6_OBJECT_WRAP_NESTED_WRAP_H_

‎test/js-native-api/6_object_wrap/nested_wrap.js

+20
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// Flags: --expose-gc
2+
3+
'use strict';
4+
const common = require('../../common');
5+
const { gcUntil } = require('../../common/gc');
6+
const assert = require('assert');
7+
const addon = require(`./build/${common.buildType}/nested_wrap`);
8+
9+
// This test verifies that ObjectWrap and napi_ref can be nested and finalized
10+
// correctly with a non-basic finalizer.
11+
(() => {
12+
let obj = new addon.NestedWrap();
13+
obj = null;
14+
// Silent eslint about unused variables.
15+
assert.strictEqual(obj, null);
16+
})();
17+
18+
gcUntil('object-wrap-ref', () => {
19+
return addon.getFinalizerCallCount() === 1;
20+
});

‎test/js-native-api/6_object_wrap/test-basic-finalizer.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
'use strict';
44
const common = require('../../common');
55
const assert = require('assert');
6-
const addon = require(`./build/${common.buildType}/6_object_wrap_basic_finalizer`);
6+
const addon = require(`./build/${common.buildType}/myobject_basic_finalizer`);
77

88
// This test verifies that ObjectWrap can be correctly finalized with a node_api_basic_finalizer
99
// in the current JS loop tick

‎test/js-native-api/6_object_wrap/test-object-wrap-ref.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
'use strict';
44
const common = require('../../common');
5-
const addon = require(`./build/${common.buildType}/6_object_wrap`);
5+
const addon = require(`./build/${common.buildType}/myobject`);
66
const { gcUntil } = require('../../common/gc');
77

88
(function scope() {

‎test/js-native-api/6_object_wrap/test.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
'use strict';
22
const common = require('../../common');
33
const assert = require('assert');
4-
const addon = require(`./build/${common.buildType}/6_object_wrap`);
4+
const addon = require(`./build/${common.buildType}/myobject`);
55

66
const getterOnlyErrorRE =
77
/^TypeError: Cannot set property .* of #<.*> which has only a getter$/;

0 commit comments

Comments
 (0)