Skip to content

webcore: sweep worker-owned blob URLs on Worker shutdown - #35827

Open
robobun wants to merge 4 commits into
mainfrom
farm/3590bb11/worker-blob-url-sweep
Open

webcore: sweep worker-owned blob URLs on Worker shutdown#35827
robobun wants to merge 4 commits into
mainfrom
farm/3590bb11/worker-blob-url-sweep

webcore: sweep worker-owned blob URLs on Worker shutdown

4eb55ba
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed Jul 25, 2026 in 16m 51s

Code review found 2 potential issues

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

Details

Severity Count
🔴 Important 0
🟡 Nit 2
🟣 Pre-existing 0
Severity File:Line Issue
🟡 Nit test/js/web/workers/worker_blob.test.ts:137-139 Windows skip drops all functional coverage of the sweep, not just the RSS check
🟡 Nit test/js/web/workers/worker_blob.test.ts:159-161 runWorker() promise has no rejection path — hangs on worker error

Annotations

Check warning on line 139 in test/js/web/workers/worker_blob.test.ts

See this annotation in the file changed.

@claude claude / Claude Code Review

Windows skip drops all functional coverage of the sweep, not just the RSS check

The `test.skipIf(isWindows)` gates the entire test because RSS doesn't reliably drop on Windows, but that also skips the two RSS-independent behavioral assertions — `deadUrlServed === false` and `parentUrlStillResolves === true` — which are the direct checks of the sweep semantics. Consider running the test on all platforms and gating only the `expect(growthMB).toBeLessThan(40)` line on `!isWindows`, so Windows CI still covers the functional behavior (per REVIEW.md: "branch per-platform rather t

Check warning on line 161 in test/js/web/workers/worker_blob.test.ts

See this annotation in the file changed.

@claude claude / Claude Code Review

runWorker() promise has no rejection path — hangs on worker error

The `runWorker()` helper awaits `new Promise(r => (w.onmessage = e => r(e.data)))` with no rejection path — if a worker errors before `postMessage` (thread spawn failure, OOM on the 4×1 MB allocs under ASAN, etc.), the promise never settles and the subprocess hangs until the outer 60 s timeout with no diagnostic. REVIEW.md ('Tests reviewers reject') is categorical: *'Wire EVERY failure event (`error`, `close`, `abort`, process exit) to reject the awaited promise'*. Add `w.onerror = reject` (usin