Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit f883bed

Browse filesBrowse files
legendecasaduh95
legendecas
authored and
aduh95
committed
node-api: allow napi_delete_reference in finalizers
`napi_delete_reference` must be called immediately for a `napi_reference` returned from `napi_wrap` in the corresponding finalizer, in order to clean up the `napi_reference` timely. `napi_delete_reference` is safe to be invoked during GC. PR-URL: #55620 Reviewed-By: Gabriel Schulhof <gabrielschulhof@gmail.com> Reviewed-By: Michael Dawson <midawson@redhat.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Vladimir Morozov <vmorozov@microsoft.com>
1 parent 65bc8e8 commit f883bed
Copy full SHA for f883bed

File tree

Expand file treeCollapse file tree

5 files changed

+71
-5
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

5 files changed

+71
-5
lines changed
Open diff view settings
Collapse file

‎src/js_native_api_v8.cc‎

Copy file name to clipboardExpand all lines: src/js_native_api_v8.cc
+3-1Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2769,10 +2769,12 @@ napi_status NAPI_CDECL napi_create_reference(napi_env env,
27692769

27702770
// Deletes a reference. The referenced value is released, and may be GC'd unless
27712771
// there are other references to it.
2772+
// For a napi_reference returned from `napi_wrap`, this must be called in the
2773+
// finalizer.
27722774
napi_status NAPI_CDECL napi_delete_reference(napi_env env, napi_ref ref) {
27732775
// Omit NAPI_PREAMBLE and GET_RETURN_STATUS because V8 calls here cannot throw
27742776
// JS exceptions.
2775-
CHECK_ENV_NOT_IN_GC(env);
2777+
CHECK_ENV(env);
27762778
CHECK_ARG(env, ref);
27772779

27782780
delete reinterpret_cast<v8impl::Reference*>(ref);
Collapse file

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

Copy file name to clipboardExpand all lines: test/js-native-api/6_object_wrap/6_object_wrap.cc
+34-3Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,25 @@
33
#include "assert.h"
44
#include "myobject.h"
55

6+
typedef int32_t FinalizerData;
7+
68
napi_ref MyObject::constructor;
79

810
MyObject::MyObject(double value)
911
: value_(value), env_(nullptr), wrapper_(nullptr) {}
1012

1113
MyObject::~MyObject() { napi_delete_reference(env_, wrapper_); }
1214

13-
void MyObject::Destructor(
14-
napi_env env, void* nativeObject, void* /*finalize_hint*/) {
15+
void MyObject::Destructor(node_api_basic_env env,
16+
void* nativeObject,
17+
void* /*finalize_hint*/) {
1518
MyObject* obj = static_cast<MyObject*>(nativeObject);
1619
delete obj;
20+
21+
FinalizerData* data;
22+
NODE_API_BASIC_CALL_RETURN_VOID(
23+
env, napi_get_instance_data(env, reinterpret_cast<void**>(&data)));
24+
*data += 1;
1725
}
1826

1927
void MyObject::Init(napi_env env, napi_value exports) {
@@ -154,7 +162,7 @@ napi_value MyObject::Multiply(napi_env env, napi_callback_info info) {
154162
}
155163

156164
// This finalizer should never be invoked.
157-
void ObjectWrapDanglingReferenceFinalizer(napi_env env,
165+
void ObjectWrapDanglingReferenceFinalizer(node_api_basic_env env,
158166
void* finalize_data,
159167
void* finalize_hint) {
160168
assert(0 && "unreachable");
@@ -198,15 +206,38 @@ napi_value ObjectWrapDanglingReferenceTest(napi_env env,
198206
return ret;
199207
}
200208

209+
static napi_value GetFinalizerCallCount(napi_env env, napi_callback_info info) {
210+
size_t argc = 1;
211+
napi_value argv[1];
212+
FinalizerData* data;
213+
napi_value result;
214+
215+
NODE_API_CALL(env,
216+
napi_get_cb_info(env, info, &argc, argv, nullptr, nullptr));
217+
NODE_API_CALL(env,
218+
napi_get_instance_data(env, reinterpret_cast<void**>(&data)));
219+
NODE_API_CALL(env, napi_create_int32(env, *data, &result));
220+
return result;
221+
}
222+
223+
static void finalizeData(napi_env env, void* data, void* hint) {
224+
delete reinterpret_cast<FinalizerData*>(data);
225+
}
226+
201227
EXTERN_C_START
202228
napi_value Init(napi_env env, napi_value exports) {
229+
FinalizerData* data = new FinalizerData;
230+
*data = 0;
231+
NODE_API_CALL(env, napi_set_instance_data(env, data, finalizeData, nullptr));
232+
203233
MyObject::Init(env, exports);
204234

205235
napi_property_descriptor descriptors[] = {
206236
DECLARE_NODE_API_PROPERTY("objectWrapDanglingReference",
207237
ObjectWrapDanglingReference),
208238
DECLARE_NODE_API_PROPERTY("objectWrapDanglingReferenceTest",
209239
ObjectWrapDanglingReferenceTest),
240+
DECLARE_NODE_API_PROPERTY("getFinalizerCallCount", GetFinalizerCallCount),
210241
};
211242

212243
NODE_API_CALL(
Collapse file

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

Copy file name to clipboardExpand all lines: test/js-native-api/6_object_wrap/binding.gyp
+7Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,13 @@
55
"sources": [
66
"6_object_wrap.cc"
77
]
8+
},
9+
{
10+
"target_name": "6_object_wrap_basic_finalizer",
11+
"defines": [ "NAPI_EXPERIMENTAL" ],
12+
"sources": [
13+
"6_object_wrap.cc"
14+
]
815
}
916
]
1017
}
Collapse file

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

Copy file name to clipboardExpand all lines: test/js-native-api/6_object_wrap/myobject.h
+3-1Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@
66
class MyObject {
77
public:
88
static void Init(napi_env env, napi_value exports);
9-
static void Destructor(napi_env env, void* nativeObject, void* finalize_hint);
9+
static void Destructor(node_api_basic_env env,
10+
void* nativeObject,
11+
void* finalize_hint);
1012

1113
private:
1214
explicit MyObject(double value_ = 0);
Collapse file

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

Copy file name to clipboard
+24Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// Flags: --expose-gc
2+
3+
'use strict';
4+
const common = require('../../common');
5+
const assert = require('assert');
6+
const addon = require(`./build/${common.buildType}/6_object_wrap_basic_finalizer`);
7+
8+
// This test verifies that ObjectWrap can be correctly finalized with a node_api_basic_finalizer
9+
// in the current JS loop tick
10+
(() => {
11+
let obj = new addon.MyObject(9);
12+
obj = null;
13+
// Silent eslint about unused variables.
14+
assert.strictEqual(obj, null);
15+
})();
16+
17+
for (let i = 0; i < 10; ++i) {
18+
global.gc();
19+
if (addon.getFinalizerCallCount() === 1) {
20+
break;
21+
}
22+
}
23+
24+
assert.strictEqual(addon.getFinalizerCallCount(), 1);

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.