sys: one single-shot POSIX close behind close_allowing_standard_io and bun_sys::close

[robobun]

Behaviour change: none

Problem

• Two functions in bun_sys close a descriptor on POSIX: FdExt::close_allowing_standard_io (src/sys/fd.rs) and bun_sys::close (src/sys/lib.rs). Each has its own copy of the call and of the rule for its result, one copy for each platform. That makes five copies.
• The rule is the same in all five: only EBADF surfaces. sys, node:fs: close reports every errno except EINTR and EINPROGRESS #44330 changes that rule. It must not edit five places.

Fix

• close_once issues the close. close_error holds the rule. Both functions call them.
• The rule does not change. The close$NOCANCEL declaration in fd.rs has no other user, so this PR removes it.
• Verified: the existing suites pass unchanged on a debug build with ASAN. Notes has the list.

Background

• Fd::close() is the close that most of bun calls. It drops the result. A debug build asserts that there is none: an error here means a use after close.
• close_allowing_standard_io returns the result. fs.close and fs.closeSync use it.
• On Linux, close(2) releases the descriptor even when it returns an error. So bun issues it once and does not retry.
• The design review compared this with one new close function for the copy paths of node:fs: copyFile and cp fail and remove the destination when close(2) of the destination fails #44331. That function leaves the five copies of the rule as they are.

Downsides

• None found. Release builds of 9f70da0 and of this PR have the same text size (80,666,326 B). Each of the 284 functions that issue close has the same code in both (objdump -d, addresses normalised).

Notes

Stack. PR 1 of 3. #44330 changes the rule: close reports every errno except EINTR and EINPROGRESS. #44331 uses that in fs.copyFile and fs.cp.

Order. The self-review of the stack (it ran on #44331) said to land this PR first and alone: it is based on main and changes no behaviour.

Overlap with #42819. That PR (Remove libuv on Windows) edits the same import lines at the top of src/sys/fd.rs, and the Windows arm of close_allowing_standard_io below them. This PR removes the macOS close$NOCANCEL declaration, which is the last user of c_int on macOS in that file. After both PRs the file needs neither c_int nor c_void. The conflict is those import lines.

Suites run on a debug build with ASAN of this commit, Linux x64:

• test/js/node/fs/fs.test.ts (it has "fs.close on stdio descriptors"): 614 pass, 8 skip.
• cp.test.ts, promises.test.js, dir.test.ts: 99 pass, 10 skip.
• Node's test-fs-close.js, test-fs-close-errors.js, test-fs-copyfile.js.
• The host was under heavy load. Two tests exceeded the 5 s timeout in those runs: "readFileSync on a FIFO larger than the stat size" and "fs.cp recursive returns ENAMETOOLONG". Both pass when run alone with a longer timeout. The second also exceeds 5 s with the released bun on this host.

Machine code. Same toolchain for both release builds.

• size: text, data and bss are equal.
• nm -S: no function changed size, once swaps between identical folded functions are cancelled.
• objdump -d: the inlined syscall(SYS_close, fd) sequence occurs at 485 sites in 284 functions in both builds, and each of those functions disassembles to the same instructions.
• gdb, single-step: fs.closeSync runs 154 instructions in both builds, fs.copyFileSync 1,825.

Other targets. cargo check -p bun_sys passes for x86_64 and aarch64 linux (gnu and musl), x86_64 and aarch64 android, x86_64 and aarch64 darwin, x86_64 freebsd, and x86_64 and aarch64 windows. The macOS and FreeBSD arms are compiled, not run.

Related. #38630 adds a debug ledger to both close entry points. After this PR there is one place for it.

[robobun]

Status

This PR changes no behaviour, so it has no reproduction of its own. It is the groundwork of a stack:

• sys, node:fs: close reports every errno except EINTR and EINPROGRESS #44330 makes close report every errno except EINTR and EINPROGRESS. Its status comment has the reproduction for fs.closeSync, fs.close, FileHandle.close and fs.createWriteStream.
• node:fs: copyFile and cp fail and remove the destination when close(2) of the destination fails #44331 makes fs.copyFile and fs.cp fail and remove the destination when close of the destination fails. Its status comment has the reproduction for the copy.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: oven-sh/bun/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Essentials

Run ID: d4a40667-f130-4126-8f42-e76fd60edd70

📥 Commits

Reviewing files that changed from the base of the PR and between 5a183c1 and b6666c4.

📒 Files selected for processing (2)

• src/sys/fd.rs
• src/sys/lib.rs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review.

---

Walkthrough

POSIX file-descriptor closing now uses shared helpers for the close attempt and error handling. Only EBADF becomes a tagged close error. Windows closing remains unchanged.

Changes

POSIX close handling

Layer / File(s)	Summary
Single-attempt POSIX close src/sys/lib.rs	close_once performs one platform-specific close and preserves errno on failure. Linux and Android use the raw syscall result. macOS uses close$NOCANCEL; other POSIX targets use libc.
Centralized close errors and wiring src/sys/fd.rs, src/sys/lib.rs	Unix file descriptors now call close_error. It converts EBADF into a Tag::close error carrying the file descriptor and treats other outcomes, including EINTR, as success. The macOS close_nocancel declaration was removed.

Suggested reviewers: jarred-sumner

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to b6666

The refactor preserves existing close behavior across the inspected platform paths. No actionable merge-blocking risk remains, subject to normal build and test checks.

🚥 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 identifies the main change: sharing a single-shot POSIX close implementation between the two close entry points. It is specific and related to the changeset, although slightly awkwar…
Description check	✅ Passed	The description explains the problem, fix, behavior, downsides, verification steps, target coverage, and related changes. It does not use the exact template headings, but it provides the required info…

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

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

[claude]

LGTM — a small, behavior-preserving consolidation of the POSIX close path.

What was reviewed:

• Traced each platform arm of close_once to its target: linux_syscall::close returns Result<(), i32>, nocancel::close is a pub(crate) safe fn gated on macOS in lib.rs, and safe_libc::close is declared for non-Linux unix — the FreeBSD/other-POSIX fallback resolves.
• Compared the old per-arm sys::Error { errno: EBADF, syscall: close, fd } literals with Error::from_code_int(...).with_fd(fd): same fields, and the with_fd debug assert is already covered by the is_valid() assert at the top of close_allowing_standard_io.
• Confirmed close_once/close_error collide with nothing re-exported by pub use posix_impl::*, and the removed close$NOCANCEL extern and c_int import in fd.rs had no other users.
• Windows arm untouched; the one-line if rc < 0 { .. } else { .. } is within rustfmt's default single-line width.

Extended reasoning...

The change touches only src/sys/fd.rs and src/sys/lib.rs in the bun_sys crate, replacing three per-OS close arms in FdExt::close_allowing_standard_io and the inline logic in posix_impl::close with two shared crate-private helpers (close_once, close_error), net -64/+24 lines. It touches no security-sensitive surface (no auth, parsing, or untrusted input); the fd-close semantics (single close, never retried on EINTR, only EBADF surfaced) are preserved line for line. No CODEOWNERS entry covers src/sys, there are no third-party reviews or open threads, and the bug hunt exited on dry_streak. I could not run cargo check in this sandbox, so the cross-platform arms were verified by tracing symbol definitions and cfg gates by hand; they are consistent with the author's reported multi-target cargo check.

[robobun]

@dylan-conway a note on overlap with #42819: both PRs edit the import lines at the top of src/sys/fd.rs. This PR removes the macOS close$NOCANCEL declaration from that file, which is the last user of c_int there on macOS. After both PRs the file needs neither c_int nor c_void. The conflict is only those lines.