PipeReader: label a chunk Eof only when the reader is at its end

Closed
RO
19d ago
+556-10

Problem

  • Bun.write(file, stream) and HTMLRewriter resolve with 262144 of 270000 bytes that are unread at the hangup. A Bun.serve file response sends 262144 with 200.
  • Cause: under a hangup, read_state (src/io/PipeReader.rs:903) labels every delivery Eof, also one that filled the buffer.
  • Found by inspection (#43900). No user report.

Fix

  • read_state returns Eof only for a 0-byte read, a used-up limit or byte budget.
  • Correct: the reader is closed before every Eof, as src/io/pipes.rs:148 documents.
  • Stacked on #37082: a file response that ends at on_reader_done needs its finish().
  • Verified: 26 new rows in spawn.test.ts, bun-serve-file.test.ts, streams.test.js. 18 fail without the fix. Self-reviewed: 16 concerns raised, 12 addressed, 4 in part.

Background

  • PosixBufferedReader reads pipes and sockets for streams, subprocess stdio and file responses. It labels each delivery Progress, Drained or Eof.
  • A hangup (EPOLLHUP, EV_EOF) says only that the writer is gone. Native code feeds a native sink, like Bun.write.
  • Considered a guard in each consumer: the wrong label stays. A read before the delivery cuts SOCK_SEQPACKET messages.

Downsides

  • Under a hangup, native sinks make 2 or 3 recv() for 1. A Bun.serve body of 131072 to 262144 bytes goes out chunked, 15 send() for 9. No hangup: no change.
  • A read error behind hung-up bytes: native sinks reject where they resolved, Bun.serve resets where it sent a complete 200.
  • A use-after-free of main stays, for a file response whose source hung up first: ASAN reports 64 of 90 runs on main and on this PR. #41874 is open for it.
Notes

Preconditions. POSIX only. A native sink is attached: Bun.write of a stream or a Response, HTMLRewriter.transform, Bun.spawn stdin, a fetch body, a Bun.serve file response. JS readers (getReader(), for await, .text()) end on on_reader_done and are not changed.

Where the delivery is labelledBytes unread at the hangupTiming
Shared 256 KiB buffer (PipeReader.rs:784)more than 262144None for Bun.file(fd).stream() and Bun.stdin.stream(): the reader starts on its poll, so the first wakeup has the bytes and the hangup. For proc.stdout and a started file response the bytes and the hangup must come in one wakeup.
Reader's own buffer (:805), used while another read loop holds the shared one131073 or moreThe wakeup is handled inside the callback of another reader.

Reach. A child on a default Linux socket cannot leave more than 262144 bytes unread: the socket holds about 219000. The first row needs SO_SNDBUF raised, net.core.wmem_default of 262144 or more, or an fd that the user passes in. The second row happens with default sockets. Bun gives the stdio sockets of a child 512 KiB on macOS (src/spawn_sys/spawn_process.rs:867), so a plain child can reach the first row there. The defect was not measured on macOS: no macOS host was available.

Affected versions (a socket that hung up with 270000 bytes before the sink attached, linux x64):

VersionBun.write(file, stream)HTMLRewriternew Response(stream).arrayBuffer()
1.1.0, 1.2.0writes [object ReadableStream]270000270000
1.3.0, 1.3.14the probe crashes
1.4.0writes [object ReadableStream]262144270000
1.4.1, 1.4.2, main262144262144270000
this PR270000270000270000

Node and libuv. Node v26.3.0 (libuv 1.52.1) with the same child: 262145, 270000 and 400000 bytes unread at the hangup all arrive, then end. libuv 1.52.0 takes EOF from a hangup only when the event has no POLLIN (uv__stream_io, src/unix/stream.c). libuv 1.51.0 took it only after a partial read.

Tests. Every row compares bytes, not lengths. The payload repeats a unit of 251 bytes. No row sleeps. The sockets, their constants and the payload come from one module, test/socketpair.ts. It does not import the harness, because the child fixtures use it and a debug build takes seconds to load the harness. The rows skip on a host whose limit for a socket buffer cannot hold their bytes. Linux cuts a request above net.core.wmem_max. macOS cuts it too, and refuses it with ENOBUFS before 14.4 (kern.ipc.maxsockbuf). So the helper does not check the result of setsockopt(). It asks a probe socket how many bytes it takes (limitIsBelow()), and that probe never throws, because a test file calls it while it loads.

FileRowsFail without the fixGuards
streams.test.js11, in-process socketpair9: hung up before the sink attached (262145, 270000, three sinks), second buffer (131073, 200000), reset behind 160000 bytes (three sinks)262144 and 131072, the most that one delivery takes
bun-serve-file.test.ts9, in-process socketpair6: 270000 on a started and on an empty response, 400000 over a unix listener, second buffer (131073, 200000), reset200000 on both response shapes, 131072
spawn.test.ts6, child process3: Bun.write of the stream, of a Response, HTMLRewriterspawn stdin, fetch body, shell capture

The child rows block the parent in readFileSync(fifo) until the child has closed its stdout. The child opens the FIFO without blocking, again and again until the parent reads it, and it ends when the parent is gone. With a blocking open, a row that timed out left its child behind. The child ends by SIGKILL after its report. It opens libc with bun:ffi, and with leak checks on (the ASAN lane) the exit of a process takes seconds after a dlopen: the leak check symbolizes the leaks that test/leaksan.supp names. The parent waits for that exit. The child closes fd 1 before it reports, because the exit of a process does not close its descriptors in a useful order. The reset rows are Linux only: a unix socket whose peer closes with unread input fails the next read with ECONNRESET. There is no row for a static file route. A static route refuses an fd, and a FIFO cannot hold 262144 bytes without F_SETPIPE_SZ, which fails in a container without CAP_SYS_RESOURCE.

Measured (release builds of the base and of this PR, linux x64, LD_PRELOAD loggers and a ptrace tracer. strace, perf and valgrind are not installed):

  • recv() per stream, sink path under a hangup, N = 1000 / 200000 / 262144 / 270000 / 400000: base 2 / 1 / 1 / 1 / 1, PR 2 / 2 / 2 / 3 / 3. The pull path makes 2 / 2 / 2 / 3 / 3 on both.
  • recv() per stream, no hangup, N = 1000 / 200000 / 600000: base 5 / 5 / 7, PR 5 / 5 / 7 (10 of 12 runs each at 600000, the others took 9 to 12 on both builds).
  • Hung-up blocking pipe into a sink (Bun.write(out, Bun.stdin.stream())): base 1 poll and 1 read, PR 2 and 2. The pull path makes 2 and 2 on both.
  • Bun.serve file response, source hung up with 200000 bytes before the first read: base Content-Length, 9 send(), 200120 wire bytes. PR chunked, 15 send(), 200138 wire bytes. Bodies under 131072 bytes: 9 send() on both. Each header part is its own send() because the file response writes from reader callbacks without a cork. That is so on main too.
  • read_loop: base 2607 bytes, 588 instructions, 68 conditional branches. PR 2614, 591, 68. Release text size: 80856331 bytes on both.
  • Read error behind 160000 hung-up bytes: Bun.write resolves 160000 on the base, and rejects with ECONNRESET after it wrote 160000 bytes on the PR. Bun.file(fd).text() rejects on both.
  • Allocations inside FileReader::on_read_chunk, pull path, first hung-up delivery of 262144 bytes: 109 on both, the same sizes in the same order.

Read errors, by consumer.

  • Native sinks: the promise rejects. Before, it resolved when 131072 bytes or more were in front of the error.
  • Bun.serve file response: the server answers a failed body with a reset. That is the rule on main for an error behind fewer than 131072 bytes. It now applies behind more bytes too. Measured with 160000 bytes: the base sends a complete message with the whole body. The PR sends a part of the body with no last chunk, then the reset (ECONNRESET at the client). The close drops what the kernel had not sent yet. The server calls no error() handler and logs nothing: on_file_stream_error ignores its error argument (src/runtime/server/RequestContext.rs:1657).
  • JS readers: the same result on both builds. A reset behind fewer than 131072 bytes is dropped on both. This PR does not change that.

Self-review. 16 concerns, all about the tests and this text. None asked for a change to the source.

  • Changed: the child closes its stdout before it reports (the rows got their order from a race). Rows compare bytes. New rows for a source that hung up before the sink attached, the second buffer, a sink that pushes back (unix listener, suspending HTMLRewriter handler), a response that starts empty, and the read error for HTMLRewriter and Bun.serve. The text names the preconditions, the reach and the versions.
  • In part, 1: macOS. The rows pass there (x64 and aarch64), which runs the kqueue arm. The defect without the fix was not measured there, because no macOS host was available.
  • In part, 2: the second buffer has rows for Bun.file(fd).stream() and for the file response, not for proc.stdout.
  • In part, 3: the unix listener row shows that every byte arrives while the response pushes back. It does not read the result of the write.
  • In part, 4: the dropped errno of a failed file body is named below. No issue is filed for it.

Review of the first push. Three optional findings.

  • The socket helper and its constants were in two test files and in a fixture. They are in test/socketpair.ts now.
  • The rows failed on queued on a host with a low net.core.wmem_max. They skip there now.
  • The framing change of Bun.serve (Downsides, bullet 1). The proposal was one more read before the delivery when the hangup is known. Measured on a release build with that change: it keeps Content-Length and 9 send() for hung-up bodies up to about 245760 bytes, with the same recv() count as this PR. It reads into the rest of a partly filled buffer, and that cuts a message on a message-oriented socket:
SOCK_SEQPACKET, hung up, into Bun.writemainthis PRread before the delivery
3 messages of 100000 bytes200000300000262144
2 messages of 140000 bytes140000280000262144
4 messages of 70000 bytes140000280000262144
20 messages of 15000 bytes135000300000300000

So this PR keeps the label change alone. The 6 more send() come from writes that the file response makes without a cork, one per header part. A cork there helps every pollable body, on main too. It is not in this PR.

Review of the second push. One optional finding. On macOS before 14.4 with a lowered kern.ipc.maxsockbuf, setsockopt() fails with ENOBUFS. The helper threw then, and three test files failed while they loaded. Changed in ef4611eda5, see Tests. Checked on Linux with a library in place of libc whose setsockopt() always fails: the old helper fails streams.test.js while it loads, the new one skips its 11 rows.

Review of the third push. No bug found. One nit: a nested-reader row of streams.test.js that fails left its outer reader to the tests after it. The row cancels that reader in a finally block now (0ce4644537). The same commit has the change to the child fixture, see Tests. Checked with the rows held to 2 CPUs: 5 of 6 rows timed out, and no fixture process was left. Before the change, rows that timed out left their children behind: 6 of them were on this host.

CI of the third push (build 120963). spawn.test.ts was red on the x64-asan lane, 4 of 4 attempts. Two causes. One was a row of this PR, see Suites. The other is not from this PR: "an idle reader stopped at the highwater mark does not keep the process alive (trickling writer)" fails there because the stderr of its child has a LeakSanitizer warning (ptrace appears to be blocked). The annotations of 37 other builds since 2026-09-25, on branches without this change, name the same test. It was red again in build 120970 (b6ee5db1ac), with the "saturating writer" row of the same test beside it, and it is the only red test there. Of 54 builds that finished after 2026-09-26T06:00Z, 12 have it red and 27 have it green after a retry.

Left for a follow-up. read_loop ignores keep_going == false under a hangup (&& !received_hup, PipeReader.rs:858). That exception made up for the old label: a parent that answered false to Eof left bytes in the kernel. It stays in this PR. Without it, pause() is honoured under a hangup, which changes more than this bug.

Found outside this change.

  • A heap-use-after-free on main (Downsides, bullet 3). The source of a file response hung up before the response starts. The stream then ends inside FileResponseStream::start and is freed at its end (src/runtime/server/FileResponseStream.rs:280). The poll of its reader stays registered, because the stream owns the fd and the reader returns its poll only when it owns the fd. The poll fires later, and begin_read reads the freed reader (src/io/PipeReader.rs:655). One fresh process per run, 30 runs per cell, debug builds with ASAN:

    SourceReports on mainReports on this PR
    socket, 1000 bytes2926
    socket, 270000 bytes1417
    FIFO, 1000 bytes2121

    One more run of the PR build made no report and did not end in 60 s. Release builds did not crash in this probe. In some runs the process spins in the event loop and the client has no answer after 8 s: 10 of 100 runs on main (367d939d9), 8 of 100 on the base, 15 of 100 on this PR (two socket cells, the builds interleaved). These counts are too small to tell the builds apart. #41874 makes the reader return its poll when it goes away, and finish() unregisters it. That is from its diff. It was not built for this PR.

  • Bun.serve drops the errno of a failed file body, see above.

  • The comment at src/uws_sys/libuwsockets.cpp:1182 names FileResponseStream::finish as a caller of end_without_body. #37082 removes that call.

Suites on the debug build of this PR. At 814a129532: streams.test.js 635 pass, and in bun-serve-file.test.ts and spawn.test.ts every new row passes. At earlier commits of this branch with the same source change: spawn.test.ts, bun-serve-file.test.ts, bunshell.test.ts, html-rewriter.test.js, bun-serve-static.test.ts, spawn-stdin-readable-stream.test.ts, spawn-stdio-syscall-error.test.ts, shell-blocking-pipe.test.ts, process-stdin-stale-hup.test.ts, filesink.test.ts, body-clone.test.ts, terminal.test.ts, filter-workspace.test.ts pass. bun run rust:check-all: 12 ok. On the last two commits this host was overloaded (load average above 300). Tests that start child processes hit the local default of 5 s in some runs, on a debug build of main too. So the new rows ran with --timeout 60000 on the last commits (0ce4644537 for streams.test.js and spawn.test.ts, ef4611eda5 for bun-serve-file.test.ts): 26 pass on a release and a debug build of this PR, 18 fail on a release build of the base and on a debug build of main. CI sets --timeout=90000, and 270000 with ASAN (scripts/runner.node.ts:2166). But spawn.test.ts runs itself once more with the default limit of 5 s ("with BUN_FEATURE_FLAG_FORCE_WAITER_THREAD", Linux only). In that run on the x64-asan lane the six child rows took 4.0 to 4.9 s each in build 120928 and 4.1 to 5.0 s in build 120963, where one row hit the limit. On two release lanes they took 37 to 82 ms. The cause was the exit of the child, see Tests. b6ee5db1ac changes it. In build 120970, with that change, the six rows took 273 to 368 ms each in that run (4 attempts). With the environment of that run on a debug build here, a row took 6.9 to 9.7 s before and 2.2 to 4.3 s after (4 runs each, interleaved, this host overloaded).


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/web/streams/streams.test.js, test/js/bun/spawn/spawn.test.ts, test/js/bun/http/bun-serve-file.test.ts