Bun.serve: pause the file reader while the response is backpressured

Merged
RO
2mo ago
+118-107

Problem

  • Bun.serve keeps reading a pollable Bun.file() body (a FIFO, a character device, a socket) while the client reads nothing. The bytes collect in the response's write buffer, which has no limit.
  • A disconnect in this state frees the stream with its poll still armed. With Bun.file(fd), the next readable event reads freed memory: AddressSanitizer: heap-use-after-free in PosixBufferedReader::begin_read.
  • Cause: on backpressure, on_read_chunk (src/runtime/server/FileResponseStream.rs:327) pauses the reader only on Windows. On POSIX the read loop re-arms the poll after EAGAIN (src/io/PipeReader.rs:846).

Fix

  • Pause the reader on backpressure on every platform. on_writable unpauses it before the next read. FileReader uses the same handshake (src/runtime/webcore/FileReader.rs:669).
  • A paused POSIX reader unregisters its poll and ignores read(). #23341 limited the pause to Windows because on_writable did not unpause. On Windows read() already unpauses.
  • Verified: test/js/bun/http/bun-serve-file.test.ts. The server must stop reading a FIFO, for Bun.file(path) and Bun.file(fd), and read again when the client does. All three cases fail on main.
  • Self-reviewed: 3 concerns raised, 3 addressed. The on_read_chunk doc is fixed, the #23341 history is above, and the Notes name the open FileReader case and the order with #41204.

Background

  • FileResponseStream streams a file descriptor into a uWS response. Without sendfile, a BufferedReader reads the fd and calls on_read_chunk for each chunk.
  • resp.write() returns Backpressure when the socket is full. uWS buffers the rest and calls on_writable when the socket drains.
  • The event loop reports a pollable fd readable once per arm. The read loop re-arms the poll after a read that ends in EAGAIN.
Notes

Measurements (Linux x64)

  • Main, release build: the server read 64 MiB from the FIFO while the client read 0 bytes. The read stopped only at the limit of the probe.
  • With the fix, the server stops after about 2.6 MiB. That is the kernel socket buffers plus one chunk.
  • On main the test prints still reading after 33554432 bytes, on the release build and on the debug build. With the fix each case takes about 1.4 s on the debug build, and about 2.5 s with all cores busy.

The crash

  • ASAN stacks on main, Bun.file(fd) case: the read is in PosixBufferedReader::begin_read, from FilePoll::on_update. The free is in the drop of FileResponseStream, from on_aborted (HttpContext::onClose). The allocation is in FileResponseStream::start, from RequestContext::do_sendfile.
  • A release build can segfault on the same path. A probe with more requests between the disconnect and the next write crashed in 3 of 3 runs: panic(main thread): Segmentation fault at address 0x98.
  • The reader's Drop does not unregister the poll, because start() clears CLOSE_HANDLE. #37083 changes that teardown. This PR only makes sure that the poll is not armed when a backpressured stream is freed.

The test

  • The old fixture printed stalled also when the drain never stalled. Its stall loop ended at a time bound, so on main it waited 5 s and passed.
  • The new fixture has no sleeps and no timeouts. It opens the FIFO with O_NONBLOCK. A write to a full pipe fails with EAGAIN, so the byte count grows only when the server reads.
  • Each step fills the FIFO and makes one request to the same server. The server shares the fixture's event loop and must poll for I/O to answer. A reader with an armed poll reads the full FIFO during that poll. On main every request shows a read. With the fix, the first request without a read comes after 2621440 bytes (40 reads of 64 KiB), in every run.
  • The limit is 32 MiB. That is far above the kernel socket buffers on loopback.
  • The resume case covers the unpause() in on_writable. With the fix, the server reads again after 5 requests. A build without the unpause fails that case with no read for resumed, and passes the other two.
  • The resume case stops reading again before the disconnect. A stream that waits for pipe data when its client disconnects stays allocated while the pipe is open (#37083). The leak check on the ASAN lane reports that stream at exit.

Regular files

  • A regular file body takes the same pause and unpause when it cannot use sendfile (TLS, HTTP/3, macOS). A scratch test served a 48 MiB file to slow readers over http and https, through a fetch handler and a static route. All bodies matched by SHA-256, and on_writable ran 167 times.

Keep-alive

  • pause() unregisters the poll, and that also releases the poll's hold on the event loop. unpause() does not take it back. The sink path of FileReader behaves the same way.
  • While the response is in flight, the server's pending-request count keeps the process alive. A difference shows only after server.unref().

Not in this PR

  • FileReader still pauses only on Windows when it stops at its high-water mark (src/runtime/webcore/FileReader.rs:647-655). So Bun.file(fifo).stream() with a stalled consumer can still buffer without a limit on POSIX. That fix needs an unpause before read_into, and its own test.
  • #41204 adds test/flaky-tests.txt with a line for test/js/bun/http/bun-serve-file.test.ts, because of the CI timeout that this bug causes. #41204 is not merged. The PR that lands second removes that line.

Other checks

  • bun bd test on bun-serve-file, bun-serve-static, serve-directory-routes, serve-file-slice-read-error, serve-http3, and serve.test.ts. Two tests in serve.test.ts fail the same way on the release build (a privileged port, and a loopback-only check). This container runs as root.
  • cargo check for x86_64-pc-windows-msvc passes.

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/http/bun-serve-file.test.ts