FinalizationRegistry: keep the registry alive while it has registrations - #35213
Open
robobun wants to merge 5 commits into
Open
FinalizationRegistry: keep the registry alive while it has registrations#35213robobun wants to merge 5 commits into
robobun wants to merge 5 commits into
Claude / Claude Code Review
completed
Jul 23, 2026 in 26m 6s
Code review found 1 potential issue
Found 1 candidates, confirmed 1. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 0 |
| 🟡 Nit | 1 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🟡 Nit | src/jsc/bindings/JSCTaskScheduler.h:36 |
Header comment inverts retention invariant: 'shortest-lived' should be 'longest-lived' |
Annotations
Check warning on line 36 in src/jsc/bindings/JSCTaskScheduler.h
claude / Claude Code Review
Header comment inverts retention invariant: 'shortest-lived' should be 'longest-lived'
The comment says "Retention is bound to the **shortest**-lived target", but `unrootFinalizationRegistryIfDrained` only drops the Strong root once `liveCount + deadCount == 0` — i.e. after *every* target has died — so retention is bound to the **longest**-lived target. The very next clause ("a target that never dies … kept until VM shutdown") is exactly the longest-lived case, making the sentence self-contradictory as written. One-word fix: `s/shortest-lived/longest-lived/`.
Loading