Skip to content

Commit 6d6466f

Browse files
authored
allow napi_delete_reference in basic finalizers (#130)
1 parent 3cb78fb commit 6d6466f

File tree

8 files changed

+263
-6
lines changed

8 files changed

+263
-6
lines changed

.eslintrc.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@ const sharedRules = {
22
'spaced-comment': 'off',
33
'no-new-func': 'off',
44
'no-implied-eval': 'off',
5-
'no-var': 'off'
5+
'no-var': 'off',
6+
'camelcase': 'off'
67
}
78

89
module.exports = {

packages/emnapi/src/life.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,8 @@ export function napi_delete_reference (
9797
env: napi_env,
9898
ref: napi_ref
9999
): napi_status {
100-
const envObject: Env = $CHECK_ENV_NOT_IN_GC!(env)
100+
$CHECK_ENV!(env)
101+
const envObject = emnapiCtx.envStore.get(env)!
101102
$CHECK_ARG!(envObject, ref)
102103
emnapiCtx.refStore.get(ref)!.dispose()
103104
return envObject.clearLastError()

packages/test/CMakeLists.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,8 @@ add_test("promise" "./promise/binding.c" OFF)
274274
add_test("object" "./object/test_null.c;./object/test_object.c" OFF)
275275
add_test("object_exception" "./object/test_exceptions.c" OFF)
276276
add_test("objwrap" "./objwrap/myobject.cc" OFF)
277+
add_test("objwrapbasicfinalizer" "./objwrap/myobject.cc" OFF)
278+
target_compile_definitions("objwrapbasicfinalizer" PRIVATE "NAPI_EXPERIMENTAL")
277279
add_test("bigint" "./bigint/binding.c" OFF)
278280
add_test("fnwrap" "./fnwrap/myobject.cc;./fnwrap/binding.cc" OFF)
279281
add_test("passwrap" "./passwrap/myobject.cc;./passwrap/binding.cc" OFF)

packages/test/common.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,11 @@
1111

1212
#else
1313
#include <stddef.h>
14+
15+
EXTERN_C_START
1416
void console_error(const char* fmt, const char* str);
17+
EXTERN_C_END
18+
1519
#define abort() __builtin_trap()
1620
#define EPRINT(str) console_error("%s", (str))
1721
#endif

packages/test/gc.js

Lines changed: 192 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,192 @@
1+
'use strict'
2+
3+
const wait = require('timers/promises').setTimeout
4+
const assert = require('assert')
5+
const common = require('./common')
6+
const gcTrackerMap = new WeakMap()
7+
const gcTrackerTag = 'NODE_TEST_COMMON_GC_TRACKER'
8+
9+
/**
10+
* Installs a garbage collection listener for the specified object.
11+
* Uses async_hooks for GC tracking, which may affect test functionality.
12+
* A full setImmediate() invocation passes between a global.gc() call and the listener being invoked.
13+
* @param {object} obj - The target object to track for garbage collection.
14+
* @param {object} gcListener - The listener object containing the ongc callback.
15+
* @param {Function} gcListener.ongc - The function to call when the target object is garbage collected.
16+
*/
17+
function onGC (obj, gcListener) {
18+
const async_hooks = require('async_hooks')
19+
20+
const onGcAsyncHook = async_hooks.createHook({
21+
init: common.mustCallAtLeast(function (id, type) {
22+
if (this.trackedId === undefined) {
23+
assert.strictEqual(type, gcTrackerTag)
24+
this.trackedId = id
25+
}
26+
}),
27+
destroy (id) {
28+
assert.notStrictEqual(this.trackedId, -1)
29+
if (id === this.trackedId) {
30+
this.gcListener.ongc()
31+
onGcAsyncHook.disable()
32+
}
33+
}
34+
}).enable()
35+
onGcAsyncHook.gcListener = gcListener
36+
37+
gcTrackerMap.set(obj, new async_hooks.AsyncResource(gcTrackerTag))
38+
obj = null
39+
}
40+
41+
/**
42+
* Repeatedly triggers garbage collection until a specified condition is met or a maximum number of attempts is reached.
43+
* @param {string|Function} [name] - Optional name, used in the rejection message if the condition is not met.
44+
* @param {Function} condition - A function that returns true when the desired condition is met.
45+
* @returns {Promise} A promise that resolves when the condition is met, or rejects after 10 failed attempts.
46+
*/
47+
function gcUntil (name, condition) {
48+
if (typeof name === 'function') {
49+
condition = name
50+
name = undefined
51+
}
52+
return new Promise((resolve, reject) => {
53+
let count = 0
54+
function gcAndCheck () {
55+
setImmediate(() => {
56+
count++
57+
global.gc()
58+
if (condition()) {
59+
resolve()
60+
} else if (count < 10) {
61+
gcAndCheck()
62+
} else {
63+
reject(name === undefined ? undefined : 'Test ' + name + ' failed')
64+
}
65+
})
66+
}
67+
gcAndCheck()
68+
})
69+
}
70+
71+
// This function can be used to check if an object factor leaks or not,
72+
// but it needs to be used with care:
73+
// 1. The test should be set up with an ideally small
74+
// --max-old-space-size or --max-heap-size, which combined with
75+
// the maxCount parameter can reproduce a leak of the objects
76+
// created by fn().
77+
// 2. This works under the assumption that if *none* of the objects
78+
// created by fn() can be garbage-collected, the test would crash due
79+
// to OOM.
80+
// 3. If *any* of the objects created by fn() can be garbage-collected,
81+
// it is considered leak-free. The FinalizationRegistry is used to
82+
// terminate the test early once we detect any of the object is
83+
// garbage-collected to make the test less prone to false positives.
84+
// This may be especially important for memory management relying on
85+
// emphemeron GC which can be inefficient to deal with extremely fast
86+
// heap growth.
87+
// Note that this can still produce false positives. When the test using
88+
// this function still crashes due to OOM, inspect the heap to confirm
89+
// if a leak is present (e.g. using heap snapshots).
90+
// The generateSnapshotAt parameter can be used to specify a count
91+
// interval to create the heap snapshot which may enforce a more thorough GC.
92+
// This can be tried for code paths that require it for the GC to catch up
93+
// with heap growth. However this type of forced GC can be in conflict with
94+
// other logic in V8 such as bytecode aging, and it can slow down the test
95+
// significantly, so it should be used scarcely and only as a last resort.
96+
async function checkIfCollectable (
97+
fn, maxCount = 4096, generateSnapshotAt = Infinity, logEvery = 128) {
98+
let anyFinalized = false
99+
let count = 0
100+
101+
const f = new FinalizationRegistry(() => {
102+
anyFinalized = true
103+
})
104+
105+
async function createObject () {
106+
const obj = await fn()
107+
f.register(obj)
108+
if (count++ < maxCount && !anyFinalized) {
109+
setImmediate(createObject, 1)
110+
}
111+
// This can force a more thorough GC, but can slow the test down
112+
// significantly in a big heap. Use it with care.
113+
if (count % generateSnapshotAt === 0) {
114+
// XXX(joyeecheung): This itself can consume a bit of JS heap memory,
115+
// but the other alternative writeHeapSnapshot can run into disk space
116+
// not enough problems in the CI & be slower depending on file system.
117+
// Just do this for now as long as it works and only invent some
118+
// internal voodoo when we absolutely have no other choice.
119+
require('v8').getHeapSnapshot().pause().read()
120+
console.log(`Generated heap snapshot at ${count}`)
121+
}
122+
if (count % logEvery === 0) {
123+
console.log(`Created ${count} objects`)
124+
}
125+
if (anyFinalized) {
126+
console.log(`Found finalized object at ${count}, stop testing`)
127+
}
128+
}
129+
130+
createObject()
131+
}
132+
133+
// Repeat an operation and give GC some breathing room at every iteration.
134+
async function runAndBreathe (fn, repeat, waitTime = 20) {
135+
for (let i = 0; i < repeat; i++) {
136+
await fn()
137+
await wait(waitTime)
138+
}
139+
}
140+
141+
/**
142+
* This requires --expose-internals.
143+
* This function can be used to check if an object factory leaks or not by
144+
* iterating over the heap and count objects with the specified class
145+
* (which is checked by looking up the prototype chain).
146+
* @param {(i: number) => number} fn The factory receiving iteration count
147+
* and returning number of objects created. The return value should be
148+
* precise otherwise false negatives can be produced.
149+
* @param {Function} ctor The constructor of the objects being counted.
150+
* @param {number} count Number of iterations that this check should be done
151+
* @param {number} waitTime Optional breathing time for GC.
152+
*/
153+
async function checkIfCollectableByCounting (fn, ctor, count, waitTime = 20) {
154+
const { queryObjects } = require('v8')
155+
const { name } = ctor
156+
const initialCount = queryObjects(ctor, { format: 'count' })
157+
console.log(`Initial count of ${name}: ${initialCount}`)
158+
let totalCreated = 0
159+
for (let i = 0; i < count; ++i) {
160+
const created = await fn(i)
161+
totalCreated += created
162+
console.log(`#${i}: created ${created} ${name}, total ${totalCreated}`)
163+
await wait(waitTime) // give GC some breathing room.
164+
const currentCount = queryObjects(ctor, { format: 'count' })
165+
const collected = totalCreated - (currentCount - initialCount)
166+
console.log(`#${i}: counted ${currentCount} ${name}, collected ${collected}`)
167+
if (collected > 0) {
168+
console.log(`Detected ${collected} collected ${name}, finish early`)
169+
return
170+
}
171+
}
172+
173+
await wait(waitTime) // give GC some breathing room.
174+
const currentCount = queryObjects(ctor, { format: 'count' })
175+
const collected = totalCreated - (currentCount - initialCount)
176+
console.log(`Last count: counted ${currentCount} ${name}, collected ${collected}`)
177+
// Some objects with the prototype can be collected.
178+
if (collected > 0) {
179+
console.log(`Detected ${collected} collected ${name}`)
180+
return
181+
}
182+
183+
throw new Error(`${name} cannot be collected`)
184+
}
185+
186+
module.exports = {
187+
checkIfCollectable,
188+
runAndBreathe,
189+
checkIfCollectableByCounting,
190+
onGC,
191+
gcUntil
192+
}

packages/test/objwrap/myobject.cc

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,25 @@ void operator delete(void* p) noexcept {
1717
}
1818
#endif
1919

20+
typedef int32_t FinalizerData;
21+
2022
napi_ref MyObject::constructor;
2123

2224
MyObject::MyObject(double value)
2325
: value_(value), env_(nullptr), wrapper_(nullptr) {}
2426

2527
MyObject::~MyObject() { napi_delete_reference(env_, wrapper_); }
2628

27-
void MyObject::Destructor(
28-
napi_env env, void* nativeObject, void* /*finalize_hint*/) {
29+
void MyObject::Destructor(node_api_basic_env env,
30+
void* nativeObject,
31+
void* /*finalize_hint*/) {
2932
MyObject* obj = static_cast<MyObject*>(nativeObject);
3033
delete obj;
34+
35+
FinalizerData* data;
36+
NODE_API_BASIC_CALL_RETURN_VOID(
37+
env, napi_get_instance_data(env, reinterpret_cast<void**>(&data)));
38+
*data += 1;
3139
}
3240

3341
void MyObject::Init(napi_env env, napi_value exports) {
@@ -169,7 +177,7 @@ napi_value MyObject::Multiply(napi_env env, napi_callback_info info) {
169177
}
170178

171179
// This finalizer should never be invoked.
172-
void ObjectWrapDanglingReferenceFinalizer(napi_env env,
180+
void ObjectWrapDanglingReferenceFinalizer(node_api_basic_env env,
173181
void* finalize_data,
174182
void* finalize_hint) {
175183
__builtin_trap();
@@ -214,15 +222,38 @@ napi_value ObjectWrapDanglingReferenceTest(napi_env env,
214222
return ret;
215223
}
216224

225+
static napi_value GetFinalizerCallCount(napi_env env, napi_callback_info info) {
226+
size_t argc = 1;
227+
napi_value argv[1];
228+
FinalizerData* data;
229+
napi_value result;
230+
231+
NODE_API_CALL(env,
232+
napi_get_cb_info(env, info, &argc, argv, nullptr, nullptr));
233+
NODE_API_CALL(env,
234+
napi_get_instance_data(env, reinterpret_cast<void**>(&data)));
235+
NODE_API_CALL(env, napi_create_int32(env, *data, &result));
236+
return result;
237+
}
238+
239+
static void finalizeData(napi_env env, void* data, void* hint) {
240+
delete reinterpret_cast<FinalizerData*>(data);
241+
}
242+
217243
EXTERN_C_START
218244
napi_value Init(napi_env env, napi_value exports) {
245+
FinalizerData* data = new FinalizerData;
246+
*data = 0;
247+
NODE_API_CALL(env, napi_set_instance_data(env, data, finalizeData, nullptr));
248+
219249
MyObject::Init(env, exports);
220250

221251
napi_property_descriptor descriptors[] = {
222252
DECLARE_NODE_API_PROPERTY("objectWrapDanglingReference",
223253
ObjectWrapDanglingReference),
224254
DECLARE_NODE_API_PROPERTY("objectWrapDanglingReferenceTest",
225255
ObjectWrapDanglingReferenceTest),
256+
DECLARE_NODE_API_PROPERTY("getFinalizerCallCount", GetFinalizerCallCount),
226257
};
227258

228259
NODE_API_CALL(

packages/test/objwrap/myobject.h

Lines 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);
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/* eslint-disable symbol-description */
2+
/* eslint-disable camelcase */
3+
'use strict'
4+
const { load } = require('../util')
5+
const assert = require('assert')
6+
const { gcUntil } = require('../gc')
7+
8+
const p = load('objwrapbasicfinalizer')
9+
module.exports = p.then(async addon => {
10+
// This test verifies that ObjectWrap can be correctly finalized with a node_api_basic_finalizer
11+
// in the current JS loop tick
12+
(() => {
13+
let obj = new addon.MyObject(9)
14+
obj = null
15+
// Silent eslint about unused variables.
16+
assert.strictEqual(obj, null)
17+
})()
18+
19+
await gcUntil(() => {
20+
return addon.getFinalizerCallCount() === 1
21+
})
22+
23+
assert.strictEqual(addon.getFinalizerCallCount(), 1)
24+
})

0 commit comments

Comments
 (0)