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/changelog.d/9686-async-yield-star-dispatch.md b/changelog.d/9686-async-yield-star-dispatch.md new file mode 100644 index 0000000000..68713337d7 --- /dev/null +++ b/changelog.d/9686-async-yield-star-dispatch.md @@ -0,0 +1,13 @@ +**Async generators with many `yield*` sites no longer make the compiler appear +to hang while consuming tens of gigabytes.** The `.throw()` lowering cloned the +entire state dispatcher into every delegation route, making transformed HIR +quadratic in the number of delegation sites and multiplying LLVM safepoints and +relocations. Delegation routing now selects one mutually exclusive branch and +falls through to a single shared dispatcher. On the real-world 32-site function +that exposed the bug, the generated async-step closure shrank from 33.6 MB to +2.5 MB of debug-rendered HIR (92.5%). + +The fix preserves `.throw()` behavior across multiple delegated generators and +still routes delegation-protocol failures through the outer generator's +`try`/`catch`; both paths are covered by a Node-parity fixture. A structural +transform test also rejects future super-linear dispatcher growth. diff --git a/changelog.d/9688-proxy-array-callbacks.md b/changelog.d/9688-proxy-array-callbacks.md new file mode 100644 index 0000000000..727270b538 --- /dev/null +++ b/changelog.d/9688-proxy-array-callbacks.md @@ -0,0 +1,6 @@ +Array iteration methods and sorting now accept callable `Proxy` callbacks, +matching Node for proxy-wrapped functions and bound functions. + +Comparator validation no longer treats a Proxy registry handle as a closure +pointer, preventing a user-triggerable crash. Non-callable callbacks and +comparators continue to throw Node-compatible `TypeError`s. diff --git a/changelog.d/9689-aliased-native-class-heritage.md b/changelog.d/9689-aliased-native-class-heritage.md new file mode 100644 index 0000000000..1fe49bd370 --- /dev/null +++ b/changelog.d/9689-aliased-native-class-heritage.md @@ -0,0 +1,6 @@ +**Native stream subclasses keep their derived methods when constructor imports +are aliased or minified.** Class heritage now resolves native import bindings to +their original exports before selecting the subclass initializer. This keeps +readdirp's `_read` implementation and the complete EventEmitter method surface +intact, allowing Claude Code's chokidar watcher to initialize without the +`once is not a function` startup errors reported in #9680. diff --git a/crates/perry-hir/src/lower/tests.rs b/crates/perry-hir/src/lower/tests.rs index 7c755d7702..54f2be4089 100644 --- a/crates/perry-hir/src/lower/tests.rs +++ b/crates/perry-hir/src/lower/tests.rs @@ -1884,6 +1884,50 @@ fn bun_transpiler_and_build_lower_to_native_dispatch() { ); } +/// #9680: native constructors retain their exported identity when a bundler +/// minifies the local import name used by a class declaration or expression. +#[test] +fn aliased_native_imports_canonicalize_class_heritage() { + let source = r#" + import { Readable as ut } from "node:stream"; + import { EventEmitter as jt } from "node:events"; + + class ReaddirpStream extends ut { + _read() {} + } + const Watcher = class extends jt {}; + "#; + let module = perry_parser::parse_typescript(source, "watcher.js").expect("source parses"); + let hir = super::lower_module(&module, "watcher", "watcher.js").expect("source lowers"); + + let stream = hir + .classes + .iter() + .find(|class| class.name == "ReaddirpStream") + .expect("stream class is lowered"); + assert_eq!(stream.extends_name.as_deref(), Some("Readable")); + assert_eq!( + stream.native_extends, + Some(("node_stream".to_string(), "Readable".to_string())) + ); + assert!( + stream.extends_expr.is_none(), + "aliased native heritage must not use replacement-object construction" + ); + + let watcher = hir + .classes + .iter() + .find(|class| class.name == "Watcher") + .expect("inferred-name class expression is lowered"); + assert_eq!(watcher.extends_name.as_deref(), Some("EventEmitter")); + assert_eq!( + watcher.native_extends, + Some(("events".to_string(), "EventEmitter".to_string())) + ); + assert!(watcher.extends_expr.is_none()); +} + /// #8882: a module-level class constructing a sibling class that is declared /// inside a function body lowered LATER. This is the shape the CJS wrap /// produces for Next's `server/lib/lru-cache.js`: `LRUCache` is hoisted out of @@ -1933,42 +1977,7 @@ fn hoisted_class_constructs_sibling_declared_inside_a_later_closure() { ); } -/// #8882 / #8730: a constructor name that resolves to nothing in the module -/// is read off `globalThis` when the `new` executes — exactly like a bare -/// identifier read — so a runtime-created global constructs and a true miss -/// throws `ReferenceError: is not defined` WITH the identifier. The -/// `typeof`-guarded browser-API shape is the one Next's `app-page` runtime -/// carries; it previously lowered to the nameless throw even though the guard -/// makes the branch dead on a server. -#[test] -fn unresolved_new_names_the_identifier_and_defers_to_a_runtime_global_lookup() { - let source = r#" - function observe(cb: any): any { - return typeof IntersectionObserver === "function" - ? new IntersectionObserver(cb) - : null; - } - "#; - let module = perry_parser::parse_typescript(source, "t.ts").expect("source parses"); - let hir = super::lower_module(&module, "t", "t.ts").expect("source lowers"); - let observe = hir - .functions - .iter() - .find(|function| function.name == "observe") - .expect("observe is lowered"); - let debug = format!("{observe:?}"); - - assert!( - !debug.contains("js_throw_reference_error_unresolved_get"), - "the nameless ReferenceError helper must not be emitted for `new ()`:\n{debug}" - ); - assert!( - debug.contains( - r#"NewDynamic { callee: Call { callee: ExternFuncRef { name: "js_global_get_or_throw_unresolved", param_types: [Any], return_type: Any }, args: [String("IntersectionObserver")]"# - ), - "an unresolved constructor must be a runtime globalThis lookup carrying its name:\n{debug}" - ); -} +mod unresolved_new_global; mod capture_stash; mod mixin_parent_chain; diff --git a/crates/perry-hir/src/lower/tests/unresolved_new_global.rs b/crates/perry-hir/src/lower/tests/unresolved_new_global.rs new file mode 100644 index 0000000000..cd76619759 --- /dev/null +++ b/crates/perry-hir/src/lower/tests/unresolved_new_global.rs @@ -0,0 +1,39 @@ +//! #8882/#8730: an unresolved `new` names the identifier and defers to a +//! runtime global lookup. Split from `tests.rs` for the 2000-line cap. + +/// #8882 / #8730: a constructor name that resolves to nothing in the module +/// is read off `globalThis` when the `new` executes — exactly like a bare +/// identifier read — so a runtime-created global constructs and a true miss +/// throws `ReferenceError: is not defined` WITH the identifier. The +/// `typeof`-guarded browser-API shape is the one Next's `app-page` runtime +/// carries; it previously lowered to the nameless throw even though the guard +/// makes the branch dead on a server. +#[test] +fn unresolved_new_names_the_identifier_and_defers_to_a_runtime_global_lookup() { + let source = r#" + function observe(cb: any): any { + return typeof IntersectionObserver === "function" + ? new IntersectionObserver(cb) + : null; + } + "#; + let module = perry_parser::parse_typescript(source, "t.ts").expect("source parses"); + let hir = super::lower_module(&module, "t", "t.ts").expect("source lowers"); + let observe = hir + .functions + .iter() + .find(|function| function.name == "observe") + .expect("observe is lowered"); + let debug = format!("{observe:?}"); + + assert!( + !debug.contains("js_throw_reference_error_unresolved_get"), + "the nameless ReferenceError helper must not be emitted for `new ()`:\n{debug}" + ); + assert!( + debug.contains( + r#"NewDynamic { callee: Call { callee: ExternFuncRef { name: "js_global_get_or_throw_unresolved", param_types: [Any], return_type: Any }, args: [String("IntersectionObserver")]"# + ), + "an unresolved constructor must be a runtime globalThis lookup carrying its name:\n{debug}" + ); +} diff --git a/crates/perry-hir/src/lower_decl/class_decl.rs b/crates/perry-hir/src/lower_decl/class_decl.rs index d7d0080aa8..e824e5ee00 100644 --- a/crates/perry-hir/src/lower_decl/class_decl.rs +++ b/crates/perry-hir/src/lower_decl/class_decl.rs @@ -7,37 +7,35 @@ use crate::lower::{lower_expr, LoweringContext}; use crate::lower_patterns::*; use crate::lower_types::*; -/// The four classic node:stream base-class names (`Readable`/`Writable`/ -/// `Duplex`/`Transform`). When a class extends a parent with one of these -/// textual names, perry routes `super()` to the native `js_node_stream_*` -/// shim (which installs the native stream surface but never sets the -/// `_readableState`/`_writableState`/`_transformState` objects). That is only -/// correct when the name actually resolves to the `node:stream` builtin — -/// i.e. it was imported from `stream`/`node:stream`, in which case the import -/// machinery registered it as the `stream` native module -/// (`register_native_module(binding, "stream", Some(name))`, see -/// `var_decl_sources::register_destructured_stream_ctors` and the import -/// arms in `lower/module_decl.rs`). -/// -/// The same textual name can instead be a userland binding from a -/// stream-shim npm package — `const { Transform } = -/// require('readable-stream')` in winston's `logger.js`, where -/// `class Logger extends Transform` then reads `this._readableState.pipes` -/// directly. Such a binding never registers as the `stream` native module -/// (readable-stream is not a node builtin), so the package's real constructor -/// must run instead. Returning `false` here lets the unknown-Ident arm -/// capture the parent as `extends_expr` and route `super()` through the -/// dynamic-parent path (`js_register_class_parent_dynamic` + the -/// `js_fetch_or_value_super` dispatch), which runs the real `Transform` -/// function body on the subclass instance. +/// Recover the exported constructor name behind a minified native import used +/// as class heritage (`import { Readable as ut }; class R extends ut`). Native +/// imports are registered under the local binding while preserving this export. +fn canonical_native_parent_name<'a>(ctx: &'a LoweringContext, name: &str) -> Option<&'a str> { + match ctx.lookup_native_module(name) { + Some(("stream", Some(class @ ("Readable" | "Writable" | "Duplex" | "Transform")))) + | Some(("events", Some(class @ ("EventEmitter" | "EventEmitterAsyncResource")))) + | Some(("async_hooks", Some(class @ ("AsyncLocalStorage" | "AsyncResource")))) + | Some(("ws", Some(class @ "WebSocketServer"))) + | Some(( + "stream/web" | "node:stream/web", + Some(class @ ("ReadableStream" | "WritableStream" | "TransformStream")), + )) => Some(class), + _ => None, + } +} + +/// Only genuine `node:stream` bindings use Perry's native subclass shims. A +/// userland binding from `readable-stream` must keep the dynamic parent path so +/// its real constructor runs. Inspecting the preserved export also supports a +/// minified local binding such as `Readable as ut`. fn is_genuine_node_stream_parent(ctx: &LoweringContext, name: &str) -> bool { - if !matches!(name, "Readable" | "Writable" | "Duplex" | "Transform") { - return false; + match ctx.lookup_native_module(name) { + Some(("stream", Some("Readable" | "Writable" | "Duplex" | "Transform"))) => true, + // Preserve the historical name-based treatment of a namespace/default + // binding whose local name itself is a classic stream constructor. + Some(("stream", None)) => matches!(name, "Readable" | "Writable" | "Duplex" | "Transform"), + _ => false, } - // Only the genuine `node:stream` builtin import registers these names as - // the `stream` native module. Anything else (a userland require/import of - // a stream-shim package, or an unbound reference) is not the builtin. - matches!(ctx.lookup_native_module(name), Some(("stream", _))) } mod class_heritage; @@ -331,8 +329,11 @@ pub fn lower_class_decl( ) } else if let ast::Expr::Ident(ident) = super_class.as_ref() { let parent_name = ident.sym.to_string(); + let canonical_parent_name = canonical_native_parent_name(ctx, &parent_name) + .unwrap_or(&parent_name) + .to_string(); // First check if it's a native module class - let native_parent = match parent_name.as_str() { + let native_parent = match canonical_parent_name.as_str() { "EventEmitter" => Some(("events".to_string(), "EventEmitter".to_string())), "EventEmitterAsyncResource" => Some(( "events".to_string(), @@ -379,7 +380,7 @@ pub fn lower_class_decl( "Readable" | "Writable" | "Duplex" | "Transform" if is_genuine_node_stream_parent(ctx, &parent_name) => { - Some(("node_stream".to_string(), parent_name.clone())) + Some(("node_stream".to_string(), canonical_parent_name.clone())) } _ => None, }; @@ -399,7 +400,7 @@ pub fn lower_class_decl( // dispatch resolves through the existing extends_name // path while the native_extends carries the (module, // class) tag for the runtime shim). - (None, Some(parent_name), native_parent, None) + (None, Some(canonical_parent_name), native_parent, None) } else if locally_shadowed { // Lexical local shadow → dynamic parent via `extends_expr` (the // in-scope local value), invoked by `super()` through @@ -1385,7 +1386,10 @@ pub fn lower_class_from_ast( ) } else if let ast::Expr::Ident(ident) = super_class.as_ref() { let parent_name = ident.sym.to_string(); - let native_parent = match parent_name.as_str() { + let canonical_parent_name = canonical_native_parent_name(ctx, &parent_name) + .unwrap_or(&parent_name) + .to_string(); + let native_parent = match canonical_parent_name.as_str() { "EventEmitter" => Some(("events".to_string(), "EventEmitter".to_string())), "EventEmitterAsyncResource" => Some(( "events".to_string(), @@ -1416,7 +1420,7 @@ pub fn lower_class_from_ast( "Readable" | "Writable" | "Duplex" | "Transform" if is_genuine_node_stream_parent(ctx, &parent_name) => { - Some(("node_stream".to_string(), parent_name.clone())) + Some(("node_stream".to_string(), canonical_parent_name.clone())) } _ => None, }; @@ -1430,7 +1434,7 @@ pub fn lower_class_from_ast( let locally_shadowed = !ctx.class_renames.contains_key(&parent_name) && ctx.locals.lookup(&parent_name).is_some(); if native_parent.is_some() && !locally_shadowed { - (None, Some(parent_name), native_parent, None) + (None, Some(canonical_parent_name), native_parent, None) } else if locally_shadowed { // #5437 (Next.js p-queue `PQueue` inside a minified bundle): a // class EXPRESSION whose parent Ident is an IN-SCOPE LOCAL diff --git a/crates/perry-runtime/src/array/iter_methods.rs b/crates/perry-runtime/src/array/iter_methods.rs index 5684512767..9a8c6ef6e7 100644 --- a/crates/perry-runtime/src/array/iter_methods.rs +++ b/crates/perry-runtime/src/array/iter_methods.rs @@ -1429,15 +1429,22 @@ fn typeof_owned_string(v: f64) -> String { /// Resolve a higher-order callback argument to its `ClosureHeader*` (as /// `i64`). Returns `Some(ptr)` only for values the runtime can actually -/// invoke (real closures, bound methods/functions); `None` for any -/// non-callable so the caller can throw the spec `TypeError`. +/// invoke (real closures, bound methods/functions, callable Proxies); `None` +/// for any non-callable so the caller can throw the spec `TypeError`. #[inline] fn resolve_callback_ptr(cb_boxed: f64) -> Option { use crate::value::JSValue; let jv = JSValue::from_bits(cb_boxed.to_bits()); if jv.is_pointer() { let ptr = jv.as_pointer::(); - if !crate::closure::get_valid_func_ptr(ptr).is_null() { + // #9681: a callable Proxy is a small registry id, so the hardened + // closure validator correctly rejects it as a ClosureHeader. Return + // that bare id anyway: DirectCallN falls back to js_closure_callN, + // whose proxy-callee path performs the Proxy [[Call]]. + if !crate::closure::get_valid_func_ptr(ptr).is_null() + || (crate::proxy::js_proxy_is_proxy(cb_boxed) == 1 + && crate::proxy::proxy_wraps_callable(cb_boxed)) + { return Some(ptr as i64); } } diff --git a/crates/perry-runtime/src/array/sort.rs b/crates/perry-runtime/src/array/sort.rs index 09775fbbe2..8d7b410c38 100644 --- a/crates/perry-runtime/src/array/sort.rs +++ b/crates/perry-runtime/src/array/sort.rs @@ -601,10 +601,21 @@ pub extern "C" fn js_validate_array_comparator(cmp_boxed: f64) -> i64 { if jv.is_undefined() { return 0; } - // Callable function -> comparator path. + // Callable closure or callable Proxy -> comparator path. Use the shared + // validator rather than probing CLOSURE_MAGIC directly: Proxy values are + // small registry ids, not dereferenceable ClosureHeader pointers. if jv.is_pointer() { let ptr = jv.as_pointer::(); - if !ptr.is_null() && unsafe { (*ptr).type_tag == crate::closure::CLOSURE_MAGIC } { + if !crate::closure::get_valid_func_ptr(ptr).is_null() { + return ptr as i64; + } + // #9681: DirectCall2 deliberately falls back to js_closure_call2 for + // this bare proxy id, which then performs the Proxy [[Call]]. Check + // callability here so a Proxy of a non-callable still throws before + // sorting starts. + if crate::proxy::js_proxy_is_proxy(cmp_boxed) == 1 + && crate::proxy::proxy_wraps_callable(cmp_boxed) + { return ptr as i64; } } @@ -634,6 +645,15 @@ fn throw_invalid_comparator(cmp_boxed: f64) -> ! { } } }; + // V8's diagnostic renderer uses # for an ordinary object (and a + // transparent Proxy of one), while JavaScript ToString yields + // [object Object]. Keep the existing ToString spellings for arrays, + // symbols, primitives, and other object kinds. + let value_str = if value_str == "[object Object]" { + "#".to_string() + } else { + value_str + }; let message = format!( "The comparison function must be either a function or undefined: {}", value_str 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-transform/src/generator/dispatch_growth_tests.rs b/crates/perry-transform/src/generator/dispatch_growth_tests.rs new file mode 100644 index 0000000000..2a35e77627 --- /dev/null +++ b/crates/perry-transform/src/generator/dispatch_growth_tests.rs @@ -0,0 +1,48 @@ +use super::*; + +fn async_generator_with_delegations(id: FuncId, delegations: usize) -> Function { + Function { + id, + name: "many_delegations".to_string(), + type_params: Vec::new(), + params: Vec::new(), + return_type: Type::Any, + body: (0..delegations) + .map(|_| { + Stmt::Expr(Expr::Yield { + value: Some(Box::new(Expr::GlobalGet(0))), + delegate: true, + }) + }) + .collect(), + is_async: true, + is_generator: true, + is_strict: false, + is_exported: false, + captures: Vec::new(), + decorators: Vec::new(), + was_plain_async: false, + was_unrolled: false, + } +} + +fn transformed_body_size(delegations: usize) -> usize { + let mut module = Module::new("delegation_growth"); + module + .functions + .push(async_generator_with_delegations(1, delegations)); + transform_generators(&mut module); + format!("{:?}", module.functions[0].body).len() +} + +#[test] +fn async_generator_yield_star_dispatch_growth_is_linear() { + let size_16 = transformed_body_size(16); + let size_32 = transformed_body_size(32); + + assert!( + size_32 < size_16 * 3, + "doubling yield* sites grew transformed HIR from {size_16} to {size_32} bytes; \ + every delegation route must share the state dispatcher instead of cloning it" + ); +} diff --git a/crates/perry-transform/src/generator/lower.rs b/crates/perry-transform/src/generator/lower.rs index 0ba687dc34..d22d51e89d 100644 --- a/crates/perry-transform/src/generator/lower.rs +++ b/crates/perry-transform/src/generator/lower.rs @@ -1090,10 +1090,10 @@ pub fn transform_generator_function_with_extra_captures( // inside a `yield *` and `gen.throw(e)` is called, forward the error into // the delegated iterator's `throw` (re-yielding or, on a `done` result, // resuming the outer body past the `yield *`) rather than routing it into - // the outer generator's own catch handlers. Each route re-drives the - // state machine via the `while_body_for_throw` continuation loop, so it - // is built BEFORE the loop is moved into `throw_continuation` below. - // Empty for sync generators (`delegations` is only recorded for async). + // the outer generator's own catch handlers. Delegation routes and the + // ordinary catch/throw fallback share one continuation dispatcher below; + // copying that full dispatcher into every route makes HIR grow + // quadratically for generators with many `yield*` sites. // #6709: for async generators `.throw(e)` is the error arm of the // shared step closure, so the thrown value arrives through the step's // value param (`next_param_id`) rather than a dedicated `.throw` @@ -1103,6 +1103,25 @@ pub fn transform_generator_function_with_extra_captures( } else { throw_param_id }; + // #4374: fresh binding for the inner catch that re-runs a try's finally + // when its catch handler itself throws (catch-rethrow-with-finally). + let inner_catch_id = alloc_local(next_local_id); + // #4374: continue the state machine after a catch — run the inlined + // finally and reach the next yield/completion within the `.throw()` call. + // The returned routing body deliberately excludes the dispatcher: all + // routes below fall through to one shared copy. + let ordinary_throw_fallback = build_async_throw_body( + &catches, + &finallys, + state_id, + done_id, + throw_val_id, + inner_catch_id, + pending_type_id, + pending_value_id, + &hoisted_ids, + true, + ); let yield_star_throw_routes = build_yield_star_throw_routes( &delegations, &catches, @@ -1111,44 +1130,21 @@ pub fn transform_generator_function_with_extra_captures( throw_val_id, pending_type_id, pending_value_id, - &while_body_for_throw, &hoisted_ids, next_local_id, + ordinary_throw_fallback, ); - // #4374: continue the state machine after a catch — run the inlined - // finally and reach the next yield/completion within the `.throw()` call. - // - // Async generators took the legacy deferred-resume path here, which inlines - // the catch body into the `.throw()` closure. That only works when the catch - // body is still inline; once the `try` contains a `yield` the linearizer has - // moved the catch into its own states, so the inlined copy had nothing to run - // and `gen.throw(e)` resolved to `{value: undefined, done: false}` instead of - // the value the catch yields. Routing to the catch's states (as sync - // generators do) is what Node's semantics require. - let throw_continuation = Some(while_body_for_throw); - // #4374: fresh binding for the inner catch that re-runs a try's finally - // when its catch handler itself throws (catch-rethrow-with-finally). - let inner_catch_id = alloc_local(next_local_id); let mut throw_resume_body = vec![Stmt::Expr(Expr::LocalSet( executing_id, Box::new(Expr::Bool(true)), ))]; - // The `yield *` throw routes return/throw from inside their own - // continuation loop on a match, so they precede the catch-routing body - // and only fall through to it when not suspended in a delegation. + // Exactly one delegation route or the ordinary catch/throw fallback + // runs, then every non-throwing path resumes through this shared loop. throw_resume_body.extend(yield_star_throw_routes); - throw_resume_body.extend(build_async_throw_body( - &catches, - &finallys, - state_id, - done_id, - throw_val_id, - inner_catch_id, - pending_type_id, - pending_value_id, - &hoisted_ids, - throw_continuation, - )); + throw_resume_body.push(Stmt::While { + condition: Expr::Bool(true), + body: while_body_for_throw, + }); // #6709: for async generators, `.next`/`.throw` are thin outer closures // driving a shared async-step `__agstep` closure so inner `await`s // suspend on the microtask queue; sync generators keep direct closures. diff --git a/crates/perry-transform/src/generator/lower/abrupt.rs b/crates/perry-transform/src/generator/lower/abrupt.rs index 6efe0b3525..2bd8301cc0 100644 --- a/crates/perry-transform/src/generator/lower/abrupt.rs +++ b/crates/perry-transform/src/generator/lower/abrupt.rs @@ -40,14 +40,12 @@ pub(crate) fn build_async_throw_body( pending_type_id: LocalId, pending_value_id: LocalId, hoisted_ids: &std::collections::HashSet, - // #4374: for sync generators, the cloned state-dispatch loop. When present, - // a matched catch route sets the resume state and *falls through* to this - // loop, so the inlined finally runs and the generator continues to the next - // yield / completion within the `.throw()` call. When `None` (async - // generators) the catch route returns {undefined, false} as before. - continuation: Option>, + // #4374: when true, a matched catch route sets the resume state and falls + // through to the state dispatcher that the caller appends. When false, use + // the legacy inline-catch behavior that returns from this body directly. + fall_through_to_dispatch: bool, ) -> Vec { - let fall_through = continuation.is_some(); + let fall_through = fall_through_to_dispatch; // #4374: when no catch handles the throw, run any pending non-yielding // `finally` before propagating the error. A `finally` that `return`s // supersedes the thrown value (rewritten to an iter-result return inside @@ -66,7 +64,7 @@ pub(crate) fn build_async_throw_body( fallback.extend(build_finally_run_stmts(finallys, state_id, hoisted_ids)); fallback.push(Stmt::Throw(Expr::LocalGet(throw_param_id))); - let mut body = if fall_through { + if fall_through { // #4438: sync generators route the thrown error to the innermost // enclosing catch (jump to its linearized states) or yielding finally // (record the pending throw + jump in), then fall through to the @@ -105,19 +103,7 @@ pub(crate) fn build_async_throw_body( }]; } fallback - }; - - // #4374: append the continuation loop. Only a fallen-through catch/finally - // route reaches it (the unhandled branch throws; matched routes set the - // resume state and fall through). - if let Some(cont) = continuation { - body.push(Stmt::While { - condition: Expr::Bool(true), - body: cont, - }); } - - body } pub(crate) fn catch_route_condition( @@ -797,11 +783,9 @@ pub(crate) fn build_yield_star_return_routes( /// completion). Both the done (resume) and not-done (re-yield) cases are handled /// uniformly: the awaited inner result is stored into the delegation's /// `result_id`, the state is set to the drive loop's condition state -/// (`resume_state`), and a clone of the state-dispatch loop (`while_body`) is -/// re-driven. The condition state reads `result.done` and either exits the loop -/// (continuing the outer body) or re-yields `result.value`. This continuation -/// loop is the async-generator abrupt-resume machinery the `.return()` path does -/// not need (a `return` always completes or re-yields, never resumes the body). +/// (`resume_state`), and control falls through to the caller's single shared +/// state dispatcher. The condition state reads `result.done` and either exits +/// the delegation (continuing the outer body) or re-yields `result.value`. /// /// An abrupt completion *of the delegation protocol itself* — `iterator.throw` /// rejecting, a non-object inner result, or the `throw`-undefined TypeError — @@ -813,9 +797,11 @@ pub(crate) fn build_yield_star_return_routes( /// inside that catch suspends) or, when no catch matches, runs pending /// non-yielding finallys and re-throws to reject the generator. /// -/// Each route returns or throws from inside its `while(true)` continuation loop, -/// so control falls through to the catch-routing fallback only when not -/// suspended in a delegation. Empty for sync generators (no routes recorded). +/// The routes form one mutually-exclusive if/else chain around `fallback`. +/// Successful delegation and catch-routing paths fall through to the one state +/// dispatcher appended by the caller. Keeping that dispatcher outside the +/// chain is load-bearing: cloning it into every route makes transformed HIR +/// quadratic in the number of `yield*` sites. #[allow(clippy::too_many_arguments)] pub(crate) fn build_yield_star_throw_routes( delegations: &[DelegationRoute], @@ -825,11 +811,11 @@ pub(crate) fn build_yield_star_throw_routes( throw_param_id: LocalId, pending_type_id: LocalId, pending_value_id: LocalId, - while_body: &[Stmt], hoisted_ids: &std::collections::HashSet, next_local_id: &mut u32, + fallback: Vec, ) -> Vec { - let mut out = Vec::with_capacity(delegations.len()); + let mut branches = Vec::with_capacity(delegations.len()); for route in delegations { let m_id = alloc_local(next_local_id); // captured `throw` method let ret_m_id = alloc_local(next_local_id); // `return` method (close path) @@ -1019,19 +1005,16 @@ pub(crate) fn build_yield_star_throw_routes( }), finally: None, }; - // Both the success path (state = resume_state) and a routed catch (state - // = catch_entry_state) fall through to this loop, which dispatches from - // the freshly-set state. - let drive = Stmt::While { - condition: Expr::Bool(true), - body: while_body.to_vec(), - }; + branches.push((in_interval, vec![protocol])); + } - out.push(Stmt::If { - condition: in_interval, - then_branch: vec![protocol, drive], - else_branch: None, - }); + let mut out = fallback; + for (condition, then_branch) in branches.into_iter().rev() { + out = vec![Stmt::If { + condition, + then_branch, + else_branch: Some(out), + }]; } out } diff --git a/crates/perry-transform/src/generator/mod.rs b/crates/perry-transform/src/generator/mod.rs index 5ac3f27a4e..cb2c340a81 100644 --- a/crates/perry-transform/src/generator/mod.rs +++ b/crates/perry-transform/src/generator/mod.rs @@ -497,3 +497,7 @@ pub fn transform_plain_async_closure_body( ); synth.body } + +#[cfg(test)] +#[path = "dispatch_growth_tests.rs"] +mod dispatch_growth_tests; 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"); + })(); +} diff --git a/test-files/test_gap_9681_proxy_array_callbacks.ts b/test-files/test_gap_9681_proxy_array_callbacks.ts new file mode 100644 index 0000000000..cea0f336ec --- /dev/null +++ b/test-files/test_gap_9681_proxy_array_callbacks.ts @@ -0,0 +1,83 @@ +// #9681: Array callback validation must recognize callable Proxy values +// without dereferencing their small registry ids as ClosureHeader pointers. +// +// Exercise every Array.prototype higher-order method plus sort with a plain +// function, an empty-handler Proxy of that function, a bound function, and a +// genuine non-callable. A Proxy of that non-callable pins the callability +// boundary too. Both invalid forms also pin Node's TypeError text. + +const source = [3, 1, 2]; + +function render(value: any): string { + const json = JSON.stringify(value); + return json === undefined ? "undefined" : json; +} + +function exercise(label: string, callback: any, invoke: any): void { + const names = ["plain", "proxy", "bound", "non-callable", "non-callable proxy"]; + const callbacks = [ + callback, + new Proxy(callback, {}), + callback.bind(null), + {}, + new Proxy({}, {}), + ]; + + for (let i = 0; i < callbacks.length; i++) { + try { + console.log(label + "/" + names[i] + ": " + render(invoke(callbacks[i]))); + } catch (e: any) { + console.log(label + "/" + names[i] + ": " + e.name + ": " + e.message); + } + } +} + +let forEachTotal = 0; +exercise( + "forEach", + (value: number) => { + forEachTotal += value; + }, + (callback: any) => { + forEachTotal = 0; + source.forEach(callback); + return forEachTotal; + }, +); + +exercise("map", (value: number, index: number) => value + index, (callback: any) => + source.map(callback), +); +exercise("filter", (value: number) => value > 1, (callback: any) => + source.filter(callback), +); +exercise("some", (value: number) => value === 1, (callback: any) => + source.some(callback), +); +exercise("every", (value: number) => value > 0, (callback: any) => + source.every(callback), +); +exercise("find", (value: number) => value < 3, (callback: any) => + source.find(callback), +); +exercise("findIndex", (value: number) => value < 3, (callback: any) => + source.findIndex(callback), +); +exercise("findLast", (value: number) => value < 3, (callback: any) => + source.findLast(callback), +); +exercise("findLastIndex", (value: number) => value < 3, (callback: any) => + source.findLastIndex(callback), +); +exercise("flatMap", (value: number) => [value, value * 10], (callback: any) => + source.flatMap(callback), +); +exercise("reduce", (acc: number, value: number) => acc + value, (callback: any) => + source.reduce(callback, 10), +); +exercise("reduceRight", (acc: number, value: number) => acc + value, (callback: any) => + source.reduceRight(callback, 10), +); +exercise("sort", (a: number, b: number) => a - b, (callback: any) => + [3, 1, 2].sort(callback), +); diff --git a/test-files/test_gap_async_generator_yield_star_shared_dispatch.ts b/test-files/test_gap_async_generator_yield_star_shared_dispatch.ts new file mode 100644 index 0000000000..7d9046decd --- /dev/null +++ b/test-files/test_gap_async_generator_yield_star_shared_dispatch.ts @@ -0,0 +1,58 @@ +// Async-generator `yield*` throw routes must select the active delegation and +// resume through one shared state dispatcher. This checks both separate +// delegation sites and a delegation-protocol error caught by the outer +// generator. The transform has a separate structural growth test ensuring the +// shared dispatcher is not cloned once per route. + +async function* inner(label: string) { + try { + yield label + ":first"; + } catch (error) { + return label + ":caught:" + error; + } +} + +async function* outer() { + const first = yield* inner("a"); + yield "after-a:" + first; + const second = yield* inner("b"); + yield "after-b:" + second; +} + +class BrokenDelegate { + [Symbol.asyncIterator]() { + return this; + } + + next() { + return Promise.resolve({ value: "broken:first", done: false }); + } + + throw() { + throw new Error("delegate-fail"); + } +} + +async function* catchesProtocolError() { + try { + yield* new BrokenDelegate(); + } catch (error: any) { + yield "outer-caught:" + error.message; + } +} + +async function main() { + const iterator = outer(); + console.log(JSON.stringify(await iterator.next())); + console.log(JSON.stringify(await iterator.throw("boom"))); + console.log(JSON.stringify(await iterator.next())); + console.log(JSON.stringify(await iterator.throw("bang"))); + console.log(JSON.stringify(await iterator.next())); + + const broken = catchesProtocolError(); + console.log(JSON.stringify(await broken.next())); + console.log(JSON.stringify(await broken.throw("ignored"))); + console.log(JSON.stringify(await broken.next())); +} + +main(); diff --git a/test-files/test_issue_9680_readdirp_stream_emitter_surface.ts b/test-files/test_issue_9680_readdirp_stream_emitter_surface.ts new file mode 100644 index 0000000000..585f41ffdc --- /dev/null +++ b/test-files/test_issue_9680_readdirp_stream_emitter_surface.ts @@ -0,0 +1,79 @@ +// Regression for #9680: the bundled readdirp source aliases its node:stream +// import, subclasses it, and hands the instance to chokidar. Chokidar chains +// on(...).on(...).once(...), so the stream must retain the complete inherited +// EventEmitter surface even though the native base's local name was minified. + +import { EventEmitter as jt } from "node:events"; +import { Readable as ut } from "node:stream"; + +class ReaddirpStyleStream extends ut { + readCalls = 0; + + constructor() { + super({ objectMode: true, autoDestroy: true, highWaterMark: 4096 }); + } + + _read(): void { + this.readCalls++; + } +} + +const Watcher = class extends jt { + marker(): string { + return "watcher-subclass"; + } +}; + +function readdirpStyle(): any { + return new ReaddirpStyleStream(); +} + +const stream = readdirpStyle(); +const emitterMethods = [ + "on", + "once", + "off", + "removeListener", + "emit", + "prependListener", + "prependOnceListener", + "listenerCount", + "eventNames", +]; + +const expectedNames = new Set(emitterMethods); +const surfaceNames = new Set(); +let prototypeCursor: any = stream; +while (prototypeCursor !== null) { + for (const name of Object.getOwnPropertyNames(prototypeCursor)) { + if (expectedNames.has(name)) surfaceNames.add(name); + } + prototypeCursor = Object.getPrototypeOf(prototypeCursor); +} + +stream._read(); +console.log("read-calls:" + stream.readCalls); +console.log("surface:" + Array.from(surfaceNames).sort().join(",")); +console.log(emitterMethods.map((name) => `${name}:${typeof stream[name]}`).join(",")); + +const calls: string[] = []; +const watcher: any = new Watcher(); +watcher.once("ready", (): void => calls.push("watcher-ready")); +watcher.emit("ready"); +console.log("watcher:" + watcher.marker()); + +const persistent = (value: string): void => calls.push(`on:${value}`); +stream + .on("entry", persistent) + .on("unused", (): void => calls.push("unused")) + .once("entry", (value: string): void => calls.push(`once:${value}`)); +stream.prependListener("entry", (value: string): void => calls.push(`prepend:${value}`)); +stream.prependOnceListener("entry", (value: string): void => calls.push(`prepend-once:${value}`)); + +console.log("events-before:" + stream.eventNames().join(",")); +console.log("listeners-before:" + stream.listenerCount("entry")); +console.log("emit-1:" + stream.emit("entry", "a")); +console.log("emit-2:" + stream.emit("entry", "b")); +stream.off("entry", persistent); +console.log("listeners-after:" + stream.listenerCount("entry")); +console.log("calls:" + calls.join(","));