Skip to content

Commit bdf3262

Browse files
trivikrrichardlau
authored andcommitted
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: #64087Fixes: #64086 Reviewed-By: René <contact.9a5d6388@renegade334.me.uk>
1 parent 9e50bb0 commit bdf3262

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
constrefs={
2219
__proto__: null,
23-
exit: [],
24-
beforeExit: [],
20+
exit: newSafeSet(),
21+
beforeExit: newSafeSet(),
2522
};
2623

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

3330
functioninstall(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
functionuninstall(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
functionclear(ref){
7774
for(consteventof['exit','beforeExit']){
78-
constindex=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||=newSafeFinalizationRegistry(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(consteventof['exit','beforeExit']){
133-
refs[event]=ArrayPrototypeFilter(refs[event],(ref)=>{
130+
for(constrefofrefs[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+
constkeptAlive=[]
5+
constfinalized=[]
6+
7+
functiononFinalize(obj){
8+
finalized.push(obj.name)
9+
}
10+
11+
{
12+
constfirst={name: 'first'}
13+
letcollected={name: 'collected'}
14+
constthird={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(leti=0;i<10;i++){
28+
gc()
29+
awaitsetImmediate()
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
constfiles=[
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)