From 909644fffc87d840464f8c997cd00f7ecd035261 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 17 May 2026 13:00:42 +0000 Subject: [PATCH 1/6] fs: reject async iterator with ERR_STREAM_PREMATURE_CLOSE on destroy() When destroy() is called during for-await iteration of a fs.ReadStream, the in-flight fs.read callback could still fire after destroy() had run. Bun unconditionally called this.push(null) (EOF) in that callback, which set _readableState.ended = true. end-of-stream's onclose handler then saw isReadableFinished(stream, false) === true and completed cleanly instead of emitting ERR_STREAM_PREMATURE_CLOSE, so the async iterator resolved instead of rejecting. Match Node's behavior and short-circuit the _read callback when this.destroyed is set. Fixes #30919 --- src/js/internal/fs/streams.ts | 10 ++++++++++ test/js/node/fs/fs.test.ts | 20 ++++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/src/js/internal/fs/streams.ts b/src/js/internal/fs/streams.ts index 939ce1153ea..c619a6370db 100644 --- a/src/js/internal/fs/streams.ts +++ b/src/js/internal/fs/streams.ts @@ -309,6 +309,16 @@ readStreamPrototype._read = function (n) { const buf = Buffer.allocUnsafeSlow(n); this[kFs].read(this.fd, buf, 0, n, this.pos, (er, bytesRead, buf) => { + // If the stream was destroyed while the read was in flight, ignore the + // result. The fd has been (or is about to be) closed by _destroy, and + // pushing more data or EOF here would clobber the destroyed state — + // in particular, push(null) would mark the readable side as ended and + // suppress the ERR_STREAM_PREMATURE_CLOSE that end-of-stream/finished/ + // async iteration relies on. Matches Node.js behavior. + if (this.destroyed) { + return; + } + if (er) { require("internal/streams/destroy").errorOrDestroy(this, er); } else if (bytesRead > 0) { diff --git a/test/js/node/fs/fs.test.ts b/test/js/node/fs/fs.test.ts index ee5c0af2ea1..b716b149034 100644 --- a/test/js/node/fs/fs.test.ts +++ b/test/js/node/fs/fs.test.ts @@ -2221,6 +2221,26 @@ describe("createReadStream", () => { }, { timeout: 100 }, ); + + // https://github.com/oven-sh/bun/issues/30919 + it("async iterator rejects with ERR_STREAM_PREMATURE_CLOSE when destroy() is called during iteration", async () => { + const stream = createReadStream(join(import.meta.dir, "readFileSync.txt")); + + let chunks = 0; + let caught: any = undefined; + try { + for await (const _ of stream) { + chunks++; + stream.destroy(); + } + } catch (err) { + caught = err; + } + + expect(chunks).toBe(1); + expect(caught).toBeDefined(); + expect(caught?.code).toBe("ERR_STREAM_PREMATURE_CLOSE"); + }); }); describe("fs.WriteStream", () => { From dff912160ff0fcee53bdf2892aa9f04302a59cc1 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 17 May 2026 13:09:00 +0000 Subject: [PATCH 2/6] ci: retrigger darwin-x64-build-rust exited with -1 (agent/infra), every other lane passed including darwin-x64-build-cpp and all other *-build-rust lanes. Diff is TS-only, no Rust code touched. From 0385d1bd4b698a9f7195e6ee97467c1ba254ad0c Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 17 May 2026 13:32:40 +0000 Subject: [PATCH 3/6] fs: fix dead-branch in ReadStream._destroy that leaked fd with {start, autoClose} MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit #16754 removed the kIsPerformingIO/kIoDone symbols but mechanically substituted kReadStreamFastPath in the _destroy check, producing a once-listener for an event that is never emitted. With `{ start, autoClose: true }` (both documented fs options) that branch was taken, so _destroy never invoked cb, 'close' never fired, and the fd leaked. Drop the dead branch. The real reason Node waits on kIoDone — protecting the fd from close() while a read() is outstanding on the thread-pool — is already handled in the _read callback by the destroyed guard added above, plus Bun can cancel in-flight reads where Node (on Windows) cannot. Also adjacent-addresses review feedback on #30920. --- src/js/internal/fs/streams.ts | 17 ++++++----------- test/js/node/fs/fs.test.ts | 16 ++++++++++++++++ 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/src/js/internal/fs/streams.ts b/src/js/internal/fs/streams.ts index c619a6370db..bb9ae218e4a 100644 --- a/src/js/internal/fs/streams.ts +++ b/src/js/internal/fs/streams.ts @@ -345,17 +345,12 @@ readStreamPrototype._read = function (n) { }; readStreamPrototype._destroy = function (this: FSStream, err, cb) { - // Usually for async IO it is safe to close a file descriptor - // even when there are pending operations. However, due to platform - // differences file IO is implemented using synchronous operations - // running in a thread pool. Therefore, file descriptors are not safe - // to close while used in a pending read or write operation. Wait for - // any pending IO (kIsPerformingIO) to complete (kIoDone). - if (this[kReadStreamFastPath]) { - this.once(kReadStreamFastPath, er => close(this, err || er, cb)); - } else { - close(this, err, cb); - } + // An in-flight fs.read() completing after destroy() is handled in the + // _read callback above (see `if (this.destroyed) return`), so it is safe + // to close the fd immediately here. (Node wires up a kIsPerformingIO / + // kIoDone event pair for this on Windows where the read can't be + // cancelled; Bun doesn't need it.) + close(this, err, cb); }; readStreamPrototype.close = function (cb) { diff --git a/test/js/node/fs/fs.test.ts b/test/js/node/fs/fs.test.ts index b716b149034..fef7018740d 100644 --- a/test/js/node/fs/fs.test.ts +++ b/test/js/node/fs/fs.test.ts @@ -2241,6 +2241,22 @@ describe("createReadStream", () => { expect(caught).toBeDefined(); expect(caught?.code).toBe("ERR_STREAM_PREMATURE_CLOSE"); }); + + // Regression: _destroy used to register `once(kReadStreamFastPath, ...)` for an + // event that is never emitted when { start, autoClose } were both truthy, so + // 'close' never fired and the fd was leaked. + it("emits 'close' and releases fd with { start: 0, autoClose: true }", async () => { + const stream = createReadStream(join(import.meta.dir, "readFileSync.txt"), { start: 0, autoClose: true }); + const { promise, resolve } = Promise.withResolvers(); + + stream.on("data", () => {}); + stream.on("close", () => resolve()); + + await promise; + expect(stream.destroyed).toBe(true); + expect(stream.closed).toBe(true); + expect(stream.fd).toBeNull(); + }); }); describe("fs.WriteStream", () => { From d689597635482928a5f1bc8f34f2bbd4f0d2fd1c Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 17 May 2026 14:07:09 +0000 Subject: [PATCH 4/6] fs: fix inaccurate comment on ReadStream._destroy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 'on Windows' qualifier was wrong — libuv runs regular-file I/O on a blocking thread pool on every platform, so Node's kIsPerformingIO/kIoDone pair isn't Windows-specific. The JS-level push(null)-after-destroy race this PR closes is separate from the kernel-level close-while-reading concern, and Bun has always had the latter behavior on the common path. --- src/js/internal/fs/streams.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/js/internal/fs/streams.ts b/src/js/internal/fs/streams.ts index bb9ae218e4a..1306ca85df8 100644 --- a/src/js/internal/fs/streams.ts +++ b/src/js/internal/fs/streams.ts @@ -345,11 +345,11 @@ readStreamPrototype._read = function (n) { }; readStreamPrototype._destroy = function (this: FSStream, err, cb) { - // An in-flight fs.read() completing after destroy() is handled in the - // _read callback above (see `if (this.destroyed) return`), so it is safe - // to close the fd immediately here. (Node wires up a kIsPerformingIO / - // kIoDone event pair for this on Windows where the read can't be - // cancelled; Bun doesn't need it.) + // If a read happens to complete after destroy() has run, the _read + // callback above short-circuits on `this.destroyed`, so the stream's + // readable state can't be clobbered. Close the fd immediately, matching + // the behavior the common `createReadStream(path)` path has had since + // #16754. close(this, err, cb); }; From 69f503dad6d82a48da40e15c8673b8a4a8dcc2f0 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 18 May 2026 06:51:27 +0000 Subject: [PATCH 5/6] fs: remove dead kReadStreamFastPath references Per maintainer request. The previous commit removed the last live reader (the broken once-listener in _destroy), leaving only: - the symbol declaration and its 4-line scaffolding comment - a constructor assignment no code consumed - a ReadStream.prototype.pipe override whose body was a commented-out stub that just delegated to Readable.prototype.pipe Remove all of them. The pipe override can come back in one line if and when the planned createReadStream().pipe(res) fast path is actually implemented. --- src/js/internal/fs/streams.ts | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/src/js/internal/fs/streams.ts b/src/js/internal/fs/streams.ts index 1306ca85df8..97686235f04 100644 --- a/src/js/internal/fs/streams.ts +++ b/src/js/internal/fs/streams.ts @@ -36,11 +36,6 @@ type FD = number; const { validateInteger, validateInt32, validateFunction } = require("internal/validators"); -// Bun supports a fast path for `createReadStream("path.txt")` with `.pipe(res)`, -// where the entire stream implementation can be bypassed, effectively making it -// `new Response(Bun.file("path.txt"))`. -// This makes an idomatic Node.js pattern much faster. -const kReadStreamFastPath = Symbol("kReadStreamFastPath"); // Bun supports a fast path for `createWriteStream("path.txt")` where instead of // using `node:fs`, `Bun.file(...).writer()` is used instead. const kWriteStreamFastPath = Symbol("kWriteStreamFastPath"); @@ -206,13 +201,6 @@ function ReadStream(this: FSStream, path, options): void { } } - this[kReadStreamFastPath] = - start === 0 && - end === Infinity && - autoClose && - !customFs && - // is it an encoding which we don't need to decode? - (encoding === "buffer" || encoding === "binary" || encoding == null || encoding === "utf-8" || encoding === "utf8"); Readable.$call(this, options); return this as unknown as void; } @@ -394,13 +382,6 @@ function closeAfterSync(stream, err, cb) { stream.fd = null; } -ReadStream.prototype.pipe = function (this: FSStream, dest, pipeOpts) { - // Fast path for streaming files: - // if (this[kReadStreamFastPath]) { - // } - return Readable.prototype.pipe.$call(this, dest, pipeOpts); -}; - function WriteStream(this: FSStream, path: string | null, options?: any): void { if (!(this instanceof WriteStream)) { return new WriteStream(path, options); From c3d113fe2e4c53978ec8e2d05b9acb9322b905e5 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 18 May 2026 07:10:23 +0000 Subject: [PATCH 6/6] fs: drop unused encoding destructure in ReadStream constructor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Left over from the kReadStreamFastPath cleanup — the old fast-path assignment was the only reader. Stream encoding is still applied via Readable.$call(this, options). --- src/js/internal/fs/streams.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/js/internal/fs/streams.ts b/src/js/internal/fs/streams.ts index 97686235f04..b6c2a618107 100644 --- a/src/js/internal/fs/streams.ts +++ b/src/js/internal/fs/streams.ts @@ -136,7 +136,7 @@ function ReadStream(this: FSStream, path, options): void { // Only buffers are supported. options.decodeStrings = true; - let { fd, autoClose, fs: customFs, start, end = Infinity, encoding } = options; + let { fd, autoClose, fs: customFs, start, end = Infinity } = options; if (fd == null) { this[kFs] = customFs || fs;