[robobun]

Problem

• test/js/node/tty.test.ts, "leaves fds 0-2 open when it has no FileSink" (tty: keep fds 0-2 open on destroy, and give WriteStream its own prototype #47395), passes only on a retry in 11 of the last 400 CI builds, on every Linux lane. The child prints "sinks":[true,true,false,false] or [true,false,true,false], not [true,false,false,false].
• The test destroys a warmup tty.WriteStream, awaits 'close', then opens files until EMFILE. On Linux the stream's FileSink closes its descriptor on a pool thread after it reports the close (src/io/pipes.rs:108). A close after the last open() frees a descriptor, and one stream under test gets a FileSink.

Fix

• The child script records the descriptor that the warmup stream opens. After 'close' it waits until that descriptor is closed.
• Correct because that close is the only one in flight. Under ulimit -n 64 a scan of 0 to 63 sees every descriptor. Without the limit nothing is scanned, so Windows runs no new code.
• Verified on a release build of main (2596ac5), 8 runs at once: 33 of 300 fail before, 0 of 1000 after. bun bd test test/js/node/tty.test.ts: 15 pass.

Background

• tty.WriteStream writes through a FileSink, which on POSIX owns a dup of the fd. If the dup fails, the stream writes with node:fs to the fd itself.
• PollOrFd::close_impl hands close(2) of a pipe to the work pool and calls on_close in the same frame. io: close a pipe in PollOrFd before it reports closed (POSIX) #46800 proposes to close in place and waits for a ruling. This change holds either way.
• Considered: keep the warmup stream open while the descriptors are held. A debug build then fails (Notes).

Behaviour change: none

Notes

Reproduction. Run the one test many times, 8 at once, on a release build:

seq 300 | xargs -P 8 -I{} sh -c 'bun test test/js/node/tty.test.ts -t "no FileSink" >/dev/null 2>&1 && echo PASS || echo FAIL' | sort | uniq -c

Main (2596ac5): 267 PASS, 33 FAIL. The failures print [true,true,false,false] (14), [true,false,true,false] (14) and [true,false,false,true] (5). An earlier run of the same command gave 284 PASS and 16 FAIL. This branch: 1000 PASS. On a debug build, 4 at once: 24 of 24 PASS.

Probes of the child script.

• Before the warmup stream, descriptors 0 to 5 are open. The stream opens descriptor 6 and nothing else, on a release build and on a debug build.
• Release build: at 'close', descriptor 6 is still open in 5 of 6 runs. It is closed one setImmediate later. Debug build: it is closed at 'close' in 4 of 4 runs.
• Main, with one more field for the number of descriptors that the loop opened, 400 runs, 8 at once: 58 in 147 runs (the pooled close ran first) and 57 in 253 runs. 35 of those 253 print a wrong list.
• This branch, 2000 runs, 8 at once: 58 in every run, and the expected list in every run. The wait loop ran at least once in 1124 of them.

The first attempt. I moved warmup.destroy() below the construction of the three streams, so that no close was in flight. A release build passed 1000 of 1000 runs. A debug build fails every run with FATAL: bun-debug failed to load bundled version of "internal:process/tick_ring": a stream without a FileSink calls process.nextTick in its constructor, and the debug build reads that module from disk at the first call. On main the warmup's destroy() makes that first call before the descriptors are gone. So the warmup stays as it is, and the script waits for its descriptor.

CI sightings. 11 of 400 builds, #137568 to #138371 (Nov 4 to Nov 7): 137823, 137881, 137894, 137969, 138047, 138063, 138093, 138140, 138213, 138333, 138350. Lanes: debian 13 x64 (3), alpine 3.23 aarch64 (3), debian 13 aarch64 (2), ubuntu 25.04 aarch64 (2), alpine 3.23 x64 (1). Output: [true,true,false,false] 6, [true,false,true,false] 5. The first is 137823 (Nov 5, 22:35 UTC). The 140 scanned builds before it have none.

macOS. close_impl closes in place there when the poll is writable and nonblocking. The 400 builds have no sighting on a macOS lane. I did not run macOS. The wait loop does not run when the descriptor is already closed.

If the descriptor never closed, that would be a descriptor leak in FileSink. The wait stops after 10 s and throws open after 'close': fd <n>. The child exits with code 1, and the message is in the stderr field of the failed assertion. CI gives each test of this file 90 s (270 s on the ASAN lane), so the 10 s limit reports first there. A copy of the script that keeps one more descriptor in the list prints error: open after 'close': fd 2 and exits with 1.

After the deadline was added (second commit): release build, 8 at once, 1000 of 1000 runs pass. Debug build: bun bd test test/js/node/tty.test.ts 15 pass, and the one test 16 of 16 runs, 4 at once.

---

no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/node/tty.test.ts

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

The TTY test now waits for sink descriptors opened by a warmup WriteStream to close before exhausting descriptors. It uses a configurable descriptor limit for the exhaustion case.

Changes

TTY descriptor exhaustion test

Layer / File(s)	Summary
Warmup stream and descriptor exhaustion test/js/node/tty.test.ts	The test records open descriptors, creates and destroys a warmup stream, and waits up to 10 seconds for its sink descriptors to close. The child shell uses a configurable descriptor limit when exhaustion is enabled.

Possibly related PRs

• oven-sh/bun#47395: Adds the no-sink test whose warmup stream descriptor cleanup is addressed here.

Suggested reviewers: dylan-conway, jarred-sumner

Severity of issue fixed: Low

Merge Risk: Minimal · up to 21e5b

No merge-blocking issue is established for this test-only synchronization change. Mergeability remains subject to normal test checks.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title names the change: a wait for the warmup stream's descriptor to close in the tty test. It matches the diff summary and the PR objectives. It is specific and not vague.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Description check		The description follows the repository template, which asks for "What does this PR do?" and "How did you verify your code works?". The Problem, Fix, and Background sections state what the change does …

Full details: Description check

Explanation

The description follows the repository template, which asks for "What does this PR do?" and "How did you verify your code works?". The Problem, Fix, and Background sections state what the change does and why. The Fix section gives the behavior change, which is test-only, and says the change does nothing on Windows. The verification section lists a release-build reproduction (33 of 300 failing before, 0 of 1000 after), the result of bun bd test test/js/node/tty.test.ts (15 pass), and a reproduction command. The description also covers the failure mode if the descriptor never closes, a 10 second deadline, and macOS coverage. Gaps: the macOS path was not run, and the author notes that CI sightings and the wait loop's behavior are based on observation. These are non-critical for the template sections.

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

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

Recent review info

Run configuration

• Source mode: Auto
• Repository: oven-sh/bun
• Branch: robobun/30445a19/tty-test-wait-for-sink-close
• File: .coderabbit.yaml
• Commit: 21e5b14
• Layers: Repository YAML
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 35a38328-33ed-4b82-986b-d251c4d593cc

Commits

Reviewing files that changed from the base of the PR and between 2596ac5 and 21e5b14.

Files selected for processing: 1

• test/js/node/tty.test.ts

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

[robobun]

^{Updated 10:05 PM PT - Nov 7th, 2026}

✅ @robobun, your commit 21e5b149095d47e6f79e928c7a15a610315a71d1 passed in Build #138485! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 48167

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

bun-48167 --bun

[claude]

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

LGTM — a small, self-contained de-flake of a single test; the inline note is a nit, not a blocker.

Extended reasoning...

The diff touches only test/js/node/tty.test.ts (+15/-1), replacing a hardcoded ulimit value with a shared constant and making the child script snapshot open fds around the warmup tty.WriteStream so it can wait for the FileSink's dup to actually close before the EMFILE loop. It touches no production code and no security-sensitive surface. The fd diff is correctly a no-op when exhaustDescriptors is false (Array(0) yields an empty list), the scan range matches the ulimit so every child descriptor is visible, and the existing assertions on sinks/destroyed/open are unchanged, so the test still protects the same property. The changed path is not covered by CODEOWNERS and the hunt ran dry with only a deadline-on-poll nit outstanding.

[robobun]

Status: ready for review. Test-only change.

CI: no test failed. In build 138485, 184 of 188 jobs passed: each Linux lane, the ASAN lane, Windows, and macOS arm64. The other 4 jobs are darwin any x64 - test-bun. They expired after 96 minutes in the queue with no agent, and they wait in the queue again. The macOS x64 test agents were all busy with older builds: 47 of the last 100 builds of the pipeline have expired jobs on that lane.

How I reproduced it. On a release build of main (2596ac5), run the one test 300 times, 8 at once:

seq 300 | xargs -P 8 -I{} sh -c 'bun test test/js/node/tty.test.ts -t "no FileSink" >/dev/null 2>&1 && echo PASS || echo FAIL' | sort | uniq -c

• Main: 33 of 300 runs fail, with the outputs that CI shows ("sinks":[true,true,false,false], [true,false,true,false]).
• This branch: 0 of 1000 runs fail.
• bun bd test test/js/node/tty.test.ts passes (15 tests).

[claude]

LGTM — the second commit bounds the wait, which was the only thing I had flagged, and the fix matches the mechanism in source.

What was reviewed:

• Confirmed the comment's claim in src/io/pipes.rs: on non-macOS POSIX PollOrFd::close_impl always hands close(2) to the work pool (Closer::close -> WorkPool::schedule_owned) and runs on_close_fn in the same frame, so there is no event to await and a bounded poll is the right shape.
• Checked the exhaustDescriptors === false path: openFds() returns [], so sinkFds is empty and the loop is skipped; the unlimited test and Windows run no new logic.
• Checked the deadline path: the throw inside the async IIFE surfaces as an unhandled rejection, so a leaked descriptor fails the test with the fd number on stderr rather than a harness timeout.

Extended reasoning...

The diff touches only test/js/node/tty.test.ts, inside the destroyStdioStreams helper: it hoists the ulimit value into a constant, snapshots open fds around the warmup tty.WriteStream construction, and after 'close' polls on setImmediate with a 10 s deadline until the sink's dup'd fd is actually closed before the EMFILE exhaustion loop. It touches no production code and no security-sensitive surface. Approve was decided by the change being small and self-contained, the cross-thread close claim being verifiable in src/io/pipes.rs and src/io/lib.rs, the prior inline nit being addressed by the new commit, and no CODEOWNER or outstanding third-party objection on the file.