Skip to content

ai slop - #29322

Closed
robobun wants to merge 2 commits into
mainfrom
farm/147c495c/worker-terminate-uaf
Closed

ai slop#29322
robobun wants to merge 2 commits into
mainfrom
farm/147c495c/worker-terminate-uaf

Worker: fix use-after-free when terminate() is called after the worke…

186d8d4
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed Apr 15, 2026 in 18m 36s

Code review found 2 potential issues

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

Details

Severity Count
🔴 Important 0
🟡 Nit 1
🟣 Pre-existing 1
Severity File:Line Issue
🟡 Nit test/js/web/workers/worker-terminate-after-exit.test.ts:39-41 Test assertion inverted: stderr check gives misleading failure output on crash
🟣 Pre-existing src/bun.js/web_worker.zig:408-418 Memory leak: worker name field allocated in create() but not freed in deinit()

Annotations

Check warning on line 41 in test/js/web/workers/worker-terminate-after-exit.test.ts

See this annotation in the file changed.

@claude claude / Claude Code Review

Test assertion inverted: stderr check gives misleading failure output on crash

The assertion at lines 39-41 is semantically inverted: when the process exits with a non-zero code (e.g., ASAN abort with exit code 134), stderr holds the crash trace, so `expect(stderr).toBe("")` fires first with the misleading message "Expected empty stderr" rather than surfacing the actual failure (non-zero exit code). The test still catches the use-after-free (no false negatives), but failure diagnostics are backwards. The fix is `expect(exitCode, `stderr: ${stderr}`).toBe(0)`, which makes t

Check notice on line 418 in src/bun.js/web_worker.zig

See this annotation in the file changed.

@claude claude / Claude Code Review

Memory leak: worker name field allocated in create() but not freed in deinit()

The `name` field is heap-allocated in `create()` when a non-empty name is provided, but `deinit()` never frees it, leaking the string for every named worker. This is a pre-existing issue, but the PR explicitly restructures `deinit()` as the canonical resource-release function (adding idempotency guards for `unresolved_specifier` and `preloads`), making the omission of `name` a conspicuous gap worth fixing at this point.