Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions changelog.d/8879-stdin-end-provider-arm.md
Original file line number Diff line number Diff line change
@@ -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 |
36 changes: 36 additions & 0 deletions crates/perry-stdlib/src/readline/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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;
}
});
}
_ => {}
}
}
Expand Down
49 changes: 49 additions & 0 deletions crates/perry-stdlib/src/readline/mod_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
Loading