Commit bdf3262
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 9e50bb0 commit bdf3262
3 files changed
Lines changed: 51 additions & 17 deletions
File tree
- lib/internal/process
- test
- fixtures/process
- parallel
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -2,11 +2,8 @@ | |||
| 2 | 2 | // This file is a modified version of the on-exit-leak-free module on npm. | |
| 3 | 3 | ||
| 4 | 4 | const { | |
| 5 | - ArrayPrototypeFilter, | ||
| 6 | - ArrayPrototypeIndexOf, | ||
| 7 | - ArrayPrototypePush, | ||
| 8 | - ArrayPrototypeSplice, | ||
| 9 | 5 | SafeFinalizationRegistry, | |
| 6 | + SafeSet, | ||
| 10 | 7 | SafeWeakRef, | |
| 11 | 8 | } = primordials; | |
| 12 | 9 | const { validateObject, kValidateObjectAllowFunction } = require('internal/validators'); | |
@@ -20,8 +17,8 @@ function createFinalization() { | |||
| 20 | 17 | ||
| 21 | 18 | const refs = { | |
| 22 | 19 | __proto__: null, | |
| 23 | - exit: [], | ||
| 24 | - beforeExit: [], | ||
| 20 | + exit: new SafeSet(), | ||
| 21 | + beforeExit: new SafeSet(), | ||
| 25 | 22 | }; | |
| 26 | 23 | ||
| 27 | 24 | const functions = { | |
@@ -31,21 +28,21 @@ function createFinalization() { | |||
| 31 | 28 | }; | |
| 32 | 29 | ||
| 33 | 30 | function install(event) { | |
| 34 | - if (refs[event].length > 0) { | ||
| 31 | + if (refs[event].size > 0) { | ||
| 35 | 32 | return; | |
| 36 | 33 | } | |
| 37 | 34 | ||
| 38 | 35 | process.on(event, functions[event]); | |
| 39 | 36 | } | |
| 40 | 37 | ||
| 41 | 38 | function uninstall(event) { | |
| 42 | - if (refs[event].length > 0) { | ||
| 39 | + if (refs[event].size > 0) { | ||
| 43 | 40 | return; | |
| 44 | 41 | } | |
| 45 | 42 | ||
| 46 | 43 | process.removeListener(event, functions[event]); | |
| 47 | 44 | ||
| 48 | - if (refs.exit.length === 0 && refs.beforeExit.length === 0) { | ||
| 45 | + if (refs.exit.size === 0 && refs.beforeExit.size === 0) { | ||
| 49 | 46 | registry = null; | |
| 50 | 47 | } | |
| 51 | 48 | } | |
@@ -70,14 +67,14 @@ function createFinalization() { | |||
| 70 | 67 | fn(obj, event); | |
| 71 | 68 | } | |
| 72 | 69 | } | |
| 73 | - refs[event] = []; | ||
| 70 | + refs[event].clear(); | ||
| 74 | 71 | } | |
| 75 | 72 | ||
| 76 | 73 | function clear(ref) { | |
| 77 | 74 | 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 | + } | ||
| 81 | 78 | } | |
| 82 | 79 | } | |
| 83 | 80 | ||
@@ -90,7 +87,7 @@ function createFinalization() { | |||
| 90 | 87 | registry ||= new SafeFinalizationRegistry(clear); | |
| 91 | 88 | registry.register(obj, ref); | |
| 92 | 89 | ||
| 93 | - ArrayPrototypePush(refs[event], ref); | ||
| 90 | + refs[event].add(ref); | ||
| 94 | 91 | } | |
| 95 | 92 | ||
| 96 | 93 | /** | |
@@ -130,10 +127,12 @@ function createFinalization() { | |||
| 130 | 127 | } | |
| 131 | 128 | registry.unregister(obj); | |
| 132 | 129 | for (const event of ['exit', 'beforeExit']) { | |
| 133 | - refs[event] = ArrayPrototypeFilter(refs[event], (ref) => { | ||
| 130 | + for (const ref of refs[event]) { | ||
| 134 | 131 | const _obj = ref.deref(); | |
| 135 | - return _obj && _obj !== obj; | ||
| 136 | - }); | ||
| 132 | + if (!_obj || _obj === obj) { | ||
| 133 | + refs[event].delete(ref); | ||
| 134 | + } | ||
| 135 | + } | ||
| 137 | 136 | uninstall(event); | |
| 138 | 137 | } | |
| 139 | 138 | } | |
| Original file line number | Diff line number | Diff 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 | + }) | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -9,6 +9,7 @@ import assert from 'assert'; | |||
| 9 | 9 | const files = [ | |
| 10 | 10 | 'close.mjs', | |
| 11 | 11 | 'before-exit.mjs', | |
| 12 | + 'finalization-cleanup.mjs', | ||
| 12 | 13 | 'gc-not-close.mjs', | |
| 13 | 14 | 'unregister.mjs', | |
| 14 | 15 | 'different-registry-per-thread.mjs', | |
0 commit comments