io(windows): don't uv_close the stdin_tty singleton from BaseWindowsPipeWriter by robobun · Pull Request #34350 · oven-sh/bun

robobun · 2026-07-16T09:36:48Z

What

On Windows, Source::open_tty(fd=0) returns a process-static uv_tty_t singleton (src/io/source.rs stdin_tty::get_stdin_tty) that is shared by every reader and writer on fd 0. WindowsBufferedReader::close_impl already guards this case and skips uv_close when the tty is the singleton (src/io/PipeReader.rs:1793). BaseWindowsPipeWriter::close did not have the same guard and called uv_close on it unconditionally.

Bun.file(0).writer() on a Windows console takes this path: Blob::get_writer (fd 0 is not stdout/stderr) calls start(fd, true) which goes through Source::open and, because uv_guess_handle(0) == UV_TTY, stores Source::Tty(<stdin_tty singleton>) on the WindowsStreamingWriter. The FileSink has owns_fd = false, so end() leaves the source in place, and the writer's Drop later runs close_without_reporting() which reaches BaseWindowsPipeWriter::close() and uv_closes the singleton.

Once that happens the INITIALIZED flag in stdin_tty stays set, so every later open_tty(fd=0) hands out the closed static without reinitialising it.

Repro

Run in a Windows console (so fd 0 is a TTY):

for (let i = 0; i < 8; i++) {
  (() => { const w = Bun.file(0).writer(); w.end(); })();
  Bun.gc(true);
  await new Promise(r => setImmediate(r));
}
process.stdin.setRawMode(true);

Debug build: the second iteration's Drop calls close_without_reporting(), which calls self.get_fd() on the now-closed singleton. uv_fileno leaves the out-param at INVALID_HANDLE_VALUE, and Fd::from_system trips:

panic: assertion failed: (h as u64) <= FD_VALUE_MASK

Release build: no assert, the loop completes, and the trailing setRawMode(true) (which routes to Source__setRawModeStdin and therefore the same singleton) fails with setRawMode failed with errno: 9 (EBADF). Two concurrent writers would also hit libuv's uv_close double-close assert(0) in debug.

Fix

Mirror the reader's guard: in BaseWindowsPipeWriter::close(), skip uv_close (and the data pointer rewrite) when is_stdin_tty(p). on_tty_close now debug_asserts the invariant instead of branching on it, matching the reader's callback.

Test

filesink.test.ts gains a Windows-only test that spawns a child under Bun.Terminal (so its stdin is a ConPTY tty), runs the create/end/GC cycle above, then asserts process.stdin.setRawMode still succeeds. Verified on Windows x64:

without the fix, debug: child panics with the FD_VALUE_MASK assertion, no RESULT line
without the fix, release canary: child prints setRawErr=setRawMode failed with errno: 9
with the fix: child prints RESULT ok setRawErr=none

Full filesink.test.ts, terminal-spawn.test.ts and tty.test.ts pass on Windows with the change; the test is skipped on POSIX.

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/bun/util/filesink.test.ts

…ipeWriter Source::open_tty(fd=0) returns a process-static uv_tty_t singleton that is shared with every reader and writer on fd 0. WindowsBufferedReader already gates its close path on is_stdin_tty() and leaves the singleton open; the writer side did not, so a FileSink on Bun.file(0) closed it on Drop. The INITIALIZED flag is sticky, so the next open_tty(fd=0) handed out the closed handle without reinit. In debug builds the next writer's close_without_reporting() tripped Fd::from_system's FD_VALUE_MASK assert (uv_fileno returns INVALID_HANDLE_VALUE on a closed handle); in release builds later consumers such as process.stdin.setRawMode() failed with EBADF. Mirror the reader's guard in BaseWindowsPipeWriter::close() and keep the on_tty_close debug_assert in sync.

coderabbitai · 2026-07-16T09:36:55Z

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: 1883218f-ac93-494c-9924-f42b5e6fae44

📥 Commits

Reviewing files that changed from the base of the PR and between 0ecd508 and 0ea6a28.

📒 Files selected for processing (2)

src/io/PipeWriter.rs
test/js/bun/util/filesink.test.ts

Walkthrough

Changes

Windows stdin TTY lifecycle

Layer / File(s)	Summary
Preserve stdin TTY during writer close `src/io/PipeWriter.rs`	Windows writer teardown skips `uv_close` for the process-static stdin TTY and retains normal cleanup for non-stdin TTY handles.
Verify stdin remains usable `test/js/bun/util/filesink.test.ts`	A Windows-only subprocess test repeatedly closes stdin writers and confirms raw-mode changes still succeed.

Possibly related PRs

oven-sh/bun#33931: Both modify Windows PipeWriter.rs close teardown logic.

Suggested reviewers: jarred-sumner

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is concise, specific, and accurately describes the main Windows pipe-writer fix.
Description check	✅ Passed	The description covers what changed and how it was verified, though it uses different headings than the template.
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.

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

claude

LGTM — mirrors the existing reader-side guard for the stdin_tty singleton.

What was reviewed:

Verified the new close() guard and on_tty_close body are byte-for-byte the same pattern as PipeReader.rs:1793/1873.
Confirmed the writer's on_tty_close has no other call sites, so replacing the runtime check with debug_assert! is safe.
Checked source.rs — is_stdin_tty is a pointer-equality check against the process-static, and the Source::Tty doc comment already mandates this gate on destroy paths.

Extended reasoning...

Overview

Two files: src/io/PipeWriter.rs adds an is_stdin_tty(p) guard in BaseWindowsPipeWriter::close() before calling uv_close on a Source::Tty, and tightens on_tty_close from a runtime branch to a debug_assert! now that its sole caller enforces the precondition. test/js/bun/util/filesink.test.ts gains a Windows-only regression test that spawns a child under Bun.Terminal (ConPTY stdin), cycles Bun.file(0).writer() create/end/GC, then asserts process.stdin.setRawMode still works.

Security risks

None. Windows-only libuv handle-lifecycle bookkeeping; no user-controlled input, no auth/crypto/network surface.

Level of scrutiny

Low-to-moderate. This is native handle-lifetime code, but the change is a mechanical copy of the guard already present in WindowsBufferedReader::close_impl (PipeReader.rs:1793-1801) and its close callback (PipeReader.rs:1873-1880). The Source::Tty variant docs in source.rs:20-27 explicitly state destroy paths must gate heap::take on !is_stdin_tty(); the writer was the outlier. The on_tty_close free function is referenced only from the one (*p).close(on_tty_close) call now inside the else branch, so the invariant it debug_asserts is guaranteed by construction.

Other factors

The PR description documents fail-before/pass-after on Windows for both debug (FD_VALUE_MASK panic) and release (EBADF from setRawMode). The test wires the terminal exit callback to a resolver so it can't hang if the child dies without emitting RESULT, and uses toContain on accumulated ConPTY output which tolerates escape-sequence noise. No CODEOWNERS cover src/io/. No outstanding review comments.

… the asserted tail

robobun · 2026-07-16T10:19:37Z

^{Updated 7:17 AM PT - Jul 16th, 2026}

❌ @robobun, your commit 0ea6a28 has 1 failures in Build #73886 (All Failures):

test/js/node/test/parallel/test-worker-message-port-transfer-terminate.js - SIGABRT on 🐧 13 x64-asan

🧪 To try this PR locally:

bunx bun-pr 34350

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

bun-34350 --bun

claude

Beyond the inline nit, I also checked that converting on_tty_close's runtime if !is_stdin_tty guard to a debug_assert! is safe — the only call site now gates before scheduling the close, and the result is byte-identical to the reader's callback at PipeReader.rs:1873. Also confirmed *self.source_mut() = None still runs on the stdin-tty branch, so the source is cleared as before.

Extended reasoning...

The native change is a mechanical mirror of the existing reader-side guard (PipeReader.rs:1793) and matches the contract documented in source.rs ("destroy paths gate heap::take on !is_stdin_tty()"). Deferring rather than approving because this is Windows-only uv handle lifecycle code that the author could not run locally (per the evidence footer), so CI on Windows is the actual verification.

…d value

claude

LGTM — the [END] sentinel in 0ea6a28 closes the ConPTY split-read window I flagged, and the guard now matches the reader side exactly.

Checked that on_tty_close has no other call sites in PipeWriter.rs, so converting its runtime is_stdin_tty check to a debug_assert! is safe — it mirrors PipeReader.rs:1876-1878 byte-for-byte. Also confirmed stdin_tty::get_stdin_tty never resets INITIALIZED on close, which is the mechanism the PR description describes.

Extended reasoning...

Overview

Two-file change: src/io/PipeWriter.rs adds an is_stdin_tty(p) guard in BaseWindowsPipeWriter::close() so the process-static uv_tty_t singleton for fd 0 is never uv_closed, and converts on_tty_close's runtime singleton check to a debug_assert!. test/js/bun/util/filesink.test.ts gains a Windows-only ConPTY test that cycles Bun.file(0).writer() under GC and asserts process.stdin.setRawMode still works afterward.

Security risks

None. Windows-only I/O teardown path; no user-controlled input reaches the changed branch. The guard is a pointer-equality check against a process-static.

Level of scrutiny

Moderate — this is libuv handle lifetime code, so a wrong change could UAF or double-close. But the fix is a line-for-line mirror of the existing reader-side guard (PipeReader.rs:1793 and :1876-1878), which has been in place and is the documented contract in source.rs ("destroy paths gate heap::take on !is_stdin_tty()"). I verified on_tty_close in PipeWriter.rs has exactly one caller (the newly-guarded site), so replacing its runtime branch with debug_assert! cannot reach heap::take on the static in release builds.

Other factors

My previous review flagged a ConPTY chunk-boundary race in the test's readiness gate; 0ea6a28 addressed it by appending an [END] sentinel after the asserted value on both output paths and gating on that — the asserted substring is now strictly before the gate token. The bug-hunting pass on the latest revision found nothing. The PR description documents fail-before/pass-after on both debug and release Windows builds.

robobun · 2026-07-16T12:07:14Z

CI summary for 0ea6a28 (build #73886, final: 284 passed / 2 failed):

filesink.test.ts (the new Windows-only test) passed on every Windows lane.
:debian: 13 x64-asan failed on test/js/node/test/parallel/test-worker-message-port-transfer-terminate.js, a JSC exception-scope assertion in worker termination that is also failing on main and unrelated to this Windows libuv tty change.
:darwin: 14 aarch64 failed at tart VM boot (guest never up after retry / The number of VMs exceeds the system limit); the job never ran any tests.
Everything else is retry-pass flake (mongodb RSS threshold, fetch h2, repl-close EPIPE, install proxy, webview, next-pages, 20144) in code this diff does not touch.

Ready for review.

robobun · 2026-09-13T18:12:11Z

Closing as part of a cleanup of stale pull requests. This PR has had no new commits since 2026-07-16, it conflicts with main, and its last CI run failed. This is not a judgment on the fix itself. If the problem still reproduces on a current build, reopen this PR after a rebase or open a new one against main.

github-actions Bot added the claude label Jul 16, 2026

claude Bot reviewed Jul 16, 2026

View reviewed changes

test: gate on the last field prefix so ConPTY split reads cannot drop…

479bab3

… the asserted tail

claude Bot reviewed Jul 16, 2026

View reviewed changes

Comment thread test/js/bun/util/filesink.test.ts Outdated

test: gate the ConPTY reader on a trailing sentinel after the asserte…

0ea6a28

…d value

claude Bot reviewed Jul 16, 2026

View reviewed changes

robobun mentioned this pull request Aug 21, 2026

Windows: one stdin uv_tty_t per VM, closed with the Worker that opened it #39981

Open

robobun closed this Sep 13, 2026

Navigation Menu

io(windows): don't uv_close the stdin_tty singleton from BaseWindowsPipeWriter - #34350

io(windows): don't uv_close the stdin_tty singleton from BaseWindowsPipeWriter#34350
robobun wants to merge 3 commits into
mainfrom
farm/1a0c10c4/win-pipewriter-stdin-tty-guard

robobun commented Jul 16, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Jul 16, 2026 •

edited

Loading

Uh oh!

claude Bot left a comment

Uh oh!

robobun commented Jul 16, 2026 •

edited

Loading

Uh oh!

claude Bot left a comment

Uh oh!

Uh oh!

claude Bot left a comment

Uh oh!

robobun commented Jul 16, 2026 •

edited

Loading

Uh oh!

robobun commented Sep 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

robobun commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Repro

Fix

Test

Uh oh!

coderabbitai Bot commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Overview

Security risks

Level of scrutiny

Other factors

Uh oh!

robobun commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Overview

Security risks

Level of scrutiny

Other factors

Uh oh!

robobun commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

robobun commented Sep 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

robobun commented Jul 16, 2026 •

edited

Loading

coderabbitai Bot commented Jul 16, 2026 •

edited

Loading

robobun commented Jul 16, 2026 •

edited

Loading

robobun commented Jul 16, 2026 •

edited

Loading