Skip to content

node:fs: mark the per-VM Binding box as LSan-ignored (fixes worker-terminate-lifetime.test.ts on main) - #35159

Open
robobun wants to merge 6 commits into
mainfrom
farm/fae20db5/node-fs-binding-lsan-false-positive
Open

node:fs: mark the per-VM Binding box as LSan-ignored (fixes worker-terminate-lifetime.test.ts on main)#35159
robobun wants to merge 6 commits into
mainfrom
farm/fae20db5/node-fs-binding-lsan-false-positive

node:fs: mark the per-VM Binding box as LSan-ignored

ec36bbc
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed Jul 22, 2026 in 7m 28s

Code review found 1 potential issue

Found 2 candidates, confirmed 1. See review comments for details.

Details

Severity Count
🔴 Important 0
🟡 Nit 1
🟣 Pre-existing 0
Severity File:Line Issue
🟡 Nit test/js/web/workers/worker-terminate-lifetime.test.ts:125-131 New test inserted mid-comment, splitting the c-ares regression comment block

Annotations

Check warning on line 131 in test/js/web/workers/worker-terminate-lifetime.test.ts

See this annotation in the file changed.

@claude claude / Claude Code Review

New test inserted mid-comment, splitting the c-ares regression comment block

The new test was inserted one line *into* the pre-existing c-ares regression comment, splitting it: line 124 (`// Regression: the per-VM c-ares channel was destroyed in deinit_runtime_state`) now sits above the unrelated node:fs test, and the dns.lookup test's comment at line 151 begins mid-sentence with `// (RuntimeState drop) AFTER JSC teardown...`. Move the new test + its comment to just above line 124 so the c-ares comment block stays contiguous with its own test.