Release the poll and surface the error when a Bun.file() stream read fails

Merged
RO
2mo ago
+195-48

Problem

  • A Bun.file().stream() whose read(2) fails with no pull pending never settles. for await (const c of Bun.file("/proc/self/mem").stream()) hangs forever, while Bun.file(path).text() rejects with EIO (#33362).
  • After a read error on a pollable fd (a pty master, a non-blocking pipe) the process never exits. reader.read() rejects with EIO: i/o error, read, but the FilePoll stays registered and the dup'd fd open. An errored stream is never cancelled, so nothing releases them.
  • Both come from FileReader::on_reader_error (src/runtime/webcore/FileReader.rs). It only runs pending, a no-op unless a pull is pending (Pending::run), and it never closes the reader.

Fix

  • on_reader_error settles the pending pull when there is one. Otherwise it stores the error in a new read_error slot, and the next on_pull returns it after any buffered bytes (reader_finished, end_of_reader).
  • on_reader_error then closes the reader (close_after_error): it marks the source done and releases the poll and the fd through deinit, which reports no second completion.
  • Verified: test/js/web/streams/streams.test.js (6 new tests, 5 fail on 1.4.1). Also the rest of test/js/web/streams/, the fs, stream and spawn suites.

Background

  • FileReader is the native source behind Bun.file().stream(). A pull first tries a synchronous read (read_into). With no data on a pollable fd, the BufferedReader registers a poll and the pull becomes pending. A registered poll keeps the event loop alive.
  • The reader reports through on_read_chunk, on_reader_done or on_reader_error. On EOF it closes itself before it reports. On error it did not.
  • sink is the native consumer path (a fetch body). It gets the error through sink.end and needs the same reader close.
Notes

This supersedes #33362, which stores the error in a read_error slot in the same way but does not release the poll and the fd. Its tests are carried over unchanged.

A stream over a non-blocking pty master with a pending read: before this change the rejection lands and the process then hangs, with or without reader.cancel() or releaseLock() afterwards. cancel() on an errored stream rejects with the stored error and never reaches the native source.

The sync pull path reaches on_reader_error from inside on_pull. The reader is closed by the time read_into returns, so on_pull returns the stored error through end_of_reader in the same call, where it previously returned Pending with no poll registered.

The second PR, for tty.ReadStream on a non-blocking fd (#41414), is built on this branch and needs the poll release: node-pty's pty master reports the child's exit as EIO.

Pre-existing failures on a debug build, with or without this change: test/js/node/process/stdin/stdin-fixtures.test.ts (the runner kills the child after 1 s, a debug child needs 1.1 s) and test/js/web/streams/streams-leak.test.ts "Absolute memory usage" (times out at 5 s).

no test proof · iteration 5 · 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