Repository navigation
Conversation
…nects A regular file that is read while the client leaves, at the first read and in the middle of the body, and a pipe body (Bun.stdin). The fixtures count the reads of the file with process.resourceUsage().fsRead.
Collaborator
Author
|
Status: draft. The fix is back on the head (a40d82e), and build 131449 runs it with the reworked tests.
|
Collaborator
Author
|
Updated 2:44 PM PT - Oct 23rd, 2026
✅ @robobun, your commit a40d82e3c0ef13690d6fe887bfd780bb41dfc8d6 passed in 🧪 To try this PR locally: bunx bun-pr 46574That installs a local version of the PR into your bun-46574 --bun |
…t [skip builds] On main the clients of this test never left while a read of the response was queued or running, so the test passed there. The fixture now runs with one pool thread and keeps it busy with open and close jobs while a client takes the response.
On Windows, FileResponseStream::finish() keeps the ref of the read in flight and nothing stops the reader. After a disconnect a regular file was read to its end, and the fd stayed open for that time. A pipe body stayed open until its writer closed it, and the dead response took the bytes of the pipe. on_aborted now calls reader.close(). That cancels the uv_fs_read in flight, or waits for it, and then calls on_reader_done, which releases the ref. The stream closes the fd after that, as before.
Two of the new tests fail on purpose at their last line, and a fourth test prints the state of a pipe fd of the caller. A later commit removes this before the PR is ready.
Build 131318 has the numbers. The only other change is a comment: the read of stdin is not in the read count.
…sole body - The parked-pool fixture has a second mode. It frees the pool from the abort listener, so the read of the response is already running when the server closes the reader. - The mid-body test no longer needs a client reset to arrive at the right moment. It calls server.stop(true) from the data callback of the client, for a fetch handler and for a static route. - The stdin test runs on every platform. - A Windows test serves Bun.stdin over a console.
The tests of the previous commit have not run against the code of main yet. This commit restores FileResponseStream.rs of main so that CI shows them without the fix. The next commit puts the fix back.
Build 131437 ran the tests against FileResponseStream.rs of main. On Windows 2019 x64 and Windows 11 aarch64 the four file and stdin tests and the console test failed in every attempt.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Bun.fileresponse reads its file to the end after an abort (client disconnect, idle timeout,server.stop(true)): 256 reads for 16 MiB.Bun.stdinbody stays open, and the nextResponse(Bun.stdin)gets"".finish()(src/runtime/server/FileResponseStream.rs:581) releases the read ref only on POSIX. The Windows reader issuesuv_fs_readuntil EOF (src/io/PipeReader.rs:1563).Fix
on_abortedcallsreader.close(). It cancels the read in flight, or waits for it.on_reader_donereleases the ref, and the stream closes the fd.close()is the reader's call for a consumer that left, as inFileReader::on_cancel.bun-serve-file.test.tsandterminal-spawn.test.ts. Reads after the abort: 0 with the read queued, 1 with it running (main 256).Background
FileResponseStreamstreams an open fd into a response. On Windows, libuv reads a file with oneuv_fs_readat a time on its thread pool.Downsides
Bun.file(fd)of a pipe with fd > 2: libuv closes the caller's fd with the reader.maindoes that at the pipe's end, this change also at the abort.bun.exeis +0.0 KB.Notes
State of this draft: build 131449 runs the head. The numbers below for the fix are from builds 131318 and 131351, before the tests were reworked. I add the numbers of build 131449 when it ends.
Without the fix (build 131437:
FileResponseStream.rsofmainplus the tests of this PR, Windows 2019 x64 and Windows 11 aarch64, 4 of 4 attempts on each lane)aborted Response(Bun.file) does not run the read it has queued: 256 reads after the abort. The test expects 0.aborted Response(Bun.file) stops after the read that is running: 256 reads. The test expects 1.Response(Bun.file) stops reading its file when server.stop(true) aborts it mid-body: 239 or 240 reads after the stop in each of 4 rounds. The test expects fewer than 8.the next response can read stdin after an aborted Response(Bun.stdin):"". The test expects"world".the next response can read the console after an aborted Response(Bun.stdin)(terminal-spawn.test.ts): the second response is reset (ECONNRESET).With the fix (builds 131318 and 131351, same lanes, earlier version of the tests)
Response(Bun.stdin)after an aborted one: 5 of 5 bytes (main 0).Response(Bun.file) does not double-close the fd on Windowspasses on both lanes.Bun.file(3)of a pipe of the caller after an aborted response:fstatgivesEBADFon both lanes (a temporary test, not in this PR). Onmainthe fd stays open until the writer closes the pipe. That is from the code, I did not run it.Cost for a response that is not aborted
FileResponseStream.rsofmainand one with this PR. All 19FileResponseStreamsymbols have the same size,.textis 82,231,705 B in both, and the stripped binaries differ only in the 20 bytes of.note.gnu.build-id.bun.exe: +0.0 KB on x64 and on arm64 (CIbinary-sizestep of build 131318 against build 131216, resolution 0.1 KB). The compressed artifacts differ by 208 B (x64) and 53 B (arm64).bun.exeand.pdbartifacts are on a host that this machine cannot reach.How the tests work
process.resourceUsage().fsReadis the read operation count of the process on Windows (GetProcessIoCounters).UV_THREADPOOL_SIZE=1) and park it in a read of an empty stdin pipe. The first read of the response is then still queued when the client leaves. The running test frees the pool from the abort listener, which runs before the server closes the reader, and waits there until the read count moves.server.stop(true)from the data callback of a client that has 1 MB of the body. The server is not in a callback of its own at that moment, so a read is in flight. It runs 4 rounds, for a fetch handler and a static route.When the bug shows on
main: the server must handle the abort while auv_fs_readis queued or running. If the read completes first and the write then fails, the stream pauses the reader and releases the ref, and the abort frees the stream at once. With client resets the bad state came up in 1 of 8 attempts on an idle pool (build 131200, 24 resets each) and in 4 of 10 runs on a busy pool (build 131216). That is why the tests do not use a reset race.Cost of one hit on
main: for a regular file, (remaining bytes / 64 KiB) reads on the pool with the fd open. For a pipe or a console there is no bound: the reader stays armed, keeps the event loop alive and takes later input.History
uv_fs_read, and the Windows lanes crashed (build 89813: heap corruption inget_read_buffer_with_stable_memory_address). Hardening: fix crashes across the runtime, bundler, dev server, shell and CLI #46021 therefore took only the POSIX line.maingot the POSIX half.on_reader_donereleases the ref after the read in flight.Limits
req.signalruns before the reader closes. A handler that answers the next request with the same fd from inside that listener still finds the old reader. POSIX has the same order.uv_read_stopon a console.Bun.file(fd)of a pipe with fd > 2: libuv closes the fd when the reader closes (src/io/PipeReader.rs:1773).mainhas this for every completed response. A fix duplicates the handle inSource::open, as libuv does for fd 0 to 2. No issue tracks it.mainhas this too.Not in this PR
start()still reads the fd, for exampleBun.file("/dev/zero")on Linux or a second response over one FIFO fd. The review reproduced it on Linux with and without this PR. It was found with Bun.serve: release the FIFO file reader when the client disconnects #41874 and is separate work. No issue tracks it.close()reports done at once, so with this statement the stream would close the fd while a read on the pool can still run. There the reader has to own the fd (close_fd), or the stream has to stay alive until the read is collected.main. The Windows half is this PR.Also considered
on_read_chunkwhen the response is done. The queued read still runs, and a pipe body still loses the next chunk of the writer.close()infinish(). It has the same effect, and it adds a call to every response.falseof the parent in the shared Windows reader. That changes a contract that 14 kinds of parent use.Linux:
bun bd test test/js/bun/http/bun-serve-file.test.tspasses (177 pass, 4 skip) andbun bd test test/js/bun/terminal/terminal-spawn.test.tspasses (16 pass, 3 skip).bun run rust:check-allpasses forx86_64-pc-windows-msvcandaarch64-pc-windows-msvc.no test proof · iteration 1 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/bun/terminal/terminal-spawn.test.ts, test/js/bun/http/bun-serve-file.test.ts