From 4ff51d8e7ac35812027933137dbd2f4f87c1348f Mon Sep 17 00:00:00 2001 From: "Kamat, Trivikram" <16024985+trivikr@users.noreply.github.com> Date: Mon, 22 Jun 2026 22:37:15 -0700 Subject: [PATCH] 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 --- lib/internal/process/finalization.js | 33 +++++++++--------- .../fixtures/process/finalization-cleanup.mjs | 34 +++++++++++++++++++ test/parallel/test-process-finalization.mjs | 1 + 3 files changed, 51 insertions(+), 17 deletions(-) create mode 100644 test/fixtures/process/finalization-cleanup.mjs diff --git a/lib/internal/process/finalization.js b/lib/internal/process/finalization.js index e5f748c37642fd..5efc2c8d78f9b7 100644 --- a/lib/internal/process/finalization.js +++ b/lib/internal/process/finalization.js @@ -2,11 +2,8 @@ // This file is a modified version of the on-exit-leak-free module on npm. const { - ArrayPrototypeFilter, - ArrayPrototypeIndexOf, - ArrayPrototypePush, - ArrayPrototypeSplice, SafeFinalizationRegistry, + SafeSet, SafeWeakRef, } = primordials; const { validateObject, kValidateObjectAllowFunction } = require('internal/validators'); @@ -20,8 +17,8 @@ function createFinalization() { const refs = { __proto__: null, - exit: [], - beforeExit: [], + exit: new SafeSet(), + beforeExit: new SafeSet(), }; const functions = { @@ -31,7 +28,7 @@ function createFinalization() { }; function install(event) { - if (refs[event].length > 0) { + if (refs[event].size > 0) { return; } @@ -39,13 +36,13 @@ function createFinalization() { } function uninstall(event) { - if (refs[event].length > 0) { + if (refs[event].size > 0) { return; } process.removeListener(event, functions[event]); - if (refs.exit.length === 0 && refs.beforeExit.length === 0) { + if (refs.exit.size === 0 && refs.beforeExit.size === 0) { registry = null; } } @@ -70,14 +67,14 @@ function createFinalization() { fn(obj, event); } } - refs[event] = []; + refs[event].clear(); } function clear(ref) { for (const event of ['exit', 'beforeExit']) { - const index = ArrayPrototypeIndexOf(refs[event], ref); - ArrayPrototypeSplice(refs[event], index, index + 1); - uninstall(event); + if (refs[event].delete(ref)) { + uninstall(event); + } } } @@ -90,7 +87,7 @@ function createFinalization() { registry ||= new SafeFinalizationRegistry(clear); registry.register(obj, ref); - ArrayPrototypePush(refs[event], ref); + refs[event].add(ref); } /** @@ -130,10 +127,12 @@ function createFinalization() { } registry.unregister(obj); for (const event of ['exit', 'beforeExit']) { - refs[event] = ArrayPrototypeFilter(refs[event], (ref) => { + for (const ref of refs[event]) { const _obj = ref.deref(); - return _obj && _obj !== obj; - }); + if (!_obj || _obj === obj) { + refs[event].delete(ref); + } + } uninstall(event); } } diff --git a/test/fixtures/process/finalization-cleanup.mjs b/test/fixtures/process/finalization-cleanup.mjs new file mode 100644 index 00000000000000..88d6c17cc7e30f --- /dev/null +++ b/test/fixtures/process/finalization-cleanup.mjs @@ -0,0 +1,34 @@ +import { deepStrictEqual } from 'assert' +import { setImmediate } from 'timers/promises' + +const keptAlive = [] +const finalized = [] + +function onFinalize(obj) { + finalized.push(obj.name) +} + +{ + const first = { name: 'first' } + let collected = { name: 'collected' } + const third = { name: 'third' } + + keptAlive.push(first, third) + + process.finalization.register(first, onFinalize) + process.finalization.register(collected, onFinalize) + process.finalization.register(third, onFinalize) + + collected = null +} + +// Give V8 a few chances to collect `collected` and run the +// FinalizationRegistry cleanup before process exit. +for (let i = 0; i < 10; i++) { + gc() + await setImmediate() +} + +process.on('exit', function () { + deepStrictEqual(finalized, ['first', 'third']) +}) diff --git a/test/parallel/test-process-finalization.mjs b/test/parallel/test-process-finalization.mjs index 5d3222aaca67d4..dddd0b8fae19d7 100644 --- a/test/parallel/test-process-finalization.mjs +++ b/test/parallel/test-process-finalization.mjs @@ -9,6 +9,7 @@ import assert from 'assert'; const files = [ 'close.mjs', 'before-exit.mjs', + 'finalization-cleanup.mjs', 'gc-not-close.mjs', 'unregister.mjs', 'different-registry-per-thread.mjs',