fix(runtime,codegen): stop shape-probing plain-double receivers — Web Streams handle ids past k=512 dereferenced unmapped memory - #6599
Conversation
…inter-shaped receivers A plain-double receiver in js_native_call_method's URLSearchParams fast-path had its low 48 bits reinterpreted as a heap address. Web Streams handles are raw numeric f64 ids starting at 0x100000: id 0x100000+k extracts to k*2^32, which crosses the macOS 2 TB heap floor once k >= 512 and the shape probe then dereferences unmapped memory. A Next.js app render burns ~48 stream ids per request, so request 12's for-await over the render stream (@@asynciterator -> bound 'values' re-dispatch) was the first to segfault. On Linux the heap floor is 0x1000, so low ids probed low memory from the start. Gate the block on a pointer-shaped receiver (NaN-boxed pointer above the handle band, or a raw untagged heap address), mirroring the AbortSignal block below it. Numeric stream receivers fall through to the primitive-methods stream dispatch that owns them.
… iterator fold The PerryTS#597 any-typed .values()/.keys()/.entries() fold masked the receiver to 48 bits in codegen before calling js_array_*_iter_obj, so a plain double receiver (a Web Streams handle id among them) became an indistinguishable garbage 'heap address' that the Map/Set/URLSearchParams registry probes dereferenced — the second half of the gscmaster request-12 SIGSEGV. Codegen now passes the full NaN-box bits (raw heap pointers still arrive untagged and take the legacy path bit-for-bit); the runtime routes pointer-shaped bits as before, dispatches live Web Streams handles through js_native_call_method to the stdlib stream arms, and throws the spec TypeError for other primitive receivers instead of dereferencing their bits.
|
Warning Review limit reached
Next review available in: 1 minute Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughIterator lowering now preserves full NaN-boxed receiver bits. Runtime iterator routing distinguishes heap pointers, stream handles, and primitive values, while URLSearchParams dispatch only probes pointer-shaped receivers. A regression test covers high stream IDs and erased iterator calls. ChangesIterator receiver routing
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant IteratorLowering
participant IterObject
participant NativeCall
participant ReadableStream
IteratorLowering->>IterObject: pass full receiver bits
IterObject->>NativeCall: dispatch stream iterator method
NativeCall->>ReadableStream: invoke values or async iterator
ReadableStream-->>NativeCall: return iterator result
NativeCall-->>IterObject: return routed iterator
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
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-runtime/src/array/iter_object.rs`:
- Around line 380-390: Update the value classification before pointer routing in
the iterator receiver logic around array_iter_obj_raw: reject zero-valued
numbers and boolean payloads with the existing TypeError path instead of
converting them to tiny/null pointers. Preserve NaN-boxing invariants and treat
values below 0x100000 as handles for small-pointer detection; add regressions
covering 0 as any and true as any for all three iterator names.
🪄 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: 930b303f-1236-4ad0-a6ea-7e5beb6c696b
📒 Files selected for processing (5)
crates/perry-codegen/src/expr/arrays_finds.rscrates/perry-codegen/src/lower_array_method.rscrates/perry-runtime/src/array/iter_object.rscrates/perry-runtime/src/object/native_call_method.rstest-files/test_gap_stream_id_band_dynamic_dispatch.ts
…g-running servers (#6608) * fix(streams): recycle Web Streams handle ids so the band survives long-running servers The stream/reader/writer/transform id counter was monotonic over a band only 0x100000 wide and ids were never reused, so a server minting ~48 stream-family ids per request exhausted the band after ~21k requests; past STREAM_ID_BAND_END every band-gated classification path stops recognizing handles (#6602, same failure class as the in-band #6599 bug with a ~20k-request fuse). New streams/idalloc.rs: an id RETIRES when its object reaches a terminal state (readable closed-and-drained/errored, writable closed/errored with no in-flight write, reader/writer releaseLock or death of their terminal stream, transform via its writable side, tee source at unlink, pipeTo lock ids — which never owned a registry entry at all — at release). It then sits in a FIFO quarantine with its registry entry INTACT, keeping full post-terminal semantics for held wrappers; only when the quarantine overflows PERRY_STREAM_ID_QUARANTINE (default 16384) is the oldest id evicted — registry + side tables (byob/tee/transform/expando) cleaned, GC roots dropped — and recycled through a free list. This also fixes the unbounded registry growth: entries used to survive for the process lifetime. Also: tee_error_branches now stamps the orphaned source Errored (it was left Readable forever), and the pipe-lock acquire failure paths recycle their freshly minted ids. Fixes #6602. * style: rustfmt idalloc.rs * fix(streams): address CodeRabbit review on id recycling - Pipe lock ids now carry an ownership mark set atomically at allocation in the allocator (next_pipe_lock_id) and cleared exactly once at retire; the old kind==0 registry probe could race a reused id's alloc→register window and retire a live id. - tee::evict_ids scrubs BOTH directions of a tee relationship: a cancelled branch keeps its links, so a one-directional key removal left the source fan-out pointing at an evicted (later reused) branch id — chunks would land in an unrelated stream. Evicted branch slots zero out so the live sibling keeps receiving; dangling TEE_BRANCH_SOURCE rows drop. - Branches minted from an already-errored source are born terminal with no tee lifecycle — retire them at creation so repeated tee() of an errored stream can't exhaust the band. - Band test takes the allocator serial guard so parallel test runs can't steal the recycle test's free-list id. --------- Co-authored-by: Ralph Küpper <ralph@skelpo.com>
…router Review follow-up: top16 == 0 covered 0.0 and denormal-range doubles, which flowed into the pointer path as null/garbage; 0x7FFC covered booleans (payloads 3/4), which guard_coercible_this passes through to the junk-pointer deref path. Gate the raw-pointer arm on is_plausible_heap_addr and forward only undefined/null (payloads 1/2) for the coercibility TypeError; zero, booleans, and denormals now reach the spec TypeError. Gap test extended with all nine zero/boolean x entries/keys/values cases, byte-identical to node.
Summary
Fixes the deterministic request-12 SIGSEGV in Next.js 16 app-router SSR under Perry (the long-hunted gscmaster "~10-render crash", previously misattributed to a missing GC root).
Root cause. Web Streams handles are raw numeric
f64ids allocated fromSTREAM_ID_BAND_START = 0x100000(one shared counter across the five stream registries). Afor awaitover a render stream resolves@@asyncIteratorthrough the #1545 number-typed stream probe to a boundvaluesre-dispatch, which lands injs_native_call_methodwith the numeric handle as receiver. The #5961 URLSearchParams fast-path block there extractedobject.to_bits() & 0xFFFF_FFFF_FFFFwith no pointer-tag gate, reinterpreting the double's low 48 bits as a heap address.For a stream id
0x100000 + k, those bits decode tok * 2^32:k < 512→ below the macOS heap floor (is_valid_obj_ptr's0x200_0000_0000) → the shape probe bails benignly and the call falls through to the primitive-methods stream dispatch that owns it (why the first 11 requests work).k >= 512→ ≥ 2 TB → passesis_plausible_heap_addr→try_read_gc_headerdereferences unmapped memory →EXC_BAD_ACCESS.A gscmaster render burns ~48 stream-family ids per request, so request 12's render stream (observed k = 526, receiver
1049102.0= bits0x4130_020E_0000_0000, faulting address0x20E00000000) was the first past the threshold. On Linux the heap floor is0x1000, so this same block probes low memory from the very first dynamic stream call (the #6271 "handle-band deref, Linux-only" family).Under
PERRY_GEN_GC=0the same misprobe reads mapped-but-wrong memory instead of faulting → dispatch miss → render abort → the previously-observed "permanent 500s from ~request 11". The GC mode was never the variable; the id counter was.Fix. Gate the URLSearchParams block on a pointer-shaped receiver — NaN-boxed pointer (
0x7FFD) above the handle band, or a raw untagged heap address — mirroring the discipline of the adjacent AbortSignal block. Numeric stream receivers now always fall through to the primitive-methods stream dispatch.Verification
test-files/test_gap_stream_id_band_dynamic_dispatch.ts(new): plain-number receiver with a URLSearchParams-list method name (the exact crashing bit pattern1049102.0), then a 700-id burn + type-erasedfor await+ dynamic.values()past the k=512 threshold. Byte-identical tonode --experimental-strip-types.PERRY_LL_O0_THRESHOLD_BYTES=1per React Flight serializer: symbol falls through the whole typeof chain, re-check at throw site is true — Next.js App Router renders all 500 #6546): previously exactly 11×200 then SIGSEGV at request 12; with this fix sustained 200s past the threshold (see issue thread for the soak numbers).Summary by CodeRabbit
Bug Fixes
keys(),values(), andentries()behavior for arrays, typed collections, and stream-like values.Tests