Skip to content

process: fix finalization cleanup ref tracking#64087

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

process: fix finalization cleanup ref tracking#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

Fix process.finalization cleanup so collecting one registered
object does not accidentally remove later live finalization refs.

Finalization refs are now tracked with SafeSet collections instead
of arrays, which matches the identity-based add/remove behavior
used by registration, cleanup, and unregister paths.


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 4 times, most recently from 35a57c9 to ce00f56 Compare June 23, 2026 19:17
@trivikr trivikr requested a review from Renegade334 June 23, 2026 22:13
@Renegade334

Copy link
Copy Markdown
Member

I would just squash the commits, there's no need to keep the splice change in the commit log if we're immediately removing it. Otherwise LGTM!

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
@trivikr trivikr force-pushed the lib-internal-process-finalization branch from ce00f56 to 4ff51d8 Compare June 24, 2026 06:25
@trivikr

trivikr commented Jun 24, 2026

Copy link
Copy Markdown
Member Author

@Renegade334 Squashed into a single commit

@trivikr trivikr changed the title process: fix finalization cleanup ref removal process: fix finalization cleanup ref tracking Jun 24, 2026
@trivikr trivikr added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jun 24, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 24, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. needs-ci PRs that need a full CI run. process Issues and PRs related to the process subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

process.finalization removes extra refs during cleanup

3 participants