From 9297458d0673916006472a5f0735edb850fbe281 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 27 Aug 2026 14:39:44 +0200 Subject: [PATCH] fix(stdin): restore the end/close arm on the listener provider path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `echo hi | claude -p "…"` never completes on main. #8861 fixed this by teaching three layers about `end` listeners; the batch landing kept the GC rooting of STDIN_END_CALLBACKS and STDIN_PULL_MODE, but dropped the `"end" | "close"` arm in stdin_on_op — the one that made it work. stdin_on_op is the provider the stdin object's native on/once/addListener delegate to: every registration that is NOT codegen's literal process.stdin.x(…) shape, i.e. an alias or stdin passed as a parameter. Claude Code's print-mode reader is the parameter form — X71(process.stdin, 3000) then stream.once("end", …) inside — so those registrations hit `_ => return` and were discarded, and the end half of its race(once("end"), timeout(3000)) could never win. stdin_off_op gets the mirror arm: #8864 removed the end/close clause from the removal path, so a provider-registered listener could be added but never removed. The two tests that guarded this were dropped with the arm, which is why CI stayed green. Restored, asserting the provider entry points directly rather than through js_readline_stdin_on — the extern path kept working, so it cannot catch this. Sabotage-checked. Bundle: 4/4 hang before, ~7s after. readline suite 21/21. --- changelog.d/8879-stdin-end-provider-arm.md | 38 ++++++++++++++ crates/perry-stdlib/src/readline/mod.rs | 36 ++++++++++++++ crates/perry-stdlib/src/readline/mod_tests.rs | 49 +++++++++++++++++++ 3 files changed, 123 insertions(+) create mode 100644 changelog.d/8879-stdin-end-provider-arm.md diff --git a/changelog.d/8879-stdin-end-provider-arm.md b/changelog.d/8879-stdin-end-provider-arm.md new file mode 100644 index 0000000000..18e5245476 --- /dev/null +++ b/changelog.d/8879-stdin-end-provider-arm.md @@ -0,0 +1,38 @@ +Restored the `end`/`close` arm on the stdin listener **provider** path, which +was lost when #8861 was batch-landed. Without it `echo hi | claude -p "…"` +never completes. + +#8861 fixed piped stdin for `-p` by teaching three layers about `end` +listeners. Two of them landed: the GC rooting of `STDIN_END_CALLBACKS`, and +`STDIN_PULL_MODE`. The third — the `"end" | "close"` arm in `stdin_on_op` — +did not, and it is the one that made the fix work. + +`stdin_on_op` is the provider that the stdin object's native +`on` / `once` / `addListener` methods delegate to: every registration that is +**not** codegen's literal `process.stdin.x(…)` shape. That means an alias +(`const s = process.stdin; s.once("end", …)`) or stdin passed as a parameter +(`helper(process.stdin)`). Claude Code's print-mode reader is exactly the +parameter form — `X71(process.stdin, 3000)`, then `stream.once("end", …)` on +the parameter — so those registrations fell into `_ => return` and were +silently discarded. The `end` half of the reader's +`race(once("end"), timeout(3000))` could never win. + +`stdin_off_op` gets the mirror arm. #8864 removed the `end`/`close` clause from +the removal path, so a provider-registered listener could be added but never +removed — `removeListener` / `off` leaked it and the pump kept a stale callback +pointer. + +The two tests that guarded this — `provider_path_registers_end_listeners` and +`provider_path_removes_end_listeners` — were dropped alongside the arm, which is +why CI stayed green through the regression. Both are restored here. They assert +the provider entry points (`stdin_on_op` / `stdin_off_op`) **directly** rather +than going through the `js_readline_stdin_on` extern, because the extern path +kept working the whole time and therefore cannot catch this. Sabotage-checked: +deleting the arm again fails `provider_path_registers_end_listeners`. + +Measured on the claude-code 2.1.112 bundle, `echo hello | cc -p "…"`: + +| build | result | +|---|---| +| before this fix | 4/4 hang (120 s timeout, no output) | +| with the arm restored | completes in ~7 s | diff --git a/crates/perry-stdlib/src/readline/mod.rs b/crates/perry-stdlib/src/readline/mod.rs index a0a0069c1b..d94fd177e2 100644 --- a/crates/perry-stdlib/src/readline/mod.rs +++ b/crates/perry-stdlib/src/readline/mod.rs @@ -376,6 +376,27 @@ extern "C" fn stdin_on_op(name_ptr: *const u8, name_len: usize, cb: i64, _once: v.push(cb); } } + // #8861: `end`/`close` MUST be handled here, not only in the syntactic + // `js_readline_stdin_on` extern. + // + // This provider is what the stdin OBJECT's native `on`/`once`/ + // `addListener` methods delegate to — i.e. every registration that does + // not match codegen's literal `process.stdin.x(…)` pattern: an alias + // (`const s = process.stdin; s.once("end", …)`) or stdin passed as a + // parameter (`helper(process.stdin)`). That is exactly what Claude + // Code's print-mode reader does: `X71(process.stdin, 3000)`, then + // `stream.once("end", …)` on the parameter inside. + // + // Falling into the `_ => return` below silently discards those + // listeners. Node fires the direct, aliased and parameter forms alike; + // without this arm perry fires only the direct one, so the `end` half + // of the reader's `race(once("end"), timeout(3000))` can never win and + // `echo hi | claude -p …` never completes. + "end" | "close" => { + if let Ok(mut v) = STDIN_END_CALLBACKS.lock() { + v.push(cb); + } + } _ => return, } try_register_pump(); @@ -410,6 +431,21 @@ extern "C" fn stdin_off_op(name_ptr: *const u8, name_len: usize, cb: i64) { v.retain(|r| *r != cb); } } + // Mirror of the `end`/`close` arm in `stdin_on_op`. Without it a + // listener registered through the provider path can be added but never + // removed, so `removeListener`/`off` leaks it and the pump keeps a + // stale callback pointer. + "end" | "close" => { + if let Ok(mut v) = STDIN_END_CALLBACKS.lock() { + v.retain(|r| *r != cb); + } + CLOSE_CALLBACK.with(|slot| { + let mut slot = slot.borrow_mut(); + if *slot == Some(cb) { + *slot = None; + } + }); + } _ => {} } } diff --git a/crates/perry-stdlib/src/readline/mod_tests.rs b/crates/perry-stdlib/src/readline/mod_tests.rs index f93865beb4..5d3e0d5b78 100644 --- a/crates/perry-stdlib/src/readline/mod_tests.rs +++ b/crates/perry-stdlib/src/readline/mod_tests.rs @@ -77,6 +77,55 @@ fn every_stdin_end_listener_fires() { ); } +/// The PROVIDER path — `stdin_on_op` / `stdin_off_op` — must handle +/// `end`/`close` too, not just the syntactic `js_readline_stdin_on` extern +/// that `every_stdin_end_listener_fires` above covers. +/// +/// The provider is what the stdin object's native `on`/`once`/`addListener` +/// delegate to, i.e. every registration that is NOT codegen's literal +/// `process.stdin.x(…)` shape: an alias, or stdin passed as a parameter. +/// Claude Code's print-mode reader is the parameter form — `X71(process.stdin, +/// 3000)` then `stream.once("end", …)` inside — so without these arms its +/// `race(once("end"), timeout(3000))` can never resolve via `end` and +/// `echo hi | claude -p …` never completes. +/// +/// These two tests guard arms that have now been lost twice, which is why they +/// assert the provider entry points directly rather than going through the +/// extern. +#[test] +fn provider_path_registers_end_listeners() { + let _g = reset(); + let a = data_counter_callback(); + let b = data_counter_callback(); + stdin_on_op(b"end".as_ptr(), 3, a, 0); + stdin_on_op(b"close".as_ptr(), 5, b, 1); + assert_eq!( + STDIN_END_CALLBACKS.lock().map(|v| v.len()).unwrap_or(0), + 2, + "aliased end/close registrations must reach the end-listener list" + ); + EOF_REACHED.store(true, Ordering::Release); + js_readline_process_pending(); + assert_eq!( + DATA_COUNT.with(|n| *n.borrow()), + 2, + "listeners registered through the provider must fire at EOF" + ); +} + +#[test] +fn provider_path_removes_end_listeners() { + let _g = reset(); + let cb = data_counter_callback(); + stdin_on_op(b"end".as_ptr(), 3, cb, 0); + stdin_off_op(b"end".as_ptr(), 3, cb); + assert_eq!( + STDIN_END_CALLBACKS.lock().map(|v| v.len()).unwrap_or(0), + 0, + "removing an aliased end listener must clear it" + ); +} + /// An `on("readable")` listener puts stdin in paused/pull mode, where the /// fd-0 reader must buffer bytes for `process.stdin.read()` instead of /// routing them to readline's line queue (which `read()` never drains).