Commit bfa0f25
fix(runtime): promise reactions must not clobber the single handler slot (#5867)
* fix(runtime): #5437 — .finally() on a settled promise dispatches its own wrapper exactly once
The finally wrappers were stored into the promise's single-valued handler
slots even when the promise was already settled, alongside a queued
Task::Promise that re-reads the slots at drain time. A settled promise
with more than one attached reaction (p.catch(cb); p.finally(end) — or N
.finally()s on one promise: Turbopack's loadChunkAsync returns a shared
pre-fulfilled loadedChunk constant and Next's CacheSignal attaches
.finally(endRead) per chunk load) dispatched whichever wrapper was stored
last once per queued task: one callback ran twice (or N times), the others
never. Next.js's cache-read count went negative, cacheReady() never
resolved, the prerender was never aborted (renderSignal), the deliberately
hanging params/searchParams promises never rejected, and the dynamic-SSR
routes parked forever with zero bytes.
Settled attaches now enqueue Task::Inline carrying their own wrapper (next
owned by the wrapper, so the runner does not settle it); the handler slots
are only used for pending promises.
Claude-Session: https://claude.ai/code/session_01RcePwqv92QidGakrfYvf3v
* fix(runtime): promise combinator/finally reactions must not clobber the inline handler slot
js_promise_attach_handlers (Promise.all/allSettled/race/any via
promise_resolve_for_combinator, stream adapters) and js_promise_finally's
PENDING arm stored reactions into the promise's single-valued handler
slots unconditionally, destroying whatever reaction was already there:
p.then(cb); Promise.all([p]) lost cb; two combinators sharing one pending
input destroyed each other's forwarder (the loser's remaining-count never
reached zero — permanent hang, the shared webpack/Turbopack chunk-promise
shape); p.then(cb); p.finally(end) lost cb AND its chained promise.
Mirror js_promise_then's occupancy protocol: the first reaction takes the
inline slot, later ones divert to PROMISE_OVERFLOW_REACTIONS (pending) or
dispatch as their own Task::Inline (settled), with null next where the
reaction owns its own settlement.
e2e suite promise_reaction_slot_overflow.rs: 6 scenarios byte-for-byte vs
node --experimental-strip-types.
* review(coderabbit): occupancy must include next; settled .finally captures attach-time context
1. A degenerate no-arg p.then() parks with both handler slots null and
only next set. All three occupancy checks (js_promise_then itself,
js_promise_attach_handlers, the .finally pending arm) now treat a
non-null next as occupied — the spec combinators attach per-element
reactions through js_promise_then via invoke_then, so
'const c = p.then(); Promise.all([p])' stranded c's chain.
New e2e: degenerate_then_chain_survives_combinator.
NOTE: suite rerun pending (build paused for CPU handover).
2. The settled .finally arms no longer store into the promise context
slot, so context_for_promise could return an earlier reaction's
context — capture_context() at attach time instead.
* lint: allowlist promise/then.rs at 2008 lines (8 over) — split tracked as follow-up
Same pattern as the body_stmt.rs entry: a correctness PR (#5867) pushes
the file marginally over the 2000-line gate; the topical split (the
#1545 value-read thunks + spec finally-wrapper tail, ~790 lines, into a
sibling module) is deferred to a focused follow-up rather than folded
into this fix.
* chore(release): v0.5.1216 — version bump + changelog for #5867
---------
Co-authored-by: Ralph Küpper <ralph@skelpo.com>1 parent c7feba7 commit bfa0f25
7 files changed
Lines changed: 468 additions & 109 deletions
File tree
- crates
- perry-runtime/src/promise
- perry/tests
- scripts
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
1 | 5 | | |
2 | 6 | | |
3 | 7 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
8 | 8 | | |
9 | 9 | | |
10 | 10 | | |
11 | | - | |
| 11 | + | |
12 | 12 | | |
13 | 13 | | |
14 | 14 | | |
| |||
0 commit comments