Skip to content

process: fix finalization cleanup ref removal#64087

Open
trivikr wants to merge 2 commits into
nodejs:mainfrom
trivikr:lib-internal-process-finalization
Open

process: fix finalization cleanup ref removal#64087
trivikr wants to merge 2 commits into
nodejs:mainfrom
trivikr:lib-internal-process-finalization

Conversation

@trivikr

@trivikr trivikr commented Jun 23, 2026

Copy link
Copy Markdown
Member

Fixes: #64086


When a registered object is garbage-collected, the cleanup path removes its
WeakRef from the internal refs arrays. That path passed index + 1 as the
second argument to splice(), but splice() expects a delete count, not an end
index. If the collected ref was not at index 0, cleanup could also remove
later refs that were still live.

This updates cleanup to remove exactly one matching ref, and only when the ref
is present. A regression fixture registers three refs, lets the middle one be
collected, and verifies that the first and third live refs still run their
callbacks on process exit.


This also updates lib/internal/process/finalization.js to store finalization refs in
SafeSet collections instead of arrays.

The refs were already being used with Set-like semantics: checking whether the
collection is empty, deleting refs by identity, and iterating over the active
refs. Using SafeSet makes that behavior explicit and simplifies cleanup logic.

Duplicate registrations preserve the existing behavior because the set stores
distinct WeakRef records, not the target objects.


Assisted-by: openai:gpt-5.5

Finalization cleanup used the ref index as part of the splice delete
count. When a collected ref was not the first entry, cleanup could
remove additional live refs from the internal list.

Remove exactly the collected ref when it is present, so later live refs
remain registered and still run their callbacks on process exit.

Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: openai:gpt-5.5
@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. process Issues and PRs related to the process subsystem. labels Jun 23, 2026
@trivikr trivikr added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 23, 2026
Comment thread lib/internal/process/finalization.js Outdated
@trivikr trivikr self-assigned this Jun 23, 2026
@trivikr trivikr force-pushed the lib-internal-process-finalization branch from 8af632e to eb35666 Compare June 23, 2026 15:23
Store finalization refs in SafeSet collections instead of arrays. This
makes identity-based insertion, deletion, and emptiness checks match
how the collections are used.

Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: openai:gpt-5.5
@trivikr trivikr force-pushed the lib-internal-process-finalization branch from eb35666 to a4efd08 Compare June 23, 2026 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ci PRs that need a full CI run. process Issues and PRs related to the process subsystem. request-ci Add this label to start a Jenkins CI on a PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

process.finalization removes extra refs during cleanup

3 participants