Skip to content

node:fs: keep ArrayBuffer storage alive across worker.terminate() during async write - #36818

Open
robobun wants to merge 7 commits into
mainfrom
claude/c51f3b14/fs-write-worker-terminate-buffer-lifetime
Open

node:fs: keep ArrayBuffer storage alive across worker.terminate() during async write#36818
robobun wants to merge 7 commits into
mainfrom
claude/c51f3b14/fs-write-worker-terminate-buffer-lifetime

test: prove fs.read/readv destination buffer survives worker.terminate()

0946896
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed Aug 3, 2026 in 14m 58s

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/node/fs/fs-read-worker-terminate.test.ts:83 Read test uses negative error-string oracle (not.toContain AddressSanitizer)

Annotations

Check warning on line 83 in test/js/node/fs/fs-read-worker-terminate.test.ts

See this annotation in the file changed.

@claude claude / Claude Code Review

Read test uses negative error-string oracle (not.toContain AddressSanitizer)

Line 83's `expect(stderr).not.toContain("AddressSanitizer")` is the negative-error-string anti-pattern CLAUDE.md forbids — the same pattern (`not.toContain("use-after-free")`) was already flagged and removed from the sibling write test in 8cc9b96a, but this read test was added afterward and reintroduced it. It's also redundant: line 85's unconditional `expect(stdout).toMatch(/^PASS addr=.../m)` already fails when ASAN aborts (PASS never prints). Drop line 83 to match the write test's post-review