Bun.stdin: reject concurrent text/bytes/arrayBuffer/json reads instead of racing on fd 0 - #35833
Open
robobun wants to merge 6 commits into
Open
Bun.stdin: reject concurrent text/bytes/arrayBuffer/json reads instead of racing on fd 0#35833robobun wants to merge 6 commits into
robobun wants to merge 6 commits into
Claude / Claude Code Review
completed
Jul 26, 2026 in 35m 25s
Code review found 1 important issue
Found 4 candidates, confirmed 3. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 1 |
| 🟡 Nit | 2 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🔴 Important | src/runtime/webcore/Blob.rs:1263-1267 |
Sequential Bun.stdin.text() rejects after process.stdin is referenced (regression) |
| 🟡 Nit | src/runtime/webcore/Blob.rs:144-152 |
fd_cached_stream: over-broad Fd(_) match + hard-coded JSBlob::stream_get_cached |
| 🟡 Nit | src/runtime/webcore/Blob.rs:122-125 |
STDIN_BLOB_READ_IN_FLIGHT is thread_local; does not guard cross-Worker stdin reads |
Annotations
Check failure on line 1267 in src/runtime/webcore/Blob.rs
claude / Claude Code Review
Sequential Bun.stdin.text() rejects after process.stdin is referenced (regression)
Merely referencing `process.stdin` (e.g. `process.stdin.isTTY` — no listener, no read) eagerly runs `Bun.stdin.stream()` and populates the cached-stream slot, so after this PR *sequential* `await Bun.stdin.text(); await Bun.stdin.text()` now rejects `ERR_INVALID_STATE` on the second call instead of resolving `""` at EOF. That directly breaks the guarantee your own "sequential Bun.stdin.text() after the first read completes does not reject" test asserts — the test only passes because it never tou
Check warning on line 152 in src/runtime/webcore/Blob.rs
claude / Claude Code Review
fd_cached_stream: over-broad Fd(_) match + hard-coded JSBlob::stream_get_cached
`fd_cached_stream` matches on `PathOrFileDescriptor::Fd(_)` — any fd-backed Blob, not just stdin — and hard-codes `JSBlob::stream_get_cached`, which `uncheckedDowncast<JSBlob>`s a receiver that can be a `JSBuildArtifact` (JSBundler.rs:1894-1912) — the exact hazard `get_stream_with_cache`'s parameterized accessor exists to avoid (see JSBundler.rs:1924-1926). Unreachable today because BuildArtifact blobs are never fd-backed, but narrowing the guard to `is_stdin_fd_store(blob)` (already defined abo
Check warning on line 125 in src/runtime/webcore/Blob.rs
claude / Claude Code Review
STDIN_BLOB_READ_IN_FLIGHT is thread_local; does not guard cross-Worker stdin reads
`STDIN_BLOB_READ_IN_FLIGHT` is `thread_local!`, so each Worker gets its own flag — but fd 0 and the `IoRequestLoop` epoll set (`static LOOP` at src/io/lib.rs:778) are process-wide. Two Workers (or main + a Worker) calling `Bun.stdin.text()` concurrently each see their own flag as `false` and still hit the EEXIST/byte-split race this PR fixes for the single-thread case. A `static AtomicBool` with `.swap(true, SeqCst)` / `.store(false, SeqCst)` closes the whole class for free (claim and release bo
Loading