Skip to content

process: fix finalization cleanup ref removal#64087

Open
trivikr wants to merge 1 commit into
nodejs:mainfrom
trivikr:lib-internal-process-finalization
Open

process: fix finalization cleanup ref removal#64087
trivikr wants to merge 1 commit 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.


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
ArrayPrototypeSplice(refs[event], index, index + 1);
uninstall(event);
if (index !== -1) {
ArrayPrototypeSplice(refs[event], index, 1);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're effectively using these arrays as Sets, would it make more sense just to implement them as such?

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