diff --git a/changelog.d/9676-stdin-unref-ref-keeps-reader.md b/changelog.d/9676-stdin-unref-ref-keeps-reader.md new file mode 100644 index 0000000000..5eeef9e7a6 --- /dev/null +++ b/changelog.d/9676-stdin-unref-ref-keeps-reader.md @@ -0,0 +1,32 @@ +### Fixed + +- A TUI no longer goes permanently deaf to the keyboard after `process.stdin` + is `unref()`d and `ref()`d again (#9676). On the stdin *object* path — an + alias, a parameter, or a destructured field, which is what ink and every TUI + built on it use — `unref` was wired to the same detach stub as + `pause`/`destroy`: it set a process-global latch, and the runtime's fd-0 + reader thread breaks its loop on that latch and exits. `ref` was wired to a + no-op stub, so nothing ever cleared the latch or restarted the reader. One + `unref()`/`ref()` pair therefore left the process with no reader on fd 0 for + the rest of its life: the event loop kept ticking, the terminal stayed in raw + mode, the process still woke on each keystroke, and not one further byte + reached JS. Ink performs exactly that pair whenever its raw-mode refcount + drops to zero and comes back — i.e. whenever the last `useInput` component + unmounts and a new one mounts, which is what a tool call does — so this is + the long-standing "TUI input dies after a minute of real use" symptom. + `ref`/`unref` now govern only the event-loop hold, as in Node: an unref'd + stdin keeps delivering `'data'`, and `ref()` restores the hold. Only an + explicit `pause()`/`destroy()` stops the reader, and `resume()` still clears + both. + +- The `process.stdin` object's `pause()`/`resume()` now reach the same flow + state as codegen's literal `process.stdin.pause()`/`.resume()` spelling + (#9676). `rl.close()` and a literal `pause()` set perry-stdlib readline's + `STDIN_PAUSED`, whose pump branch deliberately leaves `PENDING_DATA` + undrained — while readline's fd-0 reader keeps reading and keeps waking the + main thread. Only the literal `process.stdin.resume()` could clear that flag, + so a TUI that holds stdin in a variable (`const s = process.stdin; … + s.resume()`) and opens a single readline prompt went permanently deaf: bytes + still consumed off the terminal, CPU still burnt on every keystroke, nothing + ever dispatched to JS. The two spellings are now bridged the same way `on` + and `off` already were. diff --git a/crates/perry-runtime/src/os_process_streams.rs b/crates/perry-runtime/src/os_process_streams.rs index 76056db6c4..3e68b6f4d3 100644 --- a/crates/perry-runtime/src/os_process_streams.rs +++ b/crates/perry-runtime/src/os_process_streams.rs @@ -135,28 +135,92 @@ extern "C" fn process_stream_set_encoding_stub( crate::object::js_implicit_this_get() } -/// #3962: set when a TUI tears down stdin via `process.stdin.destroy()`, -/// `.pause()`, or `.unref()`. `perry-stdlib`'s readline `has_active` consults +/// #3962: set when a TUI tears down stdin via `process.stdin.destroy()` or +/// `.pause()`. `perry-stdlib`'s readline `has_active` consults /// `stdin_is_detached()` so the runtime stops holding the event loop open for /// the stdin reader, letting the process quiesce after teardown without an /// explicit `process.exit()`. +/// +/// #9676: this used to cover `.unref()` too, and that was the TUI-input-death +/// bug. `unref()` set this latch, the fd-0 reader below breaks its loop on it +/// and EXITS — and `ref()` was wired to a no-op stub, so nothing ever cleared +/// the latch or restarted the reader. One `unref()`/`ref()` pair (ink performs +/// exactly that pair every time its raw-mode refcount drops to zero and comes +/// back, i.e. whenever the last `useInput` component unmounts and a new one +/// mounts around a tool call) therefore left the process with NO reader on fd 0 +/// for the rest of its life: the terminal stayed in raw mode, the loop kept +/// ticking, and not one further keystroke ever reached JS. static STDIN_DETACHED: std::sync::atomic::AtomicBool = std::sync::atomic::AtomicBool::new(false); -/// True once `process.stdin` has been detached (`destroy`/`pause`/`unref`). +/// #9676: set by `process.stdin.unref()`, cleared by `.ref()`. +/// +/// Node's `ref`/`unref` govern ONLY whether the handle keeps the event loop +/// alive — an unref'd stdin still delivers data. So this flag feeds the +/// liveness view (`stdin_is_detached`) but NOT the reader loop, which keeps +/// reading. That separation is what makes the pair symmetric: `ref()` restores +/// the hold, and no keystroke is lost in between. +static STDIN_UNREFED: std::sync::atomic::AtomicBool = std::sync::atomic::AtomicBool::new(false); + +/// True once `process.stdin` no longer holds the event loop open — either it +/// was detached (`destroy`/`pause`) or it was `unref()`d. This is the LIVENESS +/// view; the fd-0 reader uses `stdin_reader_should_stop()` instead, which +/// deliberately ignores `unref`. pub fn stdin_is_detached() -> bool { + STDIN_DETACHED.load(std::sync::atomic::Ordering::Acquire) + || STDIN_UNREFED.load(std::sync::atomic::Ordering::Acquire) +} + +/// Whether the fd-0 reader thread should stop. `unref()` must NOT stop it +/// (#9676) — only an explicit `destroy()`/`pause()` does. +fn stdin_reader_should_stop() -> bool { STDIN_DETACHED.load(std::sync::atomic::Ordering::Acquire) } -/// `destroy`/`pause`/`unref` impl for `process.stdin` — releases the stdin -/// reader's hold on the event loop. No-op return (`undefined`). +/// `destroy`/`pause` impl for `process.stdin` — releases the stdin reader's +/// hold on the event loop and stops the reader. No-op return (`undefined`). extern "C" fn process_stdin_detach_stub( _closure: *const crate::closure::ClosureHeader, _arg: f64, ) -> f64 { STDIN_DETACHED.store(true, std::sync::atomic::Ordering::Release); + // #9676: mirror it into readline's flow state, so `pause()` means the same + // thing whichever spelling reached it (and so the `resume()` below is a + // true inverse rather than a partial one). + if let Some(pause) = stdin_flow_op(&STDIN_FLOW_PAUSE_FN) { + pause(); + } f64::from_bits(crate::value::TAG_UNDEFINED) } +/// `process.stdin.unref()` — drop the event-loop hold WITHOUT stopping +/// delivery (#9676). Node's contract: an unref'd stdin still emits `'data'`. +extern "C" fn process_stdin_unref_stub( + _closure: *const crate::closure::ClosureHeader, + _arg: f64, +) -> f64 { + STDIN_UNREFED.store(true, std::sync::atomic::Ordering::Release); + crate::object::js_implicit_this_get() +} + +/// `process.stdin.ref()` — restore the event-loop hold (#9676). Was a no-op +/// stub, which is what made `unref()` a one-way latch. +/// +/// Deliberately does NOT start a reader. `ref` is the inverse of `unref` and +/// nothing more, exactly as in Node: it does not resume a `pause()`d stream, +/// and starting one here would be actively harmful — perry-stdlib's readline +/// runs its own fd-0 reader, both readers take `std::io::stdin()`'s process +/// lock, and a program whose listeners live in readline's registry would end +/// up with the runtime's reader parked on that lock (or, worse, consuming +/// bytes into a buffer those listeners never see). `resume()` remains the one +/// call that restarts a stopped reader. +extern "C" fn process_stdin_ref_stub( + _closure: *const crate::closure::ClosureHeader, + _arg: f64, +) -> f64 { + STDIN_UNREFED.store(false, std::sync::atomic::Ordering::Release); + crate::object::js_implicit_this_get() +} + thread_local! { static STDIN_STREAM_SINGLETON: RefCell = const { RefCell::new(0) }; static STDOUT_STREAM_SINGLETON: RefCell = const { RefCell::new(0) }; @@ -253,7 +317,10 @@ fn ensure_stdin_reader() { // while a burst collapses into one lock + one notify. let mut buf = [0u8; 4096]; loop { - if stdin_is_detached() { + // #9676: `stdin_reader_should_stop`, NOT `stdin_is_detached` — + // an `unref()`d stdin still delivers data in Node, and reading + // the liveness view here is what killed the reader for good. + if stdin_reader_should_stop() { break; } match handle.read(&mut buf) { @@ -455,6 +522,40 @@ pub extern "C" fn js_register_stdin_listener_ops( STDIN_OFF_FN.store(off as *mut (), std::sync::atomic::Ordering::Release); } +/// #9676: perry-stdlib's readline `pause`/`resume`, so the stdin OBJECT's +/// `pause()`/`resume()` reach the same flow state as codegen's literal +/// `process.stdin.pause()` / `.resume()`. +/// +/// Without this bridge the two spellings latched DIFFERENT flags. `rl.close()` +/// and a literal `process.stdin.pause()` both set readline's `STDIN_PAUSED`, +/// whose pump branch returns without draining `PENDING_DATA` — while readline's +/// fd-0 reader keeps reading and keeps notifying the main thread. Recovering +/// with an ALIASED `stdin.resume()` (`const s = process.stdin; s.resume()`, and +/// every TUI that holds stdin in a variable) landed on the runtime object stub, +/// which cleared only the runtime's own flags and left `STDIN_PAUSED` set for +/// the life of the process. The result is exactly the reported wedge: bytes are +/// consumed off the terminal, the process wakes and burns CPU on every +/// keystroke, and nothing is ever dispatched to JS. +static STDIN_FLOW_PAUSE_FN: std::sync::atomic::AtomicPtr<()> = + std::sync::atomic::AtomicPtr::new(std::ptr::null_mut()); +static STDIN_FLOW_RESUME_FN: std::sync::atomic::AtomicPtr<()> = + std::sync::atomic::AtomicPtr::new(std::ptr::null_mut()); + +#[no_mangle] +pub extern "C" fn js_register_stdin_flow_ops(pause: extern "C" fn(), resume: extern "C" fn()) { + STDIN_FLOW_PAUSE_FN.store(pause as *mut (), std::sync::atomic::Ordering::Release); + STDIN_FLOW_RESUME_FN.store(resume as *mut (), std::sync::atomic::Ordering::Release); +} + +fn stdin_flow_op(slot: &std::sync::atomic::AtomicPtr<()>) -> Option { + let p = slot.load(std::sync::atomic::Ordering::Acquire); + if p.is_null() { + return None; + } + // SAFETY: `js_register_stdin_flow_ops` only ever stores this exact ABI. + Some(unsafe { std::mem::transmute::<*mut (), extern "C" fn()>(p) }) +} + /// True when readline owns the stdin listener registry (it always does once /// perry-stdlib is linked). fn stdin_ops_provider() -> Option<( @@ -810,12 +911,22 @@ extern "C" fn process_stdin_read(_closure: *const crate::closure::ClosureHeader, } /// `process.stdin.resume()` — flowing mode. Clears any prior detach (from -/// `pause`/`unref`) and (re)starts the reader, so a paused stdin can resume. +/// `pause`/`destroy`) and any prior `unref()`, and (re)starts the reader, so a +/// paused stdin can resume. extern "C" fn process_stdin_resume( _closure: *const crate::closure::ClosureHeader, _arg: f64, ) -> f64 { STDIN_DETACHED.store(false, std::sync::atomic::Ordering::Release); + STDIN_UNREFED.store(false, std::sync::atomic::Ordering::Release); + // #9676: clear readline's `STDIN_PAUSED` too. `rl.close()` and a literal + // `process.stdin.pause()` set it, its pump branch stops draining + // `PENDING_DATA`, and before this bridge only the LITERAL + // `process.stdin.resume()` could clear it — an aliased `s.resume()` left + // stdin permanently deaf while the reader kept consuming bytes. + if let Some(resume) = stdin_flow_op(&STDIN_FLOW_RESUME_FN) { + resume(); + } ensure_stdin_reader(); crate::object::js_implicit_this_get() } @@ -1254,9 +1365,18 @@ fn build_stream_object_with_write( process_stream_on_once_stub }, ); // resume - set_field_with_stub(start + 6, lifecycle); // unref + // #9676: on stdin, `unref`/`ref` are a SYMMETRIC pair that only moves + // the event-loop hold; on stdout/stderr `unref` stays the shared no-op. + set_field_with_stub( + start + 6, + if is_stdin { + process_stdin_unref_stub + } else { + process_stream_on_once_stub + }, + ); // unref if is_stdin { - set_field_with_stub(start + 7, process_stream_on_once_stub); // ref + set_field_with_stub(start + 7, process_stdin_ref_stub); // ref set_field_with_stub(start + 8, lifecycle); // destroy if is_stdin { let se = diff --git a/crates/perry-stdlib/src/readline/mod.rs b/crates/perry-stdlib/src/readline/mod.rs index 8fa5ab39ea..17ae09accd 100644 --- a/crates/perry-stdlib/src/readline/mod.rs +++ b/crates/perry-stdlib/src/readline/mod.rs @@ -360,6 +360,28 @@ extern "C" fn stdin_listeners_provider(name_ptr: *const u8, name_len: usize) -> f64::from_bits(JSValue::array_ptr(arr).bits()) } +/// `stdin.pause()` reached as an OBJECT method (an aliased binding). Bridged so +/// it latches the SAME `STDIN_PAUSED` flag as codegen's literal +/// `process.stdin.pause()` extern (#9676). +extern "C" fn stdin_pause_op() { + STDIN_PAUSED.store(true, Ordering::Release); +} + +/// `stdin.resume()` reached as an OBJECT method. This is the half that was +/// missing: `rl.close()` and the literal `process.stdin.pause()` both set +/// `STDIN_PAUSED`, and the pump's paused branch then leaves `PENDING_DATA` +/// undrained while the reader keeps consuming bytes off the terminal. Only the +/// literal `process.stdin.resume()` could clear it, so a TUI that holds stdin +/// in a variable (`const s = process.stdin; ... s.resume()`) went permanently +/// deaf — bytes consumed, CPU burnt on every keystroke, nothing dispatched. +extern "C" fn stdin_resume_op() { + if !STDIN_DESTROYED.load(Ordering::Acquire) { + STDIN_PAUSED.store(false, Ordering::Release); + try_register_pump(); + ensure_reader_started(); + } +} + /// `stdin.addListener/on(event, cb)` reached as an OBJECT method (an aliased /// binding, e.g. `const {stdin} = props; stdin.addListener("readable", h)`). /// Registered with the runtime so both that form and codegen's direct @@ -491,10 +513,13 @@ fn ensure_stdin_listeners_provider_registered() { on: extern "C" fn(*const u8, usize, i64, i32), off: extern "C" fn(*const u8, usize, i64), ); + // #9676: the flow half of the same bridge — see `stdin_pause_op`. + fn js_register_stdin_flow_ops(pause: extern "C" fn(), resume: extern "C" fn()); } unsafe { js_register_stdin_listeners_provider(stdin_listeners_provider); js_register_stdin_listener_ops(stdin_on_op, stdin_off_op); + js_register_stdin_flow_ops(stdin_pause_op, stdin_resume_op); } }); } diff --git a/crates/perry/tests/issue_9676_stdin_unref_ref_keeps_reader.rs b/crates/perry/tests/issue_9676_stdin_unref_ref_keeps_reader.rs new file mode 100644 index 0000000000..94b4f3fdea --- /dev/null +++ b/crates/perry/tests/issue_9676_stdin_unref_ref_keeps_reader.rs @@ -0,0 +1,305 @@ +//! Regression test for #9676: "TUI input dies after real use". +//! +//! THE DEFECT. On the `process.stdin` OBJECT path — an alias, a parameter, or a +//! destructured field, which is what ink and every TUI built on it use — +//! `unref` was wired to the same `process_stdin_detach_stub` as `pause` and +//! `destroy`. That stub sets a process-global `STDIN_DETACHED` latch, and the +//! runtime's fd-0 reader thread breaks its loop on that latch and EXITS. `ref` +//! was wired to a no-op stub, so nothing ever cleared the latch or restarted +//! the reader. +//! +//! One `unref()`/`ref()` pair therefore left the process with **no reader on fd +//! 0 for the rest of its life**: the event loop kept ticking, the terminal +//! stayed in raw mode, the process still woke on each keystroke — and not one +//! further byte ever reached JS. Ink performs exactly that pair every time its +//! raw-mode refcount drops to zero and comes back, i.e. whenever the last +//! `useInput` component unmounts and a new one mounts. A tool call does that. +//! Hence "input dies after a minute of real use", with the operation that +//! preceded it completing and rendering normally. +//! +//! Node's contract, which this test pins: `ref`/`unref` govern ONLY whether the +//! handle keeps the event loop alive. An unref'd stdin still emits `'data'`. +//! +//! WHY A PTY, AND WHY THIS IS THE ONLY SHAPE THAT CAN FAIL. On a pipe the bug +//! is invisible: perry-stdlib's readline reader owns fd 0 there and never +//! consults the runtime latch, so a pipe-based fixture passes both before and +//! after the fix. The runtime's own reader — the one the latch kills — is the +//! live reader only on a TTY. A test that cannot fail is not a test, so this +//! one runs the child on a real PTY. +//! +//! THE SECOND DEFECT, same family. `rl.close()` (and a literal +//! `process.stdin.pause()`) set perry-stdlib readline's `STDIN_PAUSED`, whose +//! pump branch deliberately leaves `PENDING_DATA` undrained — while readline's +//! fd-0 reader keeps reading and keeps waking the main thread. Only the LITERAL +//! `process.stdin.resume()` spelling cleared that flag; an ALIASED +//! `s.resume()` reached the runtime's object stub, which cleared the runtime's +//! own flags and nothing else. So a TUI that holds stdin in a variable and +//! opens one readline prompt went permanently deaf, with bytes still being +//! consumed off the terminal and CPU still burnt on every keystroke — which is +//! the signature the issue actually recorded. +//! +//! CONTROL. The `none` mode drives the identical keystroke stream with no +//! lifecycle calls at all and must deliver every byte. It passed before the fix +//! too, which is what makes the other two cases' failures attributable to the +//! cycle rather than to the harness, the PTY, or the timing. +//! +//! Measured on `origin/main` (17d00b28e4) before the fix, over a PTY: 2 of 157 +//! keystrokes delivered for the unref cycle, 1 of 157 for rl.close + aliased +//! resume, 157 of 157 for the control. + +#![cfg(unix)] + +use std::fs::File; +use std::io::{BufRead, BufReader, Write}; +use std::os::fd::{FromRawFd, RawFd}; +use std::os::unix::process::CommandExt; +use std::path::{Path, PathBuf}; +use std::process::{Child, Command, Stdio}; +use std::sync::mpsc::{self, Receiver, RecvTimeoutError}; +use std::time::Duration; + +/// The child models a TUI's stdin wiring: raw mode, an anonymous handler +/// reached through an ALIASED binding, and a periodic lifecycle cycle chosen by +/// `PERRY_9676_MODE`: +/// +/// `none` — control, no lifecycle calls at all. +/// `unref` — ink's raw-mode refcount pair, `unref()` then `ref()`. +/// `rlclose` — a readline prompt opened and closed (which pauses stdin, as in +/// Node) and then recovered with an aliased `resume()`. +const SOURCE: &str = r#" +import * as readline from "node:readline"; + +const s: any = process.stdin; +let rx = 0; +s.setEncoding("utf8"); +s.setRawMode(true); +s.addListener("data", (chunk: any) => { + for (const ch of String(chunk)) { + rx++; + console.log("RX:" + rx + ":" + ch); + } +}); +const mode = process.env.PERRY_9676_MODE ?? "none"; +// Nothing else in this program touches stdin, so a keystroke that goes missing +// after a cycle went missing because of it. +setInterval(() => { + if (mode === "unref") { + s.unref(); + s.ref(); + } else if (mode === "rlclose") { + const rl = readline.createInterface({ + input: process.stdin, + output: process.stdout, + terminal: false, + }); + rl.close(); + s.resume(); + } +}, 40); +console.log("READY"); +"#; + +fn perry_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_perry")) +} + +fn compile(dir: &Path) -> PathBuf { + let entry = dir.join("main.ts"); + let output = dir.join("main_bin"); + std::fs::write(&entry, SOURCE).expect("write PTY fixture"); + let compile = Command::new(perry_bin()) + .current_dir(dir) + .arg("compile") + .arg(&entry) + .arg("-o") + .arg(&output) + .output() + .expect("run perry compile"); + assert!( + compile.status.success(), + "perry compile failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&compile.stdout), + String::from_utf8_lossy(&compile.stderr) + ); + output +} + +fn open_pty() -> (File, File) { + let mut master: RawFd = -1; + let mut slave: RawFd = -1; + let rc = unsafe { + // `null_mut()` for all three: macOS types the trailing termios/winsize + // params `*mut`, Linux `*const`, and `*mut` coerces to `*const`. + libc::openpty( + &mut master, + &mut slave, + std::ptr::null_mut(), + std::ptr::null_mut(), + std::ptr::null_mut(), + ) + }; + assert_eq!(rc, 0, "openpty failed: {}", std::io::Error::last_os_error()); + assert!(master >= 0 && slave >= 0); + // SAFETY: openpty returned two fresh, owned descriptors above. + unsafe { (File::from_raw_fd(master), File::from_raw_fd(slave)) } +} + +struct PtyChild { + child: Child, + input: File, + lines: Receiver, +} + +impl PtyChild { + fn spawn(program: &Path, mode: &str) -> Self { + let (master, slave) = open_pty(); + let child_stdin = slave.try_clone().expect("clone PTY slave for stdin"); + let child_stdout = slave.try_clone().expect("clone PTY slave for stdout"); + let mut command = Command::new(program); + command + .stdin(Stdio::from(child_stdin)) + .stdout(Stdio::from(child_stdout)) + .stderr(Stdio::null()) + .env("PERRY_9676_MODE", mode); + // Give the child its own session and make fd 0's PTY its controlling + // terminal. The stdio descriptors are already installed when this runs. + unsafe { + command.pre_exec(|| { + if libc::setsid() == -1 { + return Err(std::io::Error::last_os_error()); + } + if libc::ioctl(0, libc::TIOCSCTTY as _, 0) == -1 { + return Err(std::io::Error::last_os_error()); + } + Ok(()) + }); + } + let child = command.spawn().expect("spawn PTY child"); + drop(slave); + + let input = master.try_clone().expect("clone PTY master for writes"); + let (tx, lines) = mpsc::channel(); + std::thread::spawn(move || { + for line in BufReader::new(master).lines() { + match line { + Ok(line) => { + if tx.send(line.trim_end_matches('\r').to_string()).is_err() { + break; + } + } + // Linux returns EIO from a PTY master after the slave closes. + Err(_) => break, + } + } + }); + + let mut session = Self { + child, + input, + lines, + }; + let ready = session + .recv_line(Duration::from_secs(30)) + .expect("PTY child never printed READY"); + assert_eq!(ready, "READY", "unexpected first PTY line"); + session + } + + fn send(&mut self, bytes: &[u8]) { + self.input.write_all(bytes).expect("write PTY input"); + self.input.flush().expect("flush PTY input"); + } + + fn recv_line(&mut self, timeout: Duration) -> Result { + self.lines.recv_timeout(timeout) + } + + /// Send `letters` one keystroke at a time, waiting for each echo before the + /// next. Returns what actually came back. + fn type_and_collect(&mut self, letters: &str) -> String { + let mut got = String::new(); + for ch in letters.chars() { + self.send(ch.to_string().as_bytes()); + let deadline = std::time::Instant::now() + Duration::from_millis(1500); + loop { + let remaining = deadline.saturating_duration_since(std::time::Instant::now()); + if remaining.is_zero() { + // Nothing arrived for this keystroke — input is dead. Stop + // here rather than burning the rest of the budget; the + // assertion below reports how far we got. + return got; + } + match self.recv_line(remaining) { + Ok(line) => { + if let Some(rest) = line.strip_prefix("RX:") { + if let Some((_, c)) = rest.split_once(':') { + got.push_str(c); + break; + } + } + } + Err(_) => return got, + } + } + } + got + } +} + +impl Drop for PtyChild { + fn drop(&mut self) { + let _ = self.child.kill(); + let _ = self.child.wait(); + } +} + +/// Long enough that the 40 ms cycle interval fires many times mid-stream, so a +/// single surviving keystroke after the first cycle cannot pass by luck. +const LETTERS: &str = "abcdefghijklmnopqrstuvwxyz"; + +#[test] +fn stdin_lifecycle_cycles_keep_delivering_keystrokes() { + let dir = tempfile::tempdir().expect("create fixture directory"); + let program = compile(dir.path()); + + // CONTROL: no lifecycle calls at all. This half has always passed; it is + // here so a failure of the other two cannot be blamed on the PTY, the + // harness, or timing. + let mut control = PtyChild::spawn(&program, "none"); + std::thread::sleep(Duration::from_millis(150)); + let control_got = control.type_and_collect(LETTERS); + assert_eq!( + control_got, LETTERS, + "control (no lifecycle calls) lost keystrokes: got {control_got:?} — the \ + harness itself is broken, not the behaviour under test" + ); + drop(control); + + // GAP 1: `unref()` was the same one-way detach latch as `pause()`/`destroy()` + // and `ref()` was a no-op stub, so the fd-0 reader exited for good. + let mut unref = PtyChild::spawn(&program, "unref"); + std::thread::sleep(Duration::from_millis(150)); + let unref_got = unref.type_and_collect(LETTERS); + assert_eq!( + unref_got, LETTERS, + "stdin stopped delivering after an unref()/ref() cycle: got {unref_got:?} of \ + {LETTERS:?}. `unref()` must not stop the fd-0 reader and `ref()` must restore \ + the event-loop hold (#9676)" + ); + drop(unref); + + // GAP 2: `rl.close()` pauses stdin through perry-stdlib's readline + // `STDIN_PAUSED`, whose pump branch stops draining `PENDING_DATA` while the + // reader keeps consuming bytes. Only the LITERAL `process.stdin.resume()` + // cleared that flag; an aliased `s.resume()` reached the runtime object stub + // and left stdin permanently deaf. + let mut rlclose = PtyChild::spawn(&program, "rlclose"); + std::thread::sleep(Duration::from_millis(150)); + let rlclose_got = rlclose.type_and_collect(LETTERS); + assert_eq!( + rlclose_got, LETTERS, + "stdin stopped delivering after rl.close() + an aliased resume(): got \ + {rlclose_got:?} of {LETTERS:?}. The stdin object's pause()/resume() must reach \ + the same flow state as codegen's literal process.stdin spelling (#9676)" + ); +} diff --git a/test-files/test_gap_9676_stdin_unref_ref_keeps_reader.ts b/test-files/test_gap_9676_stdin_unref_ref_keeps_reader.ts new file mode 100644 index 0000000000..2a11c08adc --- /dev/null +++ b/test-files/test_gap_9676_stdin_unref_ref_keeps_reader.ts @@ -0,0 +1,151 @@ +// #9676: `process.stdin.unref()` must not kill stdin delivery, and `.ref()` +// must undo it. +// +// THE BUG. On the stdin *object* path (an alias/parameter/field — which is what +// ink and every TUI built on it use), perry wired `unref` to the same +// `process_stdin_detach_stub` as `pause`/`destroy`: it set a process-global +// `STDIN_DETACHED` latch, and the fd-0 reader thread breaks its loop on that +// latch and EXITS. `ref` was wired to a no-op stub, so nothing ever cleared the +// latch or restarted the reader. ONE `unref()`/`ref()` pair therefore left the +// process with no reader on fd 0 for the rest of its life — the loop kept +// ticking, the terminal stayed in raw mode, and not one further keystroke +// reached JS. That is the "TUI input dies after a minute of real use" symptom: +// ink performs exactly that pair every time its raw-mode refcount drops to zero +// and comes back, i.e. whenever the last `useInput` component unmounts and a +// new one mounts — which is what a tool call does. +// +// Node's contract, which the roles below pin: `ref`/`unref` govern ONLY whether +// the handle keeps the event loop alive. An unref'd stdin still emits `'data'`. +// +// LOAD-BEARING CONSTRUCTION: +// +// * Each role acts on the FIRST chunk and asserts on a SECOND chunk written +// afterwards. A role that only ever saw one chunk cannot pass by accident, +// and the toggle happens strictly between the two. +// * Every role prints a `phase1` line before it toggles, so "the listener was +// never registered" and "the listener died at the toggle" are different +// outputs rather than the same silence. +// * The `unref-*` roles hold the loop open with a short interval — that is the +// POINT of `unref` (it drops stdin's own hold) and without it the process is +// allowed to exit, which would make the test measure loop liveness instead +// of byte delivery. +// * `churn` runs across the toggle in one role so a future regression that +// reintroduces the defect through a collected/relocated listener is caught +// by the same fixture. +// * `pause-resume` is a CONTROL: it must keep working, and it is the one +// lifecycle pair that legitimately DOES stop the reader. +import { spawn } from "node:child_process"; + +const ROLE_ENV = "PERRY_9676_ROLE"; +const WATCHDOG_MS = 20000; +const role = process.env[ROLE_ENV] ?? ""; + +function finish(line: string): void { + console.log(line); + process.exit(0); +} + +// Escaping allocation: cells survive into the old generation and are dropped a +// few blocks later, so this forces real collections rather than a nursery flip. +function churn(rounds: number): number { + let sink = 0; + let keep: any[] = []; + const held: any[] = []; + for (let i = 0; i < rounds; i++) { + const cell = { a: i, b: i + 1, c: "s" + (i & 1023), d: [i, i + 1] }; + keep.push(cell); + if (keep.length >= 1024) { + sink += keep[0].b; + if ((i & 15) === 0) held.push(keep); + if (held.length > 24) held.shift(); + keep = []; + } + } + return sink + held.length; +} + +function runRole(name: string, onFirst: (s: any) => void, doChurn: boolean): void { + const s: any = process.stdin; + // `unref()` releases stdin's hold on the loop by design, so hold it here. + const ticker = setInterval(() => {}, 20); + let phase = 0; + s.on("data", (chunk: any) => { + const text = String(chunk); + if (phase === 0 && text.indexOf("ONE") >= 0) { + phase = 1; + console.log(name + " phase1: true"); + onFirst(s); + if (doChurn) console.log(name + " churn: " + (churn(300000) > 0)); + } else if (phase === 1 && text.indexOf("TWO") >= 0) { + clearInterval(ticker); + finish(name + " phase2: true"); + } + }); +} + +if (role === "unref-ref") { + runRole("unref-ref", (s) => { + s.unref(); + s.ref(); + }, false); +} else if (role === "unref-ref-churn") { + runRole("unref-ref-churn", (s) => { + s.unref(); + s.ref(); + }, true); +} else if (role === "unref-only") { + // Node: an unref'd stdin still delivers. Only the loop hold is dropped. + runRole("unref-only", (s) => { + s.unref(); + }, false); +} else if (role === "pause-resume") { + runRole("pause-resume", (s) => { + s.pause(); + s.resume(); + }, false); +} else { + const roles = ["unref-ref", "unref-ref-churn", "unref-only", "pause-resume"]; + const childArgs = [...process.execArgv, ...process.argv.slice(1)]; + const run = (name: string) => + new Promise((resolve) => { + const child = spawn(process.execPath, childArgs, { + env: { ...process.env, [ROLE_ENV]: name }, + stdio: ["pipe", "inherit", "inherit"], + }); + let settled = false; + const watchdog = setTimeout(() => { + if (settled) return; + settled = true; + console.log(name + " exit: WATCHDOG"); + child.kill("SIGKILL"); + resolve(); + }, WATCHDOG_MS); + child.on("exit", (code) => { + if (settled) return; + settled = true; + clearTimeout(watchdog); + console.log(name + " exit:", code); + resolve(); + }); + setTimeout(() => { + try { + child.stdin!.write("ONE\n"); + } catch { + /* child already gone */ + } + }, 120); + // Late enough that the churn role has finished collecting first. + setTimeout(() => { + try { + child.stdin!.write("TWO\n"); + } catch { + /* child already gone */ + } + }, 2500); + }); + + (async () => { + for (const r of roles) await run(r); + console.log("done"); + })(); +}