webcore(Blob): coalesce concurrent fd-backed blob reads onto a single ReadFile - #35832
Open
robobun wants to merge 13 commits into
Open
webcore(Blob): coalesce concurrent fd-backed blob reads onto a single ReadFile#35832robobun wants to merge 13 commits into
robobun wants to merge 13 commits into
Claude / Claude Code Review
completed
Jul 25, 2026 in 14m 21s
Code review found 1 important issue
Found 6 candidates, confirmed 2. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 1 |
| 🟡 Nit | 1 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🔴 Important | src/runtime/webcore/blob/read_file.rs:423-443 |
Coalesced fd reads ignore the second caller's offset/size |
| 🟡 Nit | src/runtime/webcore/blob/read_file.rs:437-442 |
Stacked Borrows: &ReadFile on JS thread aliases &mut ReadFile on work pool |
Annotations
Check failure on line 443 in src/runtime/webcore/blob/read_file.rs
claude / Claude Code Review
Coalesced fd reads ignore the second caller's offset/size
`try_coalesce_fd_read` attaches a second caller to the in-flight `ReadFile` without checking that its `offset`/`size` match the primary reader's. Since `Blob.slice()` shares the same `Store`, `Promise.all([Bun.stdin.slice(0, 3).text(), Bun.stdin.arrayBuffer()])` coalesces the unsliced call onto a `max_length = 3` reader and resolves it with 3 bytes (the reverse ordering hands the sliced caller the full buffer). Before this PR that scenario failed loudly with `EEXIST`; now it silently returns wro
Check warning on line 442 in src/runtime/webcore/blob/read_file.rs
claude / Claude Code Review
Stacked Borrows: &ReadFile on JS thread aliases &mut ReadFile on work pool
Materializing `let existing = unsafe { &*(existing.cast::<ReadFile>()) }` on the JS thread forms a `&ReadFile` over the whole struct while the work-pool thread holds `&mut ReadFile` inside `run`/`do_read_loop` — that's a Stacked Borrows violation regardless of which fields are touched. The `Guarded<>` around `extra_completions` prevents the data race on the Vec but does not license the outer `&T`/`&mut T` alias (the field comment 'Guarded so the JS-thread push does not alias the work-pool `&mut
Loading