Skip to content

Commit 75aec8e

Browse files
authored
process: fix finalization cleanup ref tracking
Use SafeSet collections for finalization refs so insertion, removal, and emptiness checks match the identity-based tracking model. This also fixes cleanup removal for collected refs. Previously cleanup used the ref index as the splice delete count, which could remove later live refs when the collected ref was not first. Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com> Assisted-by: openai:gpt-5.5 PR-URL: #64087 Fixes: #64086 Reviewed-By: René <contact.9a5d6388@renegade334.me.uk>
1 parent c4429c8 commit 75aec8e

3 files changed

Lines changed: 51 additions & 17 deletions

File tree

lib/internal/process/finalization.js

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,8 @@
22
// This file is a modified version of the on-exit-leak-free module on npm.
33

44
const {
5-
ArrayPrototypeFilter,
6-
ArrayPrototypeIndexOf,
7-
ArrayPrototypePush,
8-
ArrayPrototypeSplice,
95
SafeFinalizationRegistry,
6+
SafeSet,
107
SafeWeakRef,
118
} = primordials;
129
const { validateObject, kValidateObjectAllowFunction } = require('internal/validators');
@@ -20,8 +17,8 @@ function createFinalization() {
2017

2118
const refs = {
2219
__proto__: null,
23-
exit: [],
24-
beforeExit: [],
20+
exit: new SafeSet(),
21+
beforeExit: new SafeSet(),
2522
};
2623

2724
const functions = {
@@ -31,21 +28,21 @@ function createFinalization() {
3128
};
3229

3330
function install(event) {
34-
if (refs[event].length > 0) {
31+
if (refs[event].size > 0) {
3532
return;
3633
}
3734

3835
process.on(event, functions[event]);
3936
}
4037

4138
function uninstall(event) {
42-
if (refs[event].length > 0) {
39+
if (refs[event].size > 0) {
4340
return;
4441
}
4542

4643
process.removeListener(event, functions[event]);
4744

48-
if (refs.exit.length === 0 && refs.beforeExit.length === 0) {
45+
if (refs.exit.size === 0 && refs.beforeExit.size === 0) {
4946
registry = null;
5047
}
5148
}
@@ -70,14 +67,14 @@ function createFinalization() {
7067
fn(obj, event);
7168
}
7269
}
73-
refs[event] = [];
70+
refs[event].clear();
7471
}
7572

7673
function clear(ref) {
7774
for (const event of ['exit', 'beforeExit']) {
78-
const index = ArrayPrototypeIndexOf(refs[event], ref);
79-
ArrayPrototypeSplice(refs[event], index, index + 1);
80-
uninstall(event);
75+
if (refs[event].delete(ref)) {
76+
uninstall(event);
77+
}
8178
}
8279
}
8380

@@ -90,7 +87,7 @@ function createFinalization() {
9087
registry ||= new SafeFinalizationRegistry(clear);
9188
registry.register(obj, ref);
9289

93-
ArrayPrototypePush(refs[event], ref);
90+
refs[event].add(ref);
9491
}
9592

9693
/**
@@ -130,10 +127,12 @@ function createFinalization() {
130127
}
131128
registry.unregister(obj);
132129
for (const event of ['exit', 'beforeExit']) {
133-
refs[event] = ArrayPrototypeFilter(refs[event], (ref) => {
130+
for (const ref of refs[event]) {
134131
const _obj = ref.deref();
135-
return _obj && _obj !== obj;
136-
});
132+
if (!_obj || _obj === obj) {
133+
refs[event].delete(ref);
134+
}
135+
}
137136
uninstall(event);
138137
}
139138
}
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
import { deepStrictEqual } from 'assert'
2+
import { setImmediate } from 'timers/promises'
3+
4+
const keptAlive = []
5+
const finalized = []
6+
7+
function onFinalize(obj) {
8+
finalized.push(obj.name)
9+
}
10+
11+
{
12+
const first = { name: 'first' }
13+
let collected = { name: 'collected' }
14+
const third = { name: 'third' }
15+
16+
keptAlive.push(first, third)
17+
18+
process.finalization.register(first, onFinalize)
19+
process.finalization.register(collected, onFinalize)
20+
process.finalization.register(third, onFinalize)
21+
22+
collected = null
23+
}
24+
25+
// Give V8 a few chances to collect `collected` and run the
26+
// FinalizationRegistry cleanup before process exit.
27+
for (let i = 0; i < 10; i++) {
28+
gc()
29+
await setImmediate()
30+
}
31+
32+
process.on('exit', function () {
33+
deepStrictEqual(finalized, ['first', 'third'])
34+
})

test/parallel/test-process-finalization.mjs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import assert from 'assert';
99
const files = [
1010
'close.mjs',
1111
'before-exit.mjs',
12+
'finalization-cleanup.mjs',
1213
'gc-not-close.mjs',
1314
'unregister.mjs',
1415
'different-registry-per-thread.mjs',

0 commit comments

Comments
 (0)