fix(#5437): Next.js 16 app-router SSR — boot + statics/API (5/8 routes byte-identical) - #5880
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (16)
🚧 Files skipped from review as they are similar to previous changes (16)
📝 WalkthroughWalkthroughThis PR consolidates await-loop microtask/timer runtime hooks, reworks member-assignment lowering to reuse evaluated objects via a prelude pattern, adjusts class-capture stash timing/fallback behavior, and adds a Web Streams expando property system with GC-aware storage, dispatch, write routing, and weak-target validation. ChangesAwait loop, class lowering, and Web Streams expando updates
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
crates/perry-hir/src/lower_decl/class_captures.rs (1)
556-586: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDon’t inject
thisstashes before a derived ctor’s firstsuper().position()only sees top-level statements, so a validsuper()inside control flow falls back torebind_countand emitsthis.__perry_cap_* = ...at entry, which throws beforesuper()runs. Skip the early stash unless a top-levelsuper()exists, or place it after every possiblesuper()path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/perry-hir/src/lower_decl/class_captures.rs` around lines 556 - 586, The early `this.__perry_cap_*` injection in `class_captures.rs` is too eager for derived constructors: `ctor.body.iter().position(...)` only detects top-level `super()` calls, so when `super()` is nested in control flow the code falls back to inserting stashes at entry and can run before `super()`. Update the early-insert logic around `super_pos`, `early_insert_at`, and the `assignment_stmts` insertion so derived ctors only stash after a guaranteed top-level `super()` path, or otherwise defer the stash until after all possible `super()` executions (while keeping the existing end-of-body / before-return re-stash behavior).crates/perry-stdlib/src/streams.rs (1)
274-331: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winVerify
STREAM_EXPANDOentries are removed when a stream handle is closed/dropped.
STREAM_EXPANDOaccumulates(key, value)pairs keyed by streamid, andscan_stream_roots_mutkeeps every stored value alive for GC. Nothing in the provided context removes an entry when the corresponding stream is closed or its handle id is retired (unlike, e.g.,READABLE_STREAMS/WRITABLE_STREAMS, which presumably get cleaned up on close elsewhere). If no such cleanup exists, this is an unbounded per-process memory leak that also pins arbitrary JS values (e.g.allReadypromises) alive indefinitely — worse under a long-running SSR server handling many requests/streams.Please confirm whether stream teardown removes the matching
STREAM_EXPANDOentry.#!/bin/bash # Look for any removal of stream ids from the maps (READABLE_STREAMS etc.) # and check whether STREAM_EXPANDO is touched on close/drop. rg -n 'STREAM_EXPANDO' crates/perry-stdlib/src/streams.rs crates/perry-stdlib/src/streams/subclass.rs rg -n -A5 -B5 '\.remove\(&(id|handle)\)' crates/perry-stdlib/src/streams.rs crates/perry-stdlib/src/streams/subclass.rs rg -n 'fn.*close|fn.*drop|fn.*cancel' crates/perry-stdlib/src/streams.rs crates/perry-stdlib/src/streams/subclass.rs🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/perry-stdlib/src/streams.rs` around lines 274 - 331, STREAM_EXPANDO entries are never cleaned up, so expando values can stay pinned after a stream handle is closed or retired. Add teardown logic in the stream lifecycle path that removes the matching id from STREAM_EXPANDO when the handle is dropped/closed, and make sure the cleanup is wired into the same close/drop path used by the other stream registry maps. Use the existing symbols STREAM_EXPANDO, ensure_gc_registered, and stream_expando_set_hook to locate the expando storage and the appropriate teardown point.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/perry-codegen/src/expr/fs_await.rs`:
- Around line 195-207: The await-loop in fs_await currently advances timers
twice: js_promise_run_microtasks_await_loop already enters AwaitLoop mode, and
the block then calls js_await_loop_tick_timers again. Update the timer ownership
in this path by choosing one place to drive timers—either make AwaitLoop itself
use the guard-suspending timer path and remove the explicit
js_await_loop_tick_timers call, or keep js_await_loop_tick_timers here and
change js_promise_run_microtasks_await_loop to be microtask-only. Keep the
behavior around js_run_stdlib_pump and the surrounding await-loop logic
consistent with that choice.
In `@crates/perry-runtime/src/object/field_set_by_name.rs`:
- Around line 264-288: The new stream-id band handling in
js_object_set_field_by_name can fall through into the normal ObjectHeader path
and dereference an unmapped stream address. Update the block around
is_stream_id_band, stream_handle_probe, and stream_expando_set so that once the
address is recognized as a stream-band handle it always stops further
processing, even when probe is false, hooks are missing, the key is not valid
UTF-8, or setter fails. Mirror the existing reserved-handle/small-handle pattern
by returning after the stream-band branch rather than only on successful setter
writes.
---
Outside diff comments:
In `@crates/perry-hir/src/lower_decl/class_captures.rs`:
- Around line 556-586: The early `this.__perry_cap_*` injection in
`class_captures.rs` is too eager for derived constructors:
`ctor.body.iter().position(...)` only detects top-level `super()` calls, so when
`super()` is nested in control flow the code falls back to inserting stashes at
entry and can run before `super()`. Update the early-insert logic around
`super_pos`, `early_insert_at`, and the `assignment_stmts` insertion so derived
ctors only stash after a guaranteed top-level `super()` path, or otherwise defer
the stash until after all possible `super()` executions (while keeping the
existing end-of-body / before-return re-stash behavior).
In `@crates/perry-stdlib/src/streams.rs`:
- Around line 274-331: STREAM_EXPANDO entries are never cleaned up, so expando
values can stay pinned after a stream handle is closed or retired. Add teardown
logic in the stream lifecycle path that removes the matching id from
STREAM_EXPANDO when the handle is dropped/closed, and make sure the cleanup is
wired into the same close/drop path used by the other stream registry maps. Use
the existing symbols STREAM_EXPANDO, ensure_gc_registered, and
stream_expando_set_hook to locate the expando storage and the appropriate
teardown point.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1111c9c6-2b4b-4e89-8ba9-0e6c6443093d
📒 Files selected for processing (15)
crates/perry-codegen/src/expr/fs_await.rscrates/perry-codegen/src/runtime_decls/strings_part2.rscrates/perry-hir/src/lower/expr_assign.rscrates/perry-hir/src/lower/lower_expr/assignment.rscrates/perry-hir/src/lower_decl/class_captures.rscrates/perry-runtime/src/object/class_handles.rscrates/perry-runtime/src/object/class_registry.rscrates/perry-runtime/src/object/field_set_by_name.rscrates/perry-runtime/src/promise/microtasks.rscrates/perry-runtime/src/promise/then.rscrates/perry-runtime/src/proxy/put_value.rscrates/perry-runtime/src/timer.rscrates/perry-runtime/src/weakref.rscrates/perry-stdlib/src/streams.rscrates/perry-stdlib/src/streams/subclass.rs
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/perry-stdlib/src/streams/expando.rs`:
- Around line 59-67: The expando registry in STREAM_EXPANDO is retaining entries
for dead handles, so the GC scan keeps them alive indefinitely. Update the
handle teardown/destroy path associated with expando cleanup to remove that
handle’s entry from STREAM_EXPANDO when the handle is torn down, and keep
scan_expando_roots focused only on visiting currently live entries.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b414d626-7fa2-4f52-bcd1-63d9836f387f
📒 Files selected for processing (2)
crates/perry-stdlib/src/streams.rscrates/perry-stdlib/src/streams/expando.rs
proggeramlug
pushed a commit
that referenced
this pull request
Jul 3, 2026
- [critical] js_object_set_field_by_name: a failed stream-expando write (dead/unregistered handle, hooks absent, non-UTF-8 key, or null key) fell through to the generic ObjectHeader path and dereferenced addr-GC_HEADER_SIZE at a reserved stream-band address -> SIGSEGV. Always return once the address is confirmed in the stream-id band, mirroring the reserved small-handle guard. Same early-return mirrored in proxy/put_value.rs for defense-in-depth. - [major] await loop advanced timers twice per iteration (AwaitLoop drain fired timers AND the codegen loop calls js_await_loop_tick_timers). They are always emitted as a pair and this is the mode's only caller, so make AwaitLoop microtask-only; js_await_loop_tick_timers (guard-suspending) is the sole timer owner. - [major] STREAM_EXPANDO grew one row per stream over the server lifetime (ids are monotonic, never reused) and scan_expando_roots kept those values alive. Add stream_expando_clear(id) from close_pending/error_pending (allReady is consumed during the render, before close). Claude-Session: https://claude.ai/code/session_01RcePwqv92QidGakrfYvf3v
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/perry-stdlib/src/streams/expando.rs`:
- Around line 58-69: Update the expando cleanup so it is triggered for reader
and writer handle teardown as well as stream teardown: `stream_expando_set_hook`
can attach rows for readers and writers, but only `stream_expando_clear` is
currently used from readable-stream terminal paths. Wire `stream_expando_clear`
into `js_reader_release_lock`, `js_writer_release_lock`, and the writable
close/abort cleanup paths so handle ids are removed when those resources are
released, not just when a readable stream closes or errors.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a70a8cec-e2cf-40e1-ac9d-2356b526ca84
📒 Files selected for processing (5)
crates/perry-runtime/src/object/field_set_by_name.rscrates/perry-runtime/src/promise/microtasks.rscrates/perry-runtime/src/proxy/put_value.rscrates/perry-stdlib/src/streams.rscrates/perry-stdlib/src/streams/expando.rs
🚧 Files skipped from review as they are similar to previous changes (3)
- crates/perry-runtime/src/promise/microtasks.rs
- crates/perry-stdlib/src/streams.rs
- crates/perry-runtime/src/object/field_set_by_name.rs
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/perry-stdlib/src/streams/writable.rs`:
- Around line 220-221: The terminal cleanup in writable stream transitions is
clearing user-visible expando state too early, which affects still-referenceable
handles. In the writable stream code path around `stream_expando_clear(...)` in
`writable.rs`, remove the expando-table clearing from these terminal transitions
and keep `STREAM_EXPANDO` alive until the handle is actually unreachable, or
split internal-only cleanup into a separate store so `writer.releaseLock()` and
similar live objects retain their properties.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8bb88a31-7847-4d5f-9e0d-90731adc6e62
📒 Files selected for processing (2)
crates/perry-stdlib/src/streams.rscrates/perry-stdlib/src/streams/writable.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/perry-stdlib/src/streams.rs
added 8 commits
July 3, 2026 10:31
… WeakMap keys (rebased onto main) Port of fix/nextjs-closure-dispatch-divergence-5437 onto current main: - evaluate a member-assignment object expression once when it is a local assignment (target/receiver previously duplicated the side effect) - accept a live registered Web Stream handle as a weak key in is_valid_weak_target (raw stream-id band + stream_handle_probe); plain numbers in the band still throw (guard test included) Claude-Session: https://claude.ai/code/session_01RcePwqv92QidGakrfYvf3v (cherry picked from commit 85b15ab352a4431116d7b7c9bb91a489ab9e3e5d)
…ly at ctor end ec40e45 (#5587) moved the this.__perry_cap_* stashes to the end of the constructor body so post-super() mutations of a captured outer local are reflected in the final stash (TemporalHelpers ++called). But a constructor body may call instance methods (this.has = this.getHas()), and a method reading a captured outer resolves it through the cap field — stashing only at the end left those reads undefined. Next.js's base Server constructor calls this.getHasStaticDir(), which reads the module-level _fs interop binding via its cap field, and threw "Cannot read properties of undefined (reading 'default')" at server boot (bisected to ec40e45; 8-line repro). Stash early (after super / at entry) for intra-ctor method calls AND re-stash at the end / before returns so post-super mutations still win. The assignments are idempotent. Claude-Session: https://claude.ai/code/session_01RcePwqv92QidGakrfYvf3v (cherry picked from commit 8879740fc265b7c5cd9c0f5feb8367482361afef)
…he stash is unset A method's capture prologue read this.__perry_cap_* bare. That field is stashed by the constructor only after super() returns — but a base-class constructor can virtual-dispatch into this class's override before that (Next.js: base Server's ctor calls this.getHasStaticDir(), the NextNodeServer override, which reads the module-level _fs interop binding through its cap field), so the read produced undefined and the server threw at boot. Wrap the prologue init in ClassCaptureValue with prefer_fallback: the field read stays authoritative whenever it is set (normal calls, post-writeback mutations), and the class's decl-site capture snapshot fills in only when the field is still undefined — the same param-or-snapshot machinery the constructor rebinds already use. Claude-Session: https://claude.ai/code/session_01RcePwqv92QidGakrfYvf3v (cherry picked from commit dde4c3f4e3fb93dde4ad85658e487c34f02ded04)
…ties React's renderToReadableStream attaches its shell-ready promise as an expando on the stream (stream.allReady = ...). Stream handles are raw stream-band ids with no property store, so the write was silently dropped (sloppy mode) or threw 'Cannot assign to read only property' (strict) — renderToReadableStream died, Next.js dynamic-SSR routes hung with zero bytes (the pipe stage was never reached; PUMP-probe confirmed). - stdlib: STREAM_EXPANDO side table (id -> (key, value) pairs) with the set hook registered at stream init; values GC-traced in scan_stream_roots_mut; dispatch_stream_property consults the table after the WHATWG getters so stream.allReady reads back. - runtime: stream-band branch in js_put_value_set and js_object_set_field_by_name routes property writes on live stream handles (stream_handle_probe-verified) to the registered hook, mirroring the #1545 probe pattern. Claude-Session: https://claude.ai/code/session_01RcePwqv92QidGakrfYvf3v (cherry picked from commit 78089480e7d8418e54f2c4d064c7610679e1a2f1)
…nside a timer dispatch The codegen Expr::Await busy-wait loop ticked the three timer queues, but js_callback_timer_tick / js_interval_timer_tick early-return whenever the thread is already inside a timer-callback dispatch — and every HTTP request handler runs inside one. React's server renderer schedules its render and flush work via setImmediate, so a busy-wait await in the request path (app-page bundle awaits that escape the async transform) parked in js_wait_for_event forever: microtasks drained, but the setImmediate that would settle the awaited promise never fired. Next.js dynamic-SSR routes hung with zero bytes (SIGBT backtrace: js_wait_for_event under AsyncLocalStorage.run in the app-page-turbo render). An await is a yield point — the real event loop runs due timers there. Add js_await_loop_tick_timers (suspends the dispatch-depth guard for one tick round; callbacks fired within still guard their own nested plain ticks) and a MicrotaskDrainMode::AwaitLoop entry that fires the drain-level timer block reentrantly; the await wait block now uses both. Claude-Session: https://claude.ai/code/session_01RcePwqv92QidGakrfYvf3v (cherry picked from commit 2121ef11b14b83d99b74f3d22cfb9a994930fb35)
The #5437 stream-expando store pushed streams.rs to 2008 lines, over the 2000-line file-size gate. Move the STREAM_EXPANDO table, set hook, getter, and GC scan into a sibling module (behavior-preserving); streams.rs re-exports the getter for streams/subclass.rs. streams.rs back to 1960 lines. Claude-Session: https://claude.ai/code/session_01RcePwqv92QidGakrfYvf3v
- [critical] js_object_set_field_by_name: a failed stream-expando write (dead/unregistered handle, hooks absent, non-UTF-8 key, or null key) fell through to the generic ObjectHeader path and dereferenced addr-GC_HEADER_SIZE at a reserved stream-band address -> SIGSEGV. Always return once the address is confirmed in the stream-id band, mirroring the reserved small-handle guard. Same early-return mirrored in proxy/put_value.rs for defense-in-depth. - [major] await loop advanced timers twice per iteration (AwaitLoop drain fired timers AND the codegen loop calls js_await_loop_tick_timers). They are always emitted as a pair and this is the mode's only caller, so make AwaitLoop microtask-only; js_await_loop_tick_timers (guard-suspending) is the sole timer owner. - [major] STREAM_EXPANDO grew one row per stream over the server lifetime (ids are monotonic, never reused) and scan_expando_roots kept those values alive. Add stream_expando_clear(id) from close_pending/error_pending (allReady is consumed during the render, before close). Claude-Session: https://claude.ai/code/session_01RcePwqv92QidGakrfYvf3v
…(CodeRabbit follow-up) The stream-expando set hook accepts any live stream-band handle (readable, writable, reader, writer, transform), but cleanup only ran from readable-stream close/error, so reader/writer/writable handle ids leaked their expando rows for the process lifetime. Clear on the remaining terminal points: - js_reader_release_lock / js_writer_release_lock (instance done) - finish_writable_close_success / _error (terminal Closed/Errored) - js_writable_stream_abort_inner (terminal Errored) Claude-Session: https://claude.ai/code/session_01RcePwqv92QidGakrfYvf3v
proggeramlug
force-pushed
the
fix/nextjs-ssr-5437-clean
branch
from
July 3, 2026 08:46
c4e3519 to
04efdb8
Compare
This was referenced Jul 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Enables Next.js 16.2.9 app-router SSR to boot and serve under Perry. Six runtime/HIR/codegen fixes take the reconstructed app from crash-at-boot to 5 of 8 routes byte-identical to
node --experimental-strip-typesv26.Without these, current
maincompiles the server bundle but crashes during boot withTypeError: Cannot read properties of undefined (reading 'default')before it can listen.Fixes
expr_assign.rs,assignment.rs,weakref.rs) — a member-assignment whose object is a side-effectingLocalSetwas lowered into both target and receiver, evaluating it twice; evaluate once and read back. Also lets live Web Stream handles be valid WeakMap keys. This was the original HTTP-500.class_captures.rs) — capture-field stashes must run right aftersuper()(not only at ctor end), and method cap reads fall back to the decl-site snapshot when the stash is unset. (2 commits.)streams.rs,subclass.rs,put_value.rs) — React'srenderToReadableStreamattachesstream.allReady; without an expando store the write was dropped/threw.timer.rs,microtasks.rs,fs_await.rs) — a busy-waitawaitentered from inside a timer/setImmediatecallback couldn't see a timer-scheduled resolution..finally()on a settled promise dispatches its own wrapper exactly once (then.rs).Validation
Compiled the app with
perry compile server.jsand diffed each route against a node v26 oracle (single request,lsofLISTEN wait, server confirmed alive after):/,/about,/counter(static)/api/helloGET + POST/posts/123,/fetcher,/plain(dynamic SSR)Boots cleanly (pure
maincrashes at boot without these); zero regression on the 5 passing routes.Not in this PR
The 3 dynamic-SSR routes hang in the live React streaming render. Root cause is structural, diagnosed and confirmed via a SIGUSR1 backtrace probe: the render's capturing
asyncclosures are skipped by the async→generator transform'sbody_has_capturing_closureguard, soawaitlowers to a busy-wait loop that never suspends the render — anything whose resolution needs that call chain to unwind (node's real async suspend/resume) deadlocks. The fix requires productionizing the experimentalPERRY_ASYNC_CAPTURINGtransform (which has multiple independent bugs: an async-step next-promise reuse mis-fire under many live machines, plus at least two more). That is a dedicated follow-up, tracked separately — deliberately out of scope here so the boot + 5/8 fixes can land.https://claude.ai/code/session_01RcePwqv92QidGakrfYvf3v
Summary by CodeRabbit
New Features
Bug Fixes