diff --git a/src/event_loop/MiniEventLoop.rs b/src/event_loop/MiniEventLoop.rs index c46b904a2835..9b8189572f06 100644 --- a/src/event_loop/MiniEventLoop.rs +++ b/src/event_loop/MiniEventLoop.rs @@ -405,6 +405,8 @@ bun_io::link_impl_EventLoopCtx! { (*this).after_event_loop_callback_ctx = ctx; }, pipe_read_scratch() => &raw const *(*this).pipe_read_scratch, + // No raw mode on the mini loop: a reader of fd 0 opens its own tty. + stdin_tty() => core::ptr::null_mut(), } } diff --git a/src/io/PipeReader.rs b/src/io/PipeReader.rs index 038dcc8254ae..6baf915f93fe 100644 --- a/src/io/PipeReader.rs +++ b/src/io/PipeReader.rs @@ -1236,7 +1236,7 @@ impl WindowsBufferedReader { let size = limit.clamp_len(suggested_size); // Tty reads must not target `_buffer`: libuv can retain the pointer // past reader teardown (see `uv::Tty::read_scratch` for the contract). - if matches!(self.source, Some(Source::Tty(_))) { + if matches!(self.source, Some(Source::Tty(_) | Source::StdinTty(_))) { let scratch = self .source .as_mut() @@ -1311,7 +1311,7 @@ impl WindowsBufferedReader { // Use the event loop from the parent, not the global one // This is critical for spawnSync to use its isolated loop let loop_ = self.vtable.loop_(); - let source = match Source::open(loop_.cast(), fd) { + let source = match Source::open(loop_.cast(), fd, Some(self.vtable.event_loop())) { sys::Result::Err(err) => return sys::Result::Err(err), sys::Result::Ok(source) => source, }; @@ -1408,7 +1408,7 @@ impl WindowsBufferedReader { let mut b = unsafe { *buf }; let slice = unsafe { b.slice_mut() }; let data = &mut slice[..len]; - if matches!(this.source, Some(Source::Tty(_))) { + if matches!(this.source, Some(Source::Tty(_) | Source::StdinTty(_))) { // Tty chunks arrive in the tty-owned scratch; stage them // into `_buffer` so `on_read` commits them like a pipe chunk. this._buffer.reserve(len); @@ -1668,7 +1668,7 @@ impl WindowsBufferedReader { return sys::Result::Err(err); } } - Source::Pipe(_) | Source::Tty(_) => { + Source::Pipe(_) | Source::Tty(_) | Source::StdinTty(_) => { // SAFETY: source is a live Pipe/Tty stream handle. if let Some(err) = unsafe { uv::uv_read_start( @@ -1707,7 +1707,7 @@ impl WindowsBufferedReader { Source::File(file) | Source::SyncFile(file) => { file.stop(); } - Source::Pipe(_) | Source::Tty(_) => { + Source::Pipe(_) | Source::Tty(_) | Source::StdinTty(_) => { // SAFETY: stream handle is live (just matched a stream source). unsafe { uv::uv_read_stop(source.to_stream()) }; } @@ -1759,18 +1759,26 @@ impl WindowsBufferedReader { #[cfg(windows)] Source::Tty(tty) => { let p = tty.as_ptr(); - if crate::source::stdin_tty::is_stdin_tty(p) { - // Node only ever closes stdin on process exit. - } else { - // SAFETY: tty is a live heap-allocated Tty*; - // `Tty::close` keeps whole-struct provenance so - // on_tty_close may reclaim the Box. - unsafe { - (*p).uv.data = p.cast::(); - crate::source::Tty::close(p, Self::on_tty_close); + // SAFETY: tty is a live heap-allocated Tty*; + // `Tty::close` keeps whole-struct provenance so + // on_tty_close may reclaim the Box. + unsafe { + (*p).uv.data = p.cast::(); + crate::source::Tty::close(p, Self::on_tty_close); + } + self.flags.insert(WindowsFlags::IS_PAUSED); + } + #[cfg(windows)] + Source::StdinTty(tty) => { + // Stays open for the VM's next stdin reader. + let p = tty.as_ptr(); + // SAFETY: the VM's shared tty outlives this reader. + unsafe { + if (*p).uv.data == core::ptr::from_mut(self).cast::() { + uv::uv_read_stop(p.cast()); + (*p).uv.data = core::ptr::null_mut(); } } - self.flags.insert(WindowsFlags::IS_PAUSED); } #[cfg(not(windows))] @@ -1845,12 +1853,10 @@ impl WindowsBufferedReader { extern "C" fn on_tty_close(handle: *mut uv::uv_tty_t) { // `close_impl` set `handle.data = handle` and called `uv_close(handle)`; // libuv passes the same pointer back; `Tty::from_uv` recovers the - // owning `Tty`. Caller gates on `!is_stdin_tty`, so it is heap-owned, - // and no request is pending once this runs (`uv::Tty::read_scratch`). - let tty = crate::source::Tty::from_uv(handle); - debug_assert!(!crate::source::stdin_tty::is_stdin_tty(tty)); - // SAFETY: non-stdin tty is heap-allocated; sole owner after uv_close. - drop(unsafe { bun_core::heap::take(tty) }); + // owning `Tty`. Only a `Source::Tty` (heap, `open_tty`) is closed this + // way, and no request is pending once this runs (`uv::Tty::read_scratch`). + // SAFETY: heap-owned; sole owner after uv_close. + drop(unsafe { bun_core::heap::take(crate::source::Tty::from_uv(handle)) }); } fn on_read(&mut self, amount: sys::Result, slice: &mut [u8], has_more: ReadState) { diff --git a/src/io/PipeWriter.rs b/src/io/PipeWriter.rs index d11dddf016c0..e149ae2fa67c 100644 --- a/src/io/PipeWriter.rs +++ b/src/io/PipeWriter.rs @@ -1186,13 +1186,14 @@ pub trait BaseWindowsPipeWriter: Sized { } Source::Tty(tty) => { let p = tty.as_ptr(); - // SAFETY: tty is heap-allocated (via open_tty) or the - // process-static stdin tty; freed in on_tty_close (gated on is_stdin_tty). + // SAFETY: tty is heap-allocated (open_tty); freed in on_tty_close. unsafe { (*p).uv.data = p.cast::() }; // SAFETY: tty is a live uv handle; `Tty::close` keeps // whole-struct provenance so on_tty_close may reclaim the Box. unsafe { crate::source::Tty::close(p, on_tty_close) }; } + // Not produced for a writer (`start` opens without a reader context). + Source::StdinTty(_) => {} } *self.source_mut() = None; self.on_close_source(); @@ -1271,7 +1272,7 @@ pub trait BaseWindowsPipeWriter: Sized { // This is critical for spawnSync to use its isolated loop // SAFETY: parent is BACKREF set via set_parent; valid while writer alive. let loop_ = unsafe { Self::Parent::loop_(self.parent_ptr()) }; - let mut source = match Source::open(loop_, fd) { + let mut source = match Source::open(loop_, fd, None) { sys::Result::Ok(source) => source, sys::Result::Err(err) => return sys::Result::Err(err), }; @@ -1324,12 +1325,9 @@ extern "C" fn on_pipe_close(handle: *mut uv::Pipe) { extern "C" fn on_tty_close(handle: *mut uv::uv_tty_t) { // `close()` set `handle.data = handle` and then called `uv_close(handle)`; // libuv passes the same pointer back; `Tty::from_uv` recovers the owning - // `Tty`. The stdin tty (fd 0) lives in static storage; never free it. - let tty = crate::source::Tty::from_uv(handle); - if !crate::source::stdin_tty::is_stdin_tty(tty) { - // SAFETY: non-stdin tty is heap-allocated (open_tty). - drop(unsafe { bun_core::heap::take(tty) }); - } + // `Tty`. Only a `Source::Tty` (heap, `open_tty`) is closed this way. + // SAFETY: heap-owned; sole owner after uv_close. + drop(unsafe { bun_core::heap::take(crate::source::Tty::from_uv(handle)) }); } /// Common parent requirements for Windows writers (event loop access + ref counting). diff --git a/src/io/lib.rs b/src/io/lib.rs index c9954cd44b2f..7476582735f1 100644 --- a/src/io/lib.rs +++ b/src/io/lib.rs @@ -327,6 +327,8 @@ bun_dispatch::link_interface! { ctx: Option>, ); fn pipe_read_scratch() -> *const PipeReadScratch; + // Null when the context has none (mini loop, POSIX). + fn stdin_tty() -> *mut StdinTty; } } @@ -471,6 +473,11 @@ pub use pipe_read_scratch::{PipeReadScratch, PipeReadScratchGuard}; #[cfg(windows)] #[path = "source.rs"] pub mod source; +#[cfg(windows)] +pub use source::StdinTty; +/// Never exists on POSIX: [`EventLoopCtx::stdin_tty`] returns null there. +#[cfg(not(windows))] +pub enum StdinTty {} #[path = "write.rs"] pub mod write; diff --git a/src/io/source.rs b/src/io/source.rs index 0399001dc57f..4e759a0468bb 100644 --- a/src/io/source.rs +++ b/src/io/source.rs @@ -1,6 +1,5 @@ use core::ffi::{c_int, c_void}; -use core::mem::MaybeUninit; -use core::sync::atomic::{AtomicBool, Ordering}; +use core::ptr::NonNull; use bun_sys::windows::libuv as uv; // `is_closed`/`is_active`/`fd` are default trait methods on `UvHandle`; @@ -17,14 +16,10 @@ pub use uv::Tty; pub enum Source { Pipe(Box), - /// `BackRef` not `Box`: the stdin tty (fd 0) lives in static storage - /// (`stdin_tty::value()`), and Box-from-static is UB. Heap-allocated ttys - /// use `heap::alloc`; destroy paths gate `heap::take` on `!is_stdin_tty()`. - /// In both cases the `Tty` strictly outlives every `Source` that holds it - /// (process-static, or freed only by the libuv close callback after the - /// `Source` is dropped), so the `BackRef` invariant holds and `Deref` - /// yields `&Tty` without a per-site `unsafe`. + /// From `open_tty`; the close callback frees it once the source closes it. Tty(bun_ptr::BackRef), + /// Borrowed from the VM's [`StdinTty`], which outlives it; never closed here. + StdinTty(bun_ptr::BackRef), File(Box), SyncFile(Box), } @@ -240,23 +235,21 @@ impl File { } impl Source { - /// Exclusive borrow of the `Tty` arm. `BackRef` already gives safe `Deref` - /// for shared reads; mutation still needs the per-site exclusivity - /// guarantee (single-threaded uv loop, no other `&Tty` live), so this - /// remains the one centralised `unsafe` for tty mutation. + /// The one centralised `unsafe` for tty mutation. #[inline] fn tty_mut(tty: &mut bun_ptr::BackRef) -> &mut Tty { - // SAFETY: `BackRef` invariant guarantees liveness/alignment; the uv - // loop is single-threaded and `&mut Source` (or the sole `BackRef` - // returned from `open_tty`) is the only access path, so no `&Tty` - // overlaps this `&mut Tty`. + // SAFETY: the pointee is live (`BackRef` invariant) and every holder + // borrows it only within one call on the single loop thread, so no + // other `&Tty` overlaps this `&mut Tty`. unsafe { tty.get_mut() } } /// For a tty source, hand libuv the handle-owned read buffer with at /// least `size` spare bytes (`uv::Tty::read_scratch`); `None` otherwise. pub(crate) fn tty_read_scratch(&mut self, size: usize) -> Option<&mut [u8]> { - let Source::Tty(tty) = self else { return None }; + let (Source::Tty(tty) | Source::StdinTty(tty)) = self else { + return None; + }; let scratch = &mut Self::tty_mut(tty).read_scratch; scratch.clear(); scratch.reserve(size); @@ -268,7 +261,7 @@ impl Source { pub fn is_closed(&self) -> bool { match self { Source::Pipe(pipe) => pipe.is_closed(), - Source::Tty(tty) => tty.uv.is_closed(), + Source::Tty(tty) | Source::StdinTty(tty) => tty.uv.is_closed(), Source::SyncFile(file) | Source::File(file) => file.file == -1, } } @@ -276,7 +269,7 @@ impl Source { pub(crate) fn is_active(&self) -> bool { match self { Source::Pipe(pipe) => pipe.is_active(), - Source::Tty(tty) => tty.uv.is_active(), + Source::Tty(tty) | Source::StdinTty(tty) => tty.uv.is_active(), Source::SyncFile(_) | Source::File(_) => true, } } @@ -287,7 +280,7 @@ impl Source { // uv_stream_t as their first member. // `&mut self` so the returned `*mut` carries write provenance. Source::Pipe(pipe) => core::ptr::from_mut::(pipe.as_mut()).cast(), - Source::Tty(tty) => tty.as_ptr().cast(), + Source::Tty(tty) | Source::StdinTty(tty) => tty.as_ptr().cast(), Source::SyncFile(_) | Source::File(_) => unreachable!(), } } @@ -298,7 +291,7 @@ impl Source { // Windows); tag kind=system so callers can round-trip through // `Fd::native()`. Source::Pipe(pipe) => Fd::from_system(pipe.fd()), - Source::Tty(tty) => Fd::from_system(tty.uv.fd()), + Source::Tty(tty) | Source::StdinTty(tty) => Fd::from_system(tty.uv.fd()), Source::SyncFile(file) | Source::File(file) => Fd::from_uv(file.file), } } @@ -306,7 +299,7 @@ impl Source { pub fn set_data(&mut self, data: *mut c_void) { match self { Source::Pipe(pipe) => pipe.data = data, - Source::Tty(tty) => Self::tty_mut(tty).uv.data = data, + Source::Tty(tty) | Source::StdinTty(tty) => Self::tty_mut(tty).uv.data = data, Source::SyncFile(file) | Source::File(file) => file.fs.data = data, } } @@ -331,6 +324,7 @@ impl Source { Source::Tty(tty) => { uv::open_handles::set_owner(tty.as_ptr().cast(), owner, Some(close_via_owner)) } + Source::StdinTty(_) => {} Source::SyncFile(file) | Source::File(file) => uv::open_handles::set_file_owner( core::ptr::from_mut::(file).cast(), owner, @@ -352,7 +346,7 @@ impl Source { pub fn ref_(&mut self) { match self { Source::Pipe(pipe) => pipe.ref_(), - Source::Tty(tty) => Self::tty_mut(tty).uv.ref_(), + Source::Tty(tty) | Source::StdinTty(tty) => Self::tty_mut(tty).uv.ref_(), Source::SyncFile(_) | Source::File(_) => {} } } @@ -360,7 +354,7 @@ impl Source { pub fn unref(&mut self) { match self { Source::Pipe(pipe) => pipe.unref(), - Source::Tty(tty) => Self::tty_mut(tty).uv.unref(), + Source::Tty(tty) | Source::StdinTty(tty) => Self::tty_mut(tty).uv.unref(), Source::SyncFile(_) | Source::File(_) => {} } } @@ -386,24 +380,19 @@ impl Source { bun_sys::Result::Ok(pipe) } + /// A tty of the caller's own; the shared stdin one is [`StdinTty::open`]. pub(crate) fn open_tty( loop_: *mut uv::Loop, fd: Fd, ) -> bun_sys::Result> { bun_core::scoped_log!(PipeSource, "openTTY (fd = {})", fd); - let uv_fd = fd.uv(); - - if uv_fd == 0 { - return stdin_tty::get_stdin_tty(loop_); - } - // Not `boxed_zeroed`: a zeroed `Vec` is UB. let mut tty: Box = Box::new(Tty { uv: bun_core::ffi::zeroed(), read_scratch: Vec::new(), }); - if let Some(err) = tty.init(loop_, uv_fd).to_error(bun_sys::Tag::open) { + if let Some(err) = tty.init(loop_, fd.uv()).to_error(bun_sys::Tag::open) { drop(tty); return bun_sys::Result::Err(err); } @@ -426,7 +415,12 @@ impl Source { file } - pub(crate) fn open(loop_: *mut uv::Loop, fd: Fd) -> bun_sys::Result { + /// With a reader's context, a tty on fd 0 is that context's [`StdinTty`]. + pub(crate) fn open( + loop_: *mut uv::Loop, + fd: Fd, + reader_ctx: Option, + ) -> bun_sys::Result { let rc = uv::uv_guess_handle(fd.uv()); bun_core::scoped_log!( PipeSource, @@ -440,10 +434,18 @@ impl Source { bun_sys::Result::Ok(pipe) => bun_sys::Result::Ok(Source::Pipe(pipe)), bun_sys::Result::Err(err) => bun_sys::Result::Err(err), }, - uv::HandleType::Tty => match Self::open_tty(loop_, fd) { - bun_sys::Result::Ok(tty) => bun_sys::Result::Ok(Source::Tty(tty)), - bun_sys::Result::Err(err) => bun_sys::Result::Err(err), - }, + uv::HandleType::Tty => { + if fd.uv() == 0 { + if let Some(shared) = reader_ctx.and_then(|ctx| NonNull::new(ctx.stdin_tty())) { + // SAFETY: points into the context's live rare data, on + // its own thread; `open` does not re-enter. + return unsafe { &mut *shared.as_ptr() } + .open(loop_) + .map(Source::StdinTty); + } + } + Self::open_tty(loop_, fd).map(Source::Tty) + } uv::HandleType::File => bun_sys::Result::Ok(Source::File(Self::open_file(fd))), _ => { let errno = bun_sys::windows::get_last_errno(); @@ -468,7 +470,7 @@ impl Source { pub(crate) fn set_raw_mode(&mut self, value: bool) -> bun_sys::Result<()> { match self { - Source::Tty(tty) => { + Source::Tty(tty) | Source::StdinTty(tty) => { if let Some(err) = Self::tty_mut(tty) .uv .set_mode(if value { @@ -493,61 +495,79 @@ impl Source { } } -pub(crate) mod stdin_tty { - use super::*; - - // PORTING.md §Global mutable state: init guarded by `LOCK` + `INITIALIZED`; - // afterwards only accessed by uv on the loop thread. RacyCell. - static DATA: bun_core::RacyCell> = - bun_core::RacyCell::new(MaybeUninit::uninit()); - static LOCK: bun_threading::Mutex = bun_threading::Mutex::new(); - static INITIALIZED: AtomicBool = AtomicBool::new(false); - - #[inline] - fn value() -> *mut Tty { - DATA.get().cast::() - } - - pub(crate) fn is_stdin_tty(tty: *const Tty) -> bool { - core::ptr::eq(tty, value()) - } +/// A VM's one tty over fd 0, shared so that `setRawMode` restarts the pending read. +#[derive(Default)] +pub struct StdinTty { + /// Heap-allocated so that readers can hold `BackRef`s to it. + tty: Option>, +} - pub(super) fn get_stdin_tty( +impl StdinTty { + pub(crate) fn open( + &mut self, loop_: *mut uv::Loop, ) -> bun_sys::Result> { - // bun_threading::Mutex::lock() returns `()` — must use lock_guard() for RAII - // unlock-on-drop, otherwise the mutex is held forever and the next call - // (e.g. Source__setRawModeStdin → open_tty(stdin)) deadlocks/UB-relocks. - let _guard = LOCK.lock_guard(); - - if !INITIALIZED.swap(true, Ordering::Relaxed) { - let p = value(); - // SAFETY: value() points to static storage sized for Tty; lock - // held. uv_tty_init fills the `uv` half; `read_scratch` starts - // empty via a raw write (the storage is uninit, so a plain - // assignment would drop garbage). - let rc = unsafe { uv::uv_tty_init(loop_, core::ptr::addr_of_mut!((*p).uv), 0, 0) }; - if let Some(err) = rc.to_error(bun_sys::Tag::open) { - INITIALIZED.store(false, Ordering::Relaxed); - return bun_sys::Result::Err(err); + let tty = match self.tty { + // Closed by the VM's teardown: stdin is gone for this VM. + // SAFETY: owned by `self`; `is_closing` only reads the flags. + Some(tty) if unsafe { (*tty.as_ptr()).uv.is_closing() } => { + return bun_sys::Result::Err(bun_sys::Error::from_code( + bun_sys::E::BADF, + bun_sys::Tag::open, + )); } - // SAFETY: as above; lock held, first initialization. - unsafe { core::ptr::addr_of_mut!((*p).read_scratch).write(Vec::new()) }; - } + Some(tty) => tty, + None => { + bun_core::scoped_log!(PipeSource, "openTTY (fd = 0, shared)"); + // Not `boxed_zeroed`: a zeroed `Vec` is UB. + let mut tty: Box = Box::new(Tty { + uv: bun_core::ffi::zeroed(), + read_scratch: Vec::new(), + }); + // Whole-struct pointer as in `Tty::init`, which would also list it as heap. + let uv_ptr = core::ptr::from_mut(&mut *tty).cast::(); + // SAFETY: `uv` is the first `#[repr(C)]` field, sized for uv_tty_t. + let rc = unsafe { uv::uv_tty_init(loop_, uv_ptr, 0, 0) }; + if let Some(err) = rc.to_error(bun_sys::Tag::open) { + return bun_sys::Result::Err(err); + } + let tty = bun_core::heap::into_raw_nn(tty); + uv::open_handles::add_stdin_tty(tty.as_ptr().cast::()); + self.tty = Some(tty); + tty + } + }; + // SAFETY: owned by `self`, which outlives every holder; write + // provenance from `into_raw_nn`. + bun_sys::Result::Ok(unsafe { bun_ptr::BackRef::from_raw_mut(tty.as_ptr()) }) + } +} - // Destroy path must gate `heap::take` on `!is_stdin_tty(ptr)`. - // SAFETY: `value()` is the process-global static tty (never null, - // never freed); uv writes through it. - bun_sys::Result::Ok(unsafe { bun_ptr::BackRef::from_raw_mut(value()) }) +impl Drop for StdinTty { + fn drop(&mut self) { + let Some(tty) = self.tty.take() else { + return; + }; + // SAFETY: leaked from a `Box` in `open`, freed nowhere else. A handle + // nobody closed (the exiting main thread) is still linked into its + // loop, so it is leaked rather than freed under libuv. + unsafe { + if (*tty.as_ptr()).uv.is_closed() { + drop(bun_core::heap::take(tty.as_ptr())); + } + } } } -/// The uv loop is taken as a parameter (reading it from the VM directly would -/// be a T6 dependency); the C++ caller -/// (`ProcessBindingTTYWrap.cpp`) supplies `defaultGlobalObject()->uvLoop()`. +/// `jsTTYSetMode` (`ProcessBindingTTYWrap.cpp`): the calling VM's [`StdinTty`]. #[unsafe(no_mangle)] extern "C" fn Source__setRawModeStdin(uv_loop: *mut uv::Loop, raw: bool) -> c_int { - let mut tty = match Source::open_tty(uv_loop, Fd::stdin()) { + let Some(shared) = NonNull::new(crate::js_vm_ctx().stdin_tty()) else { + return bun_sys::E::NOTSUP as c_int; + }; + // SAFETY: points into the calling VM's live rare data; JS thread, and + // `open` does not re-enter. + let mut tty = match unsafe { &mut *shared.as_ptr() }.open(uv_loop) { bun_sys::Result::Ok(tty) => tty, bun_sys::Result::Err(e) => return e.errno as c_int, }; @@ -557,9 +577,6 @@ extern "C" fn Source__setRawModeStdin(uv_loop: *mut uv::Loop, raw: bool) -> c_in // closely with POSIX platforms. This is also required to support some // control sequences at all on Windows, such as bracketed paste mode. The // Node.js readline implementation handles differences between these modes. - // `tty` is the static stdin tty (fd 0 → `get_stdin_tty`), live for the - // process — same invariant the `Source::Tty` arm relies on, so reuse the - // shared `tty_mut` accessor. if let Some(err) = Source::tty_mut(&mut tty) .uv .set_mode(if raw { diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index cbba475684ec..babd2cd5b7b6 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -2293,6 +2293,16 @@ bun_io::link_impl_EventLoopCtx! { vm.after_event_loop_callback_ctx = ctx.map(|p| p.as_ptr()); }, pipe_read_scratch() => &raw const *(*vm_from_owner(this.cast()).rare_data_ptr()).pipe_read_scratch, + stdin_tty() => { + #[cfg(windows)] + { + &raw mut (*vm_from_owner(this.cast()).rare_data_ptr()).stdin_tty + } + #[cfg(not(windows))] + { + core::ptr::null_mut() + } + }, } } diff --git a/src/jsc/rare_data.rs b/src/jsc/rare_data.rs index 2c53d2a772ab..6dc286b5297e 100644 --- a/src/jsc/rare_data.rs +++ b/src/jsc/rare_data.rs @@ -205,6 +205,9 @@ pub struct RareData { pub(crate) stdin_mode: Mode, pub stdout_store: Option>, pub(crate) stdout_mode: Mode, + /// Handed out through `EventLoopCtx::stdin_tty`. + #[cfg(windows)] + pub(crate) stdin_tty: Async::StdinTty, pub(crate) entropy_cache: Option>, @@ -303,6 +306,8 @@ impl Default for RareData { stdin_mode: 0, stdout_store: None, stdout_mode: 0, + #[cfg(windows)] + stdin_tty: Async::StdinTty::default(), entropy_cache: None, hot_map: None, cron_jobs: Vec::new(), @@ -1065,7 +1070,7 @@ impl Drop for RareData { fn drop(&mut self) { // pipe_read_scratch / h2_padded_frame_buffer / spawn_sync_event_loop_ / // s3_default_client / default_csrf_secret / cleanup_hooks / cron_jobs / - // path_buf / tls_default_ciphers: + // path_buf / tls_default_ciphers / stdin_tty: // all dropped automatically via field Drop. if let Some(engine) = self.boring_ssl_engine.take() { diff --git a/src/libuv_sys/libuv.rs b/src/libuv_sys/libuv.rs index 9be9b1a33915..0dd4a7d0fec2 100644 --- a/src/libuv_sys/libuv.rs +++ b/src/libuv_sys/libuv.rs @@ -385,7 +385,7 @@ thread_local! { // ────────────────────────────────────────────────────────────────────────── // Open stream/process handles on this thread — Bun's HandleWrap list. // -// Every `uv_pipe_t`, `uv_tty_t` (except the process-static stdin tty) and +// Every `uv_pipe_t`, `uv_tty_t` (the shared stdin one via `add_stdin_tty`) and // `uv_process_t` this thread initialises is listed here from `init`/`spawn` // until the `uv_close` for it is issued (`UvHandle::close`, // `Pipe::close_and_destroy`). Whoever currently drives the handle records @@ -1363,8 +1363,7 @@ pub struct Tty { /// `alloc_cb` buffer, and a read cancelled by `uv_read_stop` is never /// handed back through `read_cb` — so the buffer must be owned by /// something libuv guarantees outlives the read. The handle is that - /// thing: its close callback only runs once no requests are pending, - /// and the process-static stdin tty is never closed at all. + /// thing: its close callback only runs once no requests are pending. pub read_scratch: Vec, } @@ -1384,9 +1383,7 @@ impl Tty { let uv_ptr = core::ptr::from_mut(self).cast::(); // SAFETY: `uv` is the first `#[repr(C)]` field, sized for uv_tty_t. let rc = unsafe { uv_tty_init(loop_, uv_ptr, file, 0) }; - // fd 0 is the process-static stdin tty (never freed, shared across - // threads by design); everything else is a heap tty owned by this thread. - if rc.0 == 0 && file != 0 { + if rc.0 == 0 { open_handles::add_tty(uv_ptr); } rc diff --git a/src/libuv_sys/open_handles.rs b/src/libuv_sys/open_handles.rs index b12c5472c5af..7dabc53a38da 100644 --- a/src/libuv_sys/open_handles.rs +++ b/src/libuv_sys/open_handles.rs @@ -36,6 +36,8 @@ struct Entry { enum Kind { Pipe, Tty, + /// `bun_io::StdinTty`: never owned here; closed in place, freed by its owner. + StdinTty, Process, } @@ -77,6 +79,9 @@ pub(super) fn add_pipe(p: *mut Pipe) { pub(super) fn add_tty(t: *mut uv_tty_t) { add(t.cast(), Kind::Tty); } +pub fn add_stdin_tty(t: *mut uv_tty_t) { + add(t.cast(), Kind::StdinTty); +} pub(super) fn add_process(p: *mut Process) { add(p.cast(), Kind::Process); } @@ -120,10 +125,13 @@ pub fn set_file_owner(file: *mut c_void, owner: *mut c_void, close: CloseViaOwne /// `owner` now drives `handle` and closes it via `close(owner)`; pass a /// null `owner` to clear. No-op for handles not listed (never initialised -/// on this thread, already closing, or the process-static stdin tty). +/// on this thread, already closing) and for the shared stdin tty. pub fn set_owner(handle: *mut uv_handle_t, owner: *mut c_void, close: Option) { OPEN.with(|o| { if let Some(e) = o.borrow_mut().handles.get_mut(&handle) { + if e.kind == Kind::StdinTty { + return; + } e.owner = owner; e.close_via_owner = if owner.is_null() { None } else { close }; } @@ -192,6 +200,7 @@ pub fn stop_all_for_vm_teardown() { match e.kind { Kind::Pipe => "pipe", Kind::Tty => "tty", + Kind::StdinTty => "stdin tty", Kind::Process => "process", }, handle, @@ -207,8 +216,8 @@ pub fn stop_all_for_vm_teardown() { // SAFETY: as above (a leaked Box nobody adopted). (None, Kind::Tty) => unsafe { unsafe extern "C" fn free_tty(t: *mut uv_tty_t) { - // SAFETY: heap `Box` (stdin's static tty is never - // listed); `from_uv` recovers the owning box. + // SAFETY: heap `Box` (the shared stdin tty is + // `Kind::StdinTty`); `from_uv` recovers the owning box. drop(unsafe { Box::from_raw(super::Tty::from_uv(t)) }); } uv_close( @@ -221,8 +230,9 @@ pub fn stop_all_for_vm_teardown() { }, // A process handle is embedded in its owner and always adopted at // spawn; an unowned one cannot be freed safely — close in place. + // The stdin tty too: its owner frees it once the loop is closed. // SAFETY: listed ⇒ an initialised, not-closing handle on this loop. - (None, Kind::Process) => unsafe { uv_close(handle, None) }, + (None, Kind::StdinTty | Kind::Process) => unsafe { uv_close(handle, None) }, } } } diff --git a/src/runtime/webcore/FileSink.rs b/src/runtime/webcore/FileSink.rs index 98e2b28526bc..bc9735146844 100644 --- a/src/runtime/webcore/FileSink.rs +++ b/src/runtime/webcore/FileSink.rs @@ -262,9 +262,9 @@ pub(crate) extern "C" fn Bun__ForceFileSinkToBeSynchronousForProcessObjectStdio( } } bun_io::Source::Tty(tty) => { - // SAFETY: `tty` is a live `BackRef` (heap or static stdin tty); - // `Tty` (via its first field, `uv::uv_tty_t`) embeds `uv_stream_t` as - // its first member, so the cast is the libuv handle-subtype downcast. + // SAFETY: `tty` is a live heap `BackRef`; `Tty` (via its first + // field, `uv::uv_tty_t`) embeds `uv_stream_t` as its first member, so + // the cast is the libuv handle-subtype downcast. let rc = unsafe { uv::uv_stream_set_blocking(tty.as_ptr().cast::(), 1) }; diff --git a/test/js/node/tty.test.ts b/test/js/node/tty.test.ts index 3eca2321a0fa..60b912ab8932 100644 --- a/test/js/node/tty.test.ts +++ b/test/js/node/tty.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it, test } from "bun:test"; -import { bunEnv, bunExe, isWindows } from "harness"; +import { bunEnv, bunExe, isWindows, tempDir } from "harness"; import { WriteStream } from "node:tty"; describe("ReadStream.prototype.setRawMode", () => { @@ -190,6 +190,119 @@ describe("ReadStream.prototype.setRawMode", () => { }); expect(await proc.exited).toBe(0); }); + + // Windows keeps one uv_tty_t for fd 0 per VM (readers and setRawMode share + // it). It used to be one per process, on the loop of the first thread to touch + // stdin, so a Worker's exit closed it under the main thread (EBADF, EINVAL). + test("stdin still works on the main thread after a Worker used it first", async () => { + using dir = tempDir("tty-stdin-after-worker", { + "worker.js": ` + const seen = {}; + process.stdin.on("error", e => (seen.err = String(e))); + seen.isTTY = process.stdin.isTTY; + process.stdin.setRawMode(true); + seen.rawOn = process.stdin.isRaw; + process.stdin.setRawMode(false); + seen.rawOff = process.stdin.isRaw; + // Leaves a read pending on the worker's handle when it is terminated. + process.stdin.once("data", d => { + seen.line = String(d).trim(); + postMessage(seen); + }); + console.log("WORKER_READING"); + `, + "main.js": ` + // The parent test reads the report and then kills this process, so + // nothing here exits: an exit could race the report out of the pty. + const report = result => console.log("RESULT_BEGIN" + JSON.stringify(result) + "RESULT_END"); + const worker = new Worker(new URL("./worker.js", import.meta.url).href); + worker.onerror = e => report({ workerError: String(e.message) }); + worker.onmessage = async ({ data: fromWorker }) => { + await worker.terminate(); + const main = {}; + let reading = false; + process.stdin.on("error", e => { + main.err = String(e); + if (reading) report({ worker: fromWorker, main }); + }); + main.isTTY = process.stdin.isTTY; + process.stdin.setRawMode(true); + main.rawOn = process.stdin.isRaw; + process.stdin.setRawMode(false); + main.rawOff = process.stdin.isRaw; + reading = true; + process.stdin.once("data", d => { + main.line = String(d).trim(); + report({ worker: fromWorker, main }); + }); + console.log("MAIN_READING"); + }; + `, + }); + + const decoder = new TextDecoder(); + // The terminal wraps long lines and moves the cursor around; match markers + // against the whole output with the escapes and line breaks removed (an + // escape sequence can be split across chunks, so strip the whole thing). + let raw = ""; + let text = ""; + const waiters: { marker: string; resolve: () => void }[] = []; + const proc = Bun.spawn({ + cmd: [bunExe(), "main.js"], + env: bunEnv, + cwd: String(dir), + terminal: { + cols: 200, + rows: 24, + data(_t, chunk: Uint8Array) { + raw += decoder.decode(chunk, { stream: true }); + text = Bun.stripANSI(raw).replace(/[\r\n]/g, ""); + for (let i = waiters.length - 1; i >= 0; i--) { + if (text.includes(waiters[i].marker) || text.includes("RESULT_END")) { + waiters[i].resolve(); + waiters.splice(i, 1); + } + } + }, + }, + }); + const terminal = proc.terminal!; + + const exited = proc.exited.then(code => { + throw new Error(`child exited with code ${code} before it reported; terminal output: ${JSON.stringify(text)}`); + }); + exited.catch(() => {}); + // Resolves on the marker, or on an early report (a failure the assertion + // below then shows); rejects if the child dies instead. + const phase = (marker: string) => + Promise.race([ + text.includes(marker) || text.includes("RESULT_END") + ? Promise.resolve() + : new Promise(resolve => waiters.push({ marker, resolve })), + exited, + ]); + + try { + await phase("WORKER_READING"); + terminal.write("from-worker\r"); + await phase("MAIN_READING"); + terminal.write("from-main\r"); + await phase("RESULT_END"); + } finally { + proc.kill(); + await proc.exited; + terminal.close(); + } + + const match = text.match(/RESULT_BEGIN(.*?)RESULT_END/); + if (!match) { + throw new Error("child did not report; terminal output was: " + JSON.stringify(text)); + } + expect(JSON.parse(match[1])).toEqual({ + worker: { isTTY: true, rawOn: true, rawOff: false, line: "from-worker" }, + main: { isTTY: true, rawOn: true, rawOff: false, line: "from-main" }, + }); + }); }); describe("WriteStream.prototype.getColorDepth", () => {