Bun.file(buffer) / async fs: hold the path buffer's backing store instead of rooting the JS object - #38509
Open
dylan-conway wants to merge 14 commits into
Open
Bun.file(buffer) / async fs: hold the path buffer's backing store instead of rooting the JS object#38509dylan-conway wants to merge 14 commits into
dylan-conway wants to merge 14 commits into
Claude / Claude Code Review
completed
Aug 16, 2026 in 25m 59s
Code review found 1 important issue
Found 1 candidates, confirmed 1. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 1 |
| 🟡 Nit | 0 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🔴 Important | src/jsc/array_buffer.rs:1109-1118 |
assert_js_thread() passes on a foreign VM's JS thread; PinnedBuffer clone/drop races cross-VM via ObjectURLRegistry |
Annotations
Check failure on line 1118 in src/jsc/array_buffer.rs
claude / Claude Code Review
assert_js_thread() passes on a foreign VM's JS thread; PinnedBuffer clone/drop races cross-VM via ObjectURLRegistry
`assert_js_thread()` only checks "am I on *a* JS thread", so it passes on a Worker's thread too — but a `Store` holding `PathLike::PinnedBuffer` is shared cross-VM via the process-global `ObjectURLRegistry`. `URL.createObjectURL(Bun.file(Buffer.from(p)))` on the main VM, then `revokeObjectURL(url)` (or `resolveObjectURL(url).text()`) on a Worker, runs `PinnedArrayBuffer::drop`/`clone` → non-atomic `buf->deref()`/`ref()` on VM-A's `WTF::RefCounted<ArrayBuffer>` from VM-B's thread, racing VM-A's G
Loading