From 56ded68043dae101cbb324063138672096ce0c2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 16 Jul 2026 13:40:36 +0200 Subject: [PATCH 1/2] fix(streams): a throwing ReadableStream pull() errors the stream, not crash MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Perry invokes a ReadableStream's `pull()` from a Rust-owned microtask (`readable_pull_microtask`). If the pull callback threw, the exception `longjmp`ed straight out to the microtask loop — past this Rust frame — skipping the `pulling = false` reset and leaving the stream wedged; under concurrency this corrupted stream state and crashed the process. (Found driving a real Next.js app whose byte-stream pull hit a cold-path throw.) Per the Web Streams spec a throwing pull must run ReadableStreamDefaultControllerError: the stream errors and its pending reads reject with the thrown value. Add a public `exception::js_call_catching(f) -> Result` (setjmp-based Rust try/catch, mirroring `combinator_catch_js`) and wrap the pull invocation with it; on a throw, call `error_readable_stream(...)` so `reader.read()` rejects and the consumer's own handler observes it — exactly as Node does. Gap test `test_gap_readable_stream_pull_throws.ts` covers the default and byte-stream pull paths: read() rejects with the thrown message and the process survives. Claude-Session: https://claude.ai/code/session_01KD4GTmiwZjRrxShzQydJpF --- crates/perry-runtime/src/exception.rs | 26 +++++++++++++++++++ crates/perry-stdlib/src/streams.rs | 24 +++++++++++++---- .../test_gap_readable_stream_pull_throws.ts | 20 ++++++++++++++ 3 files changed, 65 insertions(+), 5 deletions(-) create mode 100644 test-files/test_gap_readable_stream_pull_throws.ts diff --git a/crates/perry-runtime/src/exception.rs b/crates/perry-runtime/src/exception.rs index e85f982e43..d7e0e39589 100644 --- a/crates/perry-runtime/src/exception.rs +++ b/crates/perry-runtime/src/exception.rs @@ -144,6 +144,32 @@ pub(crate) fn current_try_depth() -> usize { with_exception_state(|s| unsafe { (*s).try_depth }) } +/// Invoke `f` — which may call into user JS and `js_throw` — inside a `try` +/// trap, catching any JS exception. Returns `Ok(value)` on a normal return, +/// or `Err(exception_bits)` if `f` threw. The `setjmp` lives in THIS frame, +/// which stays alive while `f` runs, so the `longjmp` target is valid, and the +/// throw unwinds only up to here — NOT past the Rust caller's frame. +/// +/// Runtime helpers that drive user JS from a Rust-owned microtask/timer +/// context (e.g. a Web Streams `pull` callback) use this so a throwing +/// callback errors the relevant object per spec instead of `longjmp`-ing past +/// their frames — skipping cleanup and corrupting state. Mirrors +/// `combinators::combinator_catch_js`. +pub fn js_call_catching(f: impl FnOnce() -> f64) -> Result { + let env = js_try_push(); + let jumped = unsafe { crate::ffi::setjmp::setjmp(env as *mut std::os::raw::c_int) }; + if jumped == 0 { + let result = f(); + js_try_end(); + Ok(result) + } else { + js_try_end(); + let err = js_get_exception(); + js_clear_exception(); + Err(err) + } +} + /// Throw an exception with the given value #[no_mangle] pub extern "C" fn js_throw(value: f64) -> ! { diff --git a/crates/perry-stdlib/src/streams.rs b/crates/perry-stdlib/src/streams.rs index ad2b32d67e..c32c09bca0 100644 --- a/crates/perry-stdlib/src/streams.rs +++ b/crates/perry-stdlib/src/streams.rs @@ -686,14 +686,28 @@ extern "C" fn readable_pull_microtask(closure: *const ClosureHeader) -> f64 { } }; if should_pull { - if pull_returns_byte_chunk { - pull_deferred_byte_chunk(stream_id, cb); - } else { - js_closure_call1(cb as *const ClosureHeader, stream_id as f64); - } + // A Web Streams `pull()` that throws must ERROR the stream (its + // pending reads reject with the exception) — it must NOT `longjmp` + // past this Rust microtask frame. Without the local trap the throw + // unwinds to the microtask loop, skipping the `pulling = false` + // reset (and any live frames), leaving the stream wedged and, under + // concurrency, crashing the process. Catch it here and route to the + // spec error path so `reader.read()` rejects and the consumer's own + // handler surfaces the error, exactly as Node does. + let pull_outcome = perry_runtime::exception::js_call_catching(|| { + if pull_returns_byte_chunk { + pull_deferred_byte_chunk(stream_id, cb); + } else { + js_closure_call1(cb as *const ClosureHeader, stream_id as f64); + } + f64::from_bits(TAG_UNDEFINED) + }); if let Some(s) = READABLE_STREAMS.lock().unwrap().get_mut(&stream_id) { s.pulling = false; } + if let Err(exc) = pull_outcome { + error_readable_stream(stream_id, exc.to_bits()); + } } } f64::from_bits(TAG_UNDEFINED) diff --git a/test-files/test_gap_readable_stream_pull_throws.ts b/test-files/test_gap_readable_stream_pull_throws.ts new file mode 100644 index 0000000000..01cc783bca --- /dev/null +++ b/test-files/test_gap_readable_stream_pull_throws.ts @@ -0,0 +1,20 @@ +// A ReadableStream whose pull() throws must ERROR the stream: pending read()s +// reject with the thrown value and the process stays alive (it must NOT crash +// by unwinding out of the internal pull microtask). Byte streams take a +// separate perry pull path, so cover both. +async function probe(label: string, opts: any) { + const rs = new ReadableStream(opts); + const reader = rs.getReader(); + try { + await reader.read(); + console.log(label + ": UNEXPECTED resolve"); + } catch (e) { + console.log(label + ": rejected: " + (e as Error).message); + } +} +async function main() { + await probe("default-pull-throw", { pull() { throw new Error("boom-default"); } }); + await probe("byte-pull-throw", { type: "bytes", pull() { throw new Error("boom-byte"); } }); + console.log("process survived"); +} +main(); From 4dbaa3315b63eeef2640bbaa775b90f2179ad727 Mon Sep 17 00:00:00 2001 From: Ralph Date: Mon, 20 Jul 2026 10:13:27 -0700 Subject: [PATCH 2/2] chore(lint): allowlist streams.rs (2011 lines, +11 from this PR's throwing-pull fix) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The js_call_catching wiring pushed perry-stdlib/src/streams.rs 11 lines over the 2000-line gate. Allowlist it — same shape as the other small-overage entries; a reader/tee/BYOB sub-controller split is a mechanical follow-up. Co-Authored-By: Claude Opus 4.8 (1M context) --- scripts/check_file_size.sh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/scripts/check_file_size.sh b/scripts/check_file_size.sh index d1874566e9..dd70f5744a 100755 --- a/scripts/check_file_size.sh +++ b/scripts/check_file_size.sh @@ -120,6 +120,11 @@ crates/perry-ui-windows/src/widgets/mod.rs # split of the character-class / quantifier sub-parsers is a mechanical # follow-up. crates/perry-runtime/src/regex/grammar.rs +# Web Streams core (2011 lines): #6476's throwing-pull resilience fix added the +# js_call_catching wiring, 11 lines over. A split of the reader/tee/BYOB +# sub-controllers into siblings (tee.rs / byob already peeled) is a mechanical +# follow-up. +crates/perry-stdlib/src/streams.rs EOF )