Skip to content

Bun.serve: terminate FIFO/pipe file responses at EOF - #37082

Open
robobun wants to merge 5 commits into
mainfrom
farm/0d5eeba7/fifo-response-eof-framing
Open

Bun.serve: terminate FIFO/pipe file responses at EOF#37082
robobun wants to merge 5 commits into
mainfrom
farm/0d5eeba7/fifo-response-eof-framing

Bun.serve: terminate FIFO/pipe file responses at EOF

e14bdcc
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed Aug 7, 2026 in 36m 56s

Code review found 1 important issue

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

Details

Severity Count
🔴 Important 1
🟡 Nit 1
🟣 Pre-existing 0
Severity File:Line Issue
🔴 Important test/js/bun/http/bun-serve-file.test.ts:1249-1255 FIFO test races threadpool open(O_WRONLY) against server's O_RDONLY|O_NONBLOCK open
🟡 Nit test/js/bun/http/bun-serve-file.test.ts:1256-1260 payloadSeen not wired to close/error/deadline; writer fd not guarded

Annotations

Check failure on line 1255 in test/js/bun/http/bun-serve-file.test.ts

See this annotation in the file changed.

@claude claude / Claude Code Review

FIFO test races threadpool open(O_WRONLY) against server's O_RDONLY|O_NONBLOCK open

The comment says `fsOpen(fifoPath, "w")` "blocks (in the fs thread pool) until the fetch handler above opens the read end", but nothing synchronizes the threadpool worker actually entering `open(O_WRONLY)` against the JS thread's synchronous `open(O_RDONLY|O_NONBLOCK)` + read in `RequestContext::do_sendfile`. If the server wins (CI/ASAN scheduler jitter, or the sibling `test.concurrent` variant contending the fs threadpool), it opens/reads-0/EOFs/closes the FIFO in one call stack, and the thread

Check warning on line 1260 in test/js/bun/http/bun-serve-file.test.ts

See this annotation in the file changed.

@claude claude / Claude Code Review

payloadSeen not wired to close/error/deadline; writer fd not guarded

`payloadSeen` is resolved only in the socket `data` handler; the `close`/`error` handlers and the 3s `deadline` timer resolve only `wireDone`. If a future regression closes/aborts the connection before the payload reaches the wire, `await payloadSeen` hangs until the per-test timeout with no diagnostic instead of failing fast on the captured wire, and the un-`try/finally`'d `writer` fd leaks. Add `resolvePayloadSeen()` to the close/error/deadline paths and wrap `writer` in `try { ... } finally {