From e02ed262cdd5a945f2770060bb2a05c072a6597c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 4 Sep 2026 08:33:40 +0200 Subject: [PATCH] fix(runtime): stop process.stdin lifecycle calls from killing input for good (#9676) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "TUI input dies after real use", root-caused to two asymmetries 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. Both leave input permanently dead while the process stays alive, the loop keeps ticking and the terminal stays in raw mode. 1. `unref()` was wired to `process_stdin_detach_stub`, the same stub as `pause`/`destroy`: it sets the 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 left the process with no reader on fd 0 for the rest of its life. Ink runs 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 (confirmed verbatim in cc's own bundle). Split the latch: `STDIN_DETACHED` (destroy/pause) stops the reader and drops the loop hold; `STDIN_UNREFED` (unref/ref) drops only the loop hold, matching Node, where an unref'd stdin still emits 'data'. `stdin_is_detached()` is now the liveness view (either flag); the reader consults `stdin_reader_should_stop()` (detach only). `ref()` is a pure inverse and deliberately starts no reader — `resume()` remains the one call that does. 2. `rl.close()` and a literal `process.stdin.pause()` set perry-stdlib readline's `STDIN_PAUSED`, whose pump branch returns without draining `PENDING_DATA` — while readline's fd-0 reader keeps reading and keeps waking the main thread. Only the LITERAL `process.stdin.resume()` spelling could clear it; an aliased `s.resume()` landed on the runtime's object stub, which cleared only the runtime's own flags. A TUI holding stdin in a variable that opens one readline prompt therefore went permanently deaf with bytes still consumed off the terminal and CPU still burnt per keystroke — the exact signature the issue recorded. Bridge `pause`/`resume` through a registered op pair, as `on`/`off` already were. Regression test drives the child over a real PTY. That is load-bearing: on a pipe perry-stdlib's readline reader owns fd 0 and never consults these flags, so a pipe fixture passes before AND after the fix. Measured on 17d00b28e4 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 no-lifecycle control. --- .../9676-stdin-unref-ref-keeps-reader.md | 32 ++ .../perry-runtime/src/os_process_streams.rs | 138 +++++++- crates/perry-stdlib/src/readline/mod.rs | 25 ++ ...issue_9676_stdin_unref_ref_keeps_reader.rs | 305 ++++++++++++++++++ ...t_gap_9676_stdin_unref_ref_keeps_reader.ts | 151 +++++++++ 5 files changed, 642 insertions(+), 9 deletions(-) create mode 100644 changelog.d/9676-stdin-unref-ref-keeps-reader.md create mode 100644 crates/perry/tests/issue_9676_stdin_unref_ref_keeps_reader.rs create mode 100644 test-files/test_gap_9676_stdin_unref_ref_keeps_reader.ts 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"); + })(); +}