fs: reject async iterator with ERR_STREAM_PREMATURE_CLOSE when destroy() is called during iteration

[robobun]

Fixes #30919.

Repro

const { createReadStream } = require('fs');
(async () => {
  const stream = createReadStream(__filename);
  try {
    for await (const chunk of stream) {
      console.log('got chunk');
      stream.destroy();
    }
    console.log('completed');
  } catch (e) {
    console.log('error:', e.code || e);
  }
})();

Node: prints got chunk then error: ERR_STREAM_PREMATURE_CLOSE.
Bun (before): prints got chunk then completed — the async iterator resolves instead of rejecting.

Cause

for await is pull-based: yielding a chunk returns control to user code while the next _read() is still in flight on the fs thread pool. When stream.destroy() runs synchronously in the iterator body, the pending read callback still fires. Bun's ReadStream._read callback unconditionally called this.push(null) on EOF (bytesRead === 0), even if the stream was already destroyed.

push(null) flips _readableState.ended to true. Then end-of-stream's onclose reads

if (readable && !readableFinished && isReadableNodeStream(stream, true)) {
  if (!isReadableFinished(stream, false)) return callback.$call(stream, $ERR_STREAM_PREMATURE_CLOSE());
}

where isReadableFinished(stream, false) returns !!(rState.endEmitted || (strict === false && rState.ended === true && rState.length === 0)) — so with ended=true Bun skipped the premature-close error.

Node guards the same fs read callback with if (this.destroyed) return;. This PR adds the same guard.

Fix

src/js/internal/fs/streams.ts — in ReadStream._read's fs callback, short-circuit when this.destroyed:

this[kFs].read(this.fd, buf, 0, n, this.pos, (er, bytesRead, buf) => {
  if (this.destroyed) {
    return;
  }
  // ... existing push/errorOrDestroy path
});

Test

test/js/node/fs/fs.test.ts — adds a regression test in the createReadStream describe block that matches the issue repro. Fails on unpatched Bun (iterator resolves with no error) and passes after the fix.

Verification

$ USE_SYSTEM_BUN=1 bun test fs.test.ts -t 'async iterator rejects'
(fail) createReadStream > async iterator rejects with ERR_STREAM_PREMATURE_CLOSE ...

$ bun bd test fs.test.ts -t 'async iterator rejects'
(pass) createReadStream > async iterator rejects with ERR_STREAM_PREMATURE_CLOSE ...

All other createReadStream, fs.ReadStream, and Node's test-fs-read-stream* / test-stream-readable-* parallel tests still pass locally.

[robobun]

^{Updated 2:15 AM PT - May 18th, 2026}

✅ @robobun, your commit c3d113fe2e4c53978ec8e2d05b9acb9322b905e5 passed in Build #55722! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 30920

That installs a local version of the PR into your bun-30920 executable, so you can run:

bun-30920 --bun

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 471bcd84-8bb4-44c0-85bd-7935428dd38e

📥 Commits

Reviewing files that changed from the base of the PR and between 69f503d and c3d113f.

📒 Files selected for processing (1)

• src/js/internal/fs/streams.ts

---

Walkthrough

ReadStream._read now ignores fs.read results if the stream was destroyed mid-read; _destroy closes the file descriptor immediately. Tests added verify async-iterator rejection on destroy and autoClose/close semantics.

Changes

ReadStream Async Iteration Destruction

Layer / File(s)	Summary
ReadStream destruction guard in read callback src/js/internal/fs/streams.ts	_read callback checks this.destroyed immediately after fs.read completes and returns early to avoid pushing buffered data or EOF when the stream is already destroyed.
Immediate fd close in _destroy & fast-path/pipe removals src/js/internal/fs/streams.ts	Removes the kReadStreamFastPath symbol and constructor fast-path logic; ReadStream.prototype._destroy now calls close(this, err, cb) immediately; deletes module's custom ReadStream.prototype.pipe.
Async iterator & autoClose regression tests test/js/node/fs/fs.test.ts	Adds test asserting for await...of rejects with ERR_STREAM_PREMATURE_CLOSE when stream.destroy() is called mid-iteration (one chunk observed), and a test that createReadStream(..., { start: 0, autoClose: true }) emits 'close' and clears fd.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: preventing async iterator from resolving when destroy() is called during iteration by making it reject with ERR_STREAM_PREMATURE_CLOSE.
Description check	✅ Passed	The PR description comprehensively covers the repro, cause, fix, test, and verification steps against the required template sections.
Linked Issues check	✅ Passed	The code changes directly address issue #30919: adding a this.destroyed guard in ReadStream._read's fs.read callback to prevent push(null) after destroy, making the async iterator reject with ERR_STREAM_PREMATURE_CLOSE as expected.
Out of Scope Changes check	✅ Passed	All changes are scoped to fixing #30919: the ReadStream._read guard, removal of unrelated kReadStreamFastPath mechanisms, and regression tests are all directly related to the async iterator premature-close fix.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[robobun]

CI update: two back-to-back darwin build-stage flakes on unrelated lanes.

• #55496 `darwin-x64-build-rust` → exit status -1 (agent killed)
• #55497 `darwin-aarch64-build-cpp` → exit status 1

Diff is TypeScript-only (src/js/internal/fs/streams.ts + a regression test); no C++ or Rust touched. Both failing lanes passed on the other build in the pair (darwin-x64-build-cpp green in #55496, darwin-x64-build-rust was the only fail there). Every non-darwin build+test lane passed.

Spent my one re-roll budget. Handing off for a maintainer re-roll / merge.

[robobun]

CI update on 0385d1b (the adjacent fd-leak fix):

Failures are on unrelated tests that dont touch fs/streams:

• test/js/bun/util/v8-heap-snapshot.test.ts — SIGKILL on ubuntu x64 shard 11 (host-level kill, tests Bun.generateHeapSnapshot, unrelated to fs.ReadStream).
• test/js/node/worker_threads/worker_threads.test.ts — already classified by buildkite as style: WARNING, context: flaky with "(1 retry)". Tests worker memory leak detection; unrelated to this diff.

Every other lane passed (60 steps passed, 0 step-level failures). Diff is TypeScript-only: _read callback guards this.destroyed, _destroy drops the dead once(kReadStreamFastPath, ...) branch.

Already spent my one re-roll budget on earlier darwin flakes. Needs a maintainer re-roll / merge.

[claude]

LGTM — the fd-leak fix in 0385d1b addresses my earlier comment, and the remaining inline note is just a comment-wording nit.

Extended reasoning...

Overview

This PR touches src/js/internal/fs/streams.ts (two small changes to ReadStream.prototype._read and _destroy) and adds two regression tests in test/js/node/fs/fs.test.ts. The _read change adds an if (this.destroyed) return guard in the fs.read callback — a direct port of Node's behavior — so a late-arriving read result can't push(null) and suppress ERR_STREAM_PREMATURE_CLOSE for async iterators. The _destroy change (added after my earlier review comment) drops a dead once(kReadStreamFastPath, ...) branch that listened for an event nothing emits, fixing an fd leak / missing 'close' for { start: 0, autoClose: true }.

Security risks

None. This is Node-compat stream-state bookkeeping in pure TypeScript; no auth, crypto, parsing, or untrusted-input surface is touched. The fd-lifecycle change makes the { start: 0, autoClose: true } path behave identically to the default path (immediate close()), which is strictly safer than the previous behavior of leaking the fd forever.

Level of scrutiny

Moderate — fd lifecycle in fs streams is worth a careful read, which I gave it. Key observation: _destroy already called close(this, err, cb) immediately on the common createReadStream(path) path before this PR (because kReadStreamFastPath requires an explicit start: 0 to be truthy). So the simplified _destroy is not a behavioral change for the default case; it just unifies the rare opted-in path with the existing default and removes a listener that could never fire. The "close fd while a thread-pool read may be in flight" concern is therefore pre-existing (since #16754) and orthogonal to this PR.

Other factors

• My prior inline comment (pre-existing fd leak via the never-emitted kReadStreamFastPath listener) was addressed in 0385d1b with both a fix and a regression test.
• Both behavioral changes have targeted regression tests; the PR description shows fail-before/pass-after locally and that the broader test-fs-read-stream* / test-stream-readable-* Node parallel suites still pass.
• CI red is two darwin build-stage flakes (build-cpp / build-rust agent failures) on a TypeScript-only diff — unrelated infra, as the author noted.
• The one remaining inline finding is a nit: the new _destroy comment's "on Windows" qualifier is inaccurate (Node's kIsPerformingIO/kIoDone pair runs on all platforms). It's documentary only and doesn't affect runtime behavior.
• kReadStreamFastPath is now write-only (assigned in the constructor, only read in a commented-out pipe block). Harmless dead state; fine to leave for a follow-up cleanup.

[Jarred-Sumner]

Delete remaining references to kReadStreamFastPath. It is no longer used. Including the comment.

[claude]

Nothing further from me — all four prior threads are resolved and c3d113f addresses the last nit. Deferring final sign-off to a maintainer since this touches fs.ReadStream destroy/fd-lifecycle semantics (in particular the now-unconditional immediate close() in _destroy, which matches what the common path has done since #16754 but is worth a human glance).

Extended reasoning...

Overview

The PR fixes #30919 by adding an if (this.destroyed) return guard in ReadStream._read's fs callback so a late-arriving read result can't push(null) after destroy() and suppress ERR_STREAM_PREMATURE_CLOSE for async iterators / finished(). In response to my earlier review it also (a) drops the dead once(kReadStreamFastPath, ...) branch in _destroy that leaked fds for { start: 0, autoClose: true }, (b) removes the now-unused kReadStreamFastPath symbol, constructor assignment, and no-op pipe override, and (c) drops the orphaned encoding destructure. Two regression tests added in test/js/node/fs/fs.test.ts.

Security risks

None identified. No user-controlled input reaches new code paths; the change only short-circuits a callback and removes dead code. No injection, auth, or data-exposure surface.

Level of scrutiny

Moderate-to-high. fs.ReadStream is a core Node-compat API and the changes affect destroy/close ordering and fd lifecycle. Each individual edit is small and well-reasoned — the destroyed guard mirrors Node exactly, and the _destroy simplification makes a rare edge case match the behavior the default createReadStream(path) path has already had since #16754 — but stream-destruction semantics are subtle enough that a maintainer should confirm the reasoning, particularly that closing the fd immediately while a thread-pool read may still be in flight is acceptable for Bun (Node defers via kIsPerformingIO/kIoDone; the author's position is that Bun's common path has already done immediate close without issue).

Other factors

I've reviewed this PR across four rounds; every comment was addressed and all threads are resolved. The bug-hunting system found nothing on the latest revision. CI failures noted in the thread are unrelated infra flakes (darwin build agents, heap-snapshot SIGKILL, a flaky-tagged worker_threads test). No CODEOWNERS entry covers these files. The removed pipe override was a pure no-op delegating to Readable.prototype.pipe, so its removal is behaviorally inert. Net: I have no remaining concerns, but this isn't a mechanical/config change, so deferring rather than approving.