io(windows): honor CLOSE_HANDLE in WindowsBufferedReader close path

[robobun]

Symptom

Intermittent EBADF: bad file descriptor, fstat in unrelated Bun.file(path).text() calls on Windows. The most visible CI victim is test/cli/install/bun-install.test.ts (15 flaky hits across the last 40 PR builds, spread across many different test cases), because its dummy registry serves every tarball via new Response(Bun.file(path)).

EBADF: bad file descriptor, fstat
 syscall: "fstat",
   errno: -9,
    code: "EBADF"
      at async <anonymous> (test/cli/install/bun-install.test.ts:6461)

Cause

FileResponseStream::start clears ReaderFlags::CLOSE_HANDLE on its BufferedReader so it can close the fd itself in Drop (src/runtime/server/FileResponseStream.rs:182). PosixBufferedReader checks that flag before closing (src/io/PipeReader.rs:283/356/374/385). WindowsBufferedReader defines the flag (:1143) and sets it in Default (:1155) but never reads it, so close_impl's Source::File arm unconditionally calls File::detach() which queues uv_fs_close on the same CRT fd that FileResponseStream::Drop already queued a Closer::close for (:547).

Both closes are async on the libuv threadpool. Between close #1 freeing the CRT slot and close #2 running, an unrelated uv_fs_open (from another Response(Bun.file) open, or a Bun.file().text()) can be handed the recycled slot; close #2 then closes the wrong fd, and its next fstat or read sees EBADF.

Fix

Honor WindowsFlags::CLOSE_HANDLE in WindowsBufferedReader::close_impl via a new File::detach_borrowed_fd() that mirrors detach() but leaves close_after_operation unset, so no uv_fs_close is scheduled for a parent-owned fd:

• close_impl with CLOSE_HANDLE set: unchanged (detach() schedules uv_fs_close now or after the pending read).
• close_impl with CLOSE_HANDLE cleared: detach_borrowed_fd(). If idle, drop the Box<File> there; if a read is in flight, null fs.data and let on_file_read's detached branch reclaim the Box after complete().
• on_file_read's parent_ptr.is_null() path now handles both shapes (state Closing: on_close_complete frees; otherwise: free here).

BaseWindowsPipeWriter::close's !owns_fd() branch already open-coded the same sequence and now routes through the shared detach_borrowed_fd(), so the reader and writer close paths share one contract.

Verification

The double-close only exists on Windows (POSIX honors the flag), so the Linux gate cannot observe fail-before. Windows x64-baseline at 91675d0:

• fail-before: 5/15 runs of the new test fail under the system bun with EBADF: bad file descriptor, fstat 'served.bin'
• pass-after: 8/8 under the debug build with this patch

The new test is gated behind isWindows.

---

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

[robobun]

^{Updated 5:52 PM PT - Jul 10th, 2026}

❌ @robobun, your commit d23a604 has 1 failures in Build #71663 (All Failures):

• test/js/bun/http/proxy-stress-concurrent.test.ts - code 1 on 🍎 14 x64

---

🧪   To try this PR locally:

bunx bun-pr 33931

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

bun-33931 --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

Walkthrough

The change updates Windows borrowed-file detachment and boxed-file reclamation across readers and writers, while adding a stress regression test for concurrent completed and aborted served-file requests.

Changes

Windows file descriptor lifecycle

Layer / File(s)	Summary
Coordinate borrowed descriptor detachment src/io/source.rs, src/io/PipeWriter.rs, src/io/PipeReader.rs	Borrowed file descriptors are stopped without immediate closure; boxed files are reclaimed immediately when idle or by the pending callback when operations remain active.
Exercise served-file close paths test/js/bun/http/bun-serve-file.test.ts	A Windows fixture stresses full and aborted file responses, checks another file descriptor with fstatSync, and validates successful child-process output.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly matches the main change: WindowsBufferedReader now honors CLOSE_HANDLE in its close path.
Description check	✅ Passed	The description includes the problem, cause, fix, and verification details, which covers the required PR info despite different headings.

---

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

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/js/bun/http/bun-serve-file.test.ts`:
- Line 1071: Gate the regression tests “Response(Bun.file) does not double-close
the fd on Windows” and the related test at the second occurrence behind the
imported `isWindows` guard. Ensure `isWindows` is imported from `harness`, and
condition both tests so they run only on Windows, avoiding unnecessary POSIX
workload and timeout impact.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: ec35f2ee-8442-4ea8-bd02-5d56b42fc32c

📥 Commits

Reviewing files that changed from the base of the PR and between 095eb31 and da2d050.

📒 Files selected for processing (2)

• src/io/PipeReader.rs
• test/js/bun/http/bun-serve-file.test.ts

[claude]

I didn't find any bugs, but this touches Windows fd lifecycle/ownership in the libuv close path — worth a human look, especially to confirm uv_fs_close(-1) is safe across all detach() states (in-flight read vs. idle).

Extended reasoning...

Overview

This PR adds a 5-line guard to WindowsBufferedReader::close_impl in src/io/PipeReader.rs: when WindowsFlags::CLOSE_HANDLE is cleared, it sets file.file = -1 before calling detach(), so the subsequent uv_fs_close targets an invalid fd instead of double-closing one the parent (FileResponseStream) already owns and closes. It also adds a Windows-gated stress test to test/js/bun/http/bun-serve-file.test.ts that reproduces the EBADF race via concurrent full/aborted Response(Bun.file) requests plus canary fd checks.

Security risks

None identified. This is an internal fd-lifecycle fix; no user-controlled input, auth, crypto, or trust-boundary code is touched.

Level of scrutiny

High. Per this repo's review history, fd/handle ownership and "released exactly once" is the single most-blocked category of native change. The fix is small but sits squarely in that space:

• It changes Windows-only libuv fd close ordering — a code path I cannot exercise on this Linux host, and the PR's own verification is probabilistic (5/15 fail-before).
• Correctness depends on uv_fs_close(loop, req, -1, cb) being a harmless failure whose callback still fires to reclaim the Box<File>. I traced detach() → start_close() and this holds for both the idle (Deinitialized) and in-flight (Operating → complete() → start_close()) paths, but a maintainer familiar with the Windows libuv wrapper should confirm there's no third path (e.g. SyncFile) where file.file is read before the close for something other than uv_fs_close.
• The mut file binding change on the match arm is fine — file is a moved Box<File>, so mutating the field before into_raw is sound.

Other factors

• The diagnosis in the PR description is detailed and traces the exact double-close (FileResponseStream::Drop's Closer::close + File::detach()'s unconditional uv_fs_close), which matches what I see in src/io/source.rs and src/runtime/server/FileResponseStream.rs:182.
• The one CodeRabbit nit (gate the test to Windows) has been addressed and resolved; the explicit per-test timeout was also dropped in a follow-up commit.
• The test asserts stderr: "" exactly, which the repo guidelines caution against for debug/ASAN builds — but it's Windows-only where ASAN doesn't run, so likely fine in practice.
• No CODEOWNERS entry covers these paths.

[robobun]

Re the uv_fs_close(-1) safety question: both detach() states are covered.

• Idle (state == Deinitialized): detach() calls start_close() immediately, which issues uv_fs_close(loop, fs, self.file, on_close_complete). With self.file == -1, libuv's fs__close calls _close(-1), sets req->result = UV_EBADF, and fires the callback; on_close_complete (source.rs:213) ignores the result and reclaims the Box<File>.
• In-flight (Operating/Canceling): detach() nulls fs.data and sets close_after_operation. When on_file_read (PipeReader.rs:1483) fires it calls file.complete(), which runs start_close() because close_after_operation is set, reaching the same uv_fs_close(-1) as above. on_file_read then sees parent_ptr == null and returns without touching the detached reader.

Nothing reads File.file between the assignment and start_close(): complete() only touches fs and state, and on_file_read's syslog reads fs.file_fd() (the fd stored in the in-flight uv_fs_t request), not File.file. SyncFile shares the same File struct and detach path.

On the stderr: "" assertion: there is no Windows ASAN lane, and the 8/8 pass-after under bun bd on Windows produced empty stderr each time.

[robobun]

Agreed, swapped it out in 5539d12. The fix is still at the same layer (the reader must honor CLOSE_HANDLE, which the POSIX reader already does) but the mechanism is now direct instead of leaning on a no-op syscall:

• source::File::detach_borrowed_fd() mirrors detach() but leaves close_after_operation unset and returns whether an operation is still in flight.
• WindowsBufferedReader::close_impl for a Source::File with CLOSE_HANDLE cleared: if idle, drop the Box<File> there; if a read is in flight, null fs.data + stop() and let on_file_read's detached branch reclaim the Box.
• on_file_read's parent_ptr.is_null() branch now handles both shapes (state Closing: on_close_complete frees; otherwise: free here).

On whether it belongs in FileResponseStream instead: line 182 clears CLOSE_HANDLE because FileResponseStream owns the fd for both the sendfile path (where the reader is never used) and the BufferedReader path, and it also has to cover auto_close == false where neither side closes. The flag is the ownership contract; the Windows reader just wasn't reading it. Making FileResponseStream hand ownership to the reader instead would still require this change for the auto_close == false case.

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/io/PipeReader.rs`:
- Around line 1506-1509: Condense the ownership and safety comments near the
detached file handling and the corresponding block around the second occurrence
into no more than three lines each, while preserving the key details about
parent fd ownership, the absence of an active close, and safe reclamation of the
Box.

In `@src/io/source.rs`:
- Around line 175-178: Shorten the documentation comment above the detach method
to no more than three lines while preserving its explanation of parent ownership
and callback-versus-caller cleanup behavior.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 93d06f8f-e996-414e-8e47-fdbe24045bd8

📥 Commits

Reviewing files that changed from the base of the PR and between 0d407d4 and 5539d12.

📒 Files selected for processing (2)

• src/io/PipeReader.rs
• src/io/source.rs

[claude]

No bugs found, and my earlier dedup suggestion was applied in f43b74f. This still touches Windows-only libuv Box<File> lifecycle across async callbacks (unsafe raw-pointer reclaim), so it warrants a human look before merge.

Checked: detach_borrowed_fd state transitions — idle → caller drops Box, Operating/Canceling → on_file_read's null-parent branch reclaims after complete() leaves state Deinitialized; the owned-fd detach() path is unchanged (state Closing → on_close_complete frees). The writer refactor at PipeWriter.rs:1213 is behavior-preserving — on_fs_write_complete's null-parent branch already reclaims on state == Deinitialized. close_after_operation is left unset by the new helper, so complete() never schedules a close for a borrowed fd.

Extended reasoning...

Overview

The PR fixes a Windows-only double-close: WindowsBufferedReader::close_impl unconditionally called File::detach() (which queues uv_fs_close) even when CLOSE_HANDLE was cleared by FileResponseStream, which then closed the same CRT fd again in its own Drop. The fix adds File::detach_borrowed_fd() (nulls fs.data, cancels any in-flight op, does not set close_after_operation) and branches on CLOSE_HANDLE in the reader's close path. on_file_read's detached branch is extended to reclaim the Box<File> when no close was scheduled. After my earlier inline suggestion, the writer's !owns_fd() branch was refactored to use the same helper.

Security risks

None. This is fd-lifecycle plumbing with no user-input parsing, auth, or crypto surface.

Level of scrutiny

High. This is exactly the CLAUDE.md "most-blocked category": raw-pointer heap::into_raw/heap::take round-trips of a Box<File> whose uv_fs_t may still be owned by libuv, with reclaim split across three call sites (close_impl, on_file_read, on_close_complete) depending on async state. A missed reclaim leaks; a double reclaim or premature drop UAFs the uv_fs_t under a threadpool worker. The fix is Windows-only and the Linux host can't observe fail-before, so verification depends on the author's 8/8 Windows debug run and CI.

Other factors

I traced every FileState at the point detach_borrowed_fd() is called (Deinitialized/Operating/Canceling; never Closing because close_impl runs before any start_close) and confirmed each ends with exactly one heap::take. The writer-side change is a pure refactor: the pre-existing inline sequence (stop() → null fs.data → branch on state == Deinitialized) is byte-equivalent to the new helper modulo statement order, and uv_cancel doesn't synchronously fire the callback so ordering is immaterial. The test is Windows-gated, follows harness conventions, and asserts a combined {stdout, stderr, exitCode} object. Given the unsafe/async nature and that Windows CI hasn't reported yet, a human should sign off.

[Jarred-Sumner]

It shouldn't be calling uv_fs_close(-1).

[robobun]

It doesn't anymore as of 5539d12 / f43b74f (the PR body was stale; just updated it). When CLOSE_HANDLE is cleared, close_impl now calls File::detach_borrowed_fd():

pub fn detach_borrowed_fd(&mut self) -> bool {
    self.fs.data = core::ptr::null_mut();
    self.stop();
    self.state != FileState::Deinitialized
}

If idle the caller drops the Box<File> directly; if a read is in flight on_file_read's detached branch reclaims it after complete(). No uv_fs_close is scheduled for the parent-owned fd on either path. The first revision did use file.file = -1; that was replaced after Dylan flagged it.

[claude]

I didn't find any bugs, but this touches Box ownership hand-off across libuv fs callbacks on Windows — worth a human sign-off given Jarred has already engaged.

Checked: detach_borrowed_fd() leaves close_after_operation unset, so complete() lands in Deinitialized and on_file_read's new state != Closing branch reclaims the Box — equivalent to the pre-existing state == Deinitialized check in both on_fs_write_complete callbacks (PipeWriter.rs:1627, 2197). The writer's !owns_fd() refactor is behavior-identical to the removed inline sequence (stop() doesn't synchronously fire the fs callback, so null-fs.data ordering is immaterial). No path leaves the Box unreclaimed and none double-frees.

Extended reasoning...

Overview

This PR fixes a Windows-only fd double-close in WindowsBufferedReader::close_impl. FileResponseStream clears CLOSE_HANDLE on its embedded reader so it can close the fd itself in Drop, and PosixBufferedReader honors that flag — but WindowsBufferedReader defined the flag and never read it, so close_impl unconditionally called File::detach() which schedules uv_fs_close on an fd the parent is also closing. The fix adds File::detach_borrowed_fd() (nulls fs.data, calls stop(), does not set close_after_operation) and branches on CLOSE_HANDLE in close_impl. on_file_read's detached-parent branch is extended to reclaim the Box when complete() didn't schedule a close. The writer's pre-existing !owns_fd() open-coded sequence in BaseWindowsPipeWriter::close is replaced with the same helper (my earlier suggestion, now applied). A Windows-gated stress test is added.

Security risks

None. This is an internal I/O lifetime fix with no user-controlled input reaching the changed code paths; the fd in question is opened by Bun itself for Response(Bun.file(path)).

Level of scrutiny

High. The change is small and well-reasoned, but it sits squarely in the most-blocked review category for this repo: native memory safety around raw-pointer Box ownership handed to libuv callbacks. Correctness hinges on the FileState state machine — after complete(), state is either Deinitialized (borrowed detach → reclaim in on_file_read) or Closing (owned detach → on_close_complete reclaims). I traced all four combinations (owned/borrowed × idle/in-flight) and each has exactly one reclaimer, and the writer callbacks at PipeWriter.rs:1627/2197 already implement the same contract with the equivalent state == Deinitialized predicate. But this is Windows-only code I cannot execute, and a mistake here is a UAF or fd leak rather than a test failure.

Other factors

Jarred already engaged on this PR (flagging the earlier uv_fs_close(-1) approach, which was replaced) and hasn't approved yet; deferring so he or another maintainer can confirm the final shape. All prior review feedback (CodeRabbit's isWindows gate and comment-length nits, my writer-dedup suggestion) has been applied. The test asserts stderr: "", which is fine here since there is no Windows ASAN lane.

[robobun]

CI status on the re-roll (build 71663):

• The only error-level failure is test/js/bun/http/proxy-stress-concurrent.test.ts on darwin 14 x64 (https-proxy → https-origin mode=complete ×1200, 1/1200 requests failed). This diff touches only the Windows Source::File close path and a Windows-gated test; it does not touch the HTTP client or proxy code.
• Two darwin-14-aarch64 jobs are still scheduled (agent availability; the previous build had one expire).
• Everything else is in the flaky-retried bucket and matches the pre-existing flake set (update_interactive_install, dev-and-prod, test-repl-close, napi.test.ts, etc.). bun-install.test.ts and bun-serve-file.test.ts do not appear, consistent with the fix.

All Windows lanes are green. Ready for review.