fix(worker): root the awaited handler Promise across perry_poll - #15
Conversation
await_promise cached a raw Promise pointer and then called perry_poll in a loop. perry_poll runs JS, which reaches GC safepoints, and an evacuating minor MOVES the Promise -- after which the cached address is stale from-space memory and js_promise_state reads recycled bytes. Caught precisely with PERRY_GC_PROTECT_FROMSPACE: [gc-fromspace-protect] FAULT: signal 10 This address is RETIRED FROM-SPACE. The evacuating minor moved or freed the object here and the holder kept the pre-collection address. last-known object: obj_type=5 size=72 (5 = GC_TYPE_PROMISE) The faulting instruction IS the stale use. Re-deriving the pointer from the NaN-boxed value each turn does not help -- the box holds the same pre-collection address. Root it in Perry's FFI root scope and re-read the slot each turn, so the collector rewrites it. An RAII guard pops the scope on every exit path, including the timeout return. A/B on the same fixture and seed (PERRY_GC_SCHEDULE_SEED=12345 PERRY_GC_SCHEDULE_RATE=0.5), instrument armed both times: before: 116 copying minors -> FAULT (obj_type=5) after: 107 copying minors -> 0 faults, no TypeError Refs PerryTS/perry#8546. That issue is NOT fully closed by this: with two Next.js apps the remaining failures are now NAMED ("_onUserlandLoaded is not a function"), indicating further holders of the same class.
ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Free Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthrough
ChangesPromise GC Rooting
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This change roots the awaited Promise across garbage-collection safepoints and re-reads it safely during polling; no actionable merge-blocking risk remains beyond normal checks and review. Note 🎁 Summarized by CodeRabbit FreeYour organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/settings/billing. Comment |
…-promise # Conflicts: # crates/coop-worker/src/plugin_host.rs # crates/coop-worker/src/runtime_libraries.rs
The bug
await_promiseextracted a raw Promise pointer from the NaN-boxed handlerresult and then called
perry_pollin a loop:perry_pollruns JS, which reaches GC safepoints, and an evacuating minormoves the Promise. From then on
promise_ptris a stale from-space addressand
js_promise_statereads recycled memory.How it was found
PERRY_GC_PROTECT_FROMSPACE=1faults at the stale use, not cycles later:obj_type=5isGC_TYPE_PROMISE, and the backtrace lands inDeploymentHost::load_with_optionson the app's Perry thread.The fix
Re-deriving the pointer from
promise_valueeach turn does not help — theNaN-box holds the same pre-collection address. Only a root the collector
rewrites is stable.
Perry already exports the FFI root scope for exactly this
(
js_ffi_root_scope_enter/exit,js_ffi_root_push_nanbox,js_ffi_root_get_nanbox). Root the Promise for the duration of the await andre-read the slot every turn. An RAII guard pops the scope on every exit path,
including the
?and timeout returns.Verification
Same fixture, same seed (
PERRY_GC_SCHEDULE_SEED=12345 PERRY_GC_SCHEDULE_RATE=0.5 PERRY_GC_PROTECT_FROMSPACE=1), instrument armed in both arms — a run with zerocopying minors would protect nothing, so the minor count is the anti-vacuity
check:
cargo check -p coop-workerclean;cargo fmtclean.Scope — this does NOT close PerryTS/perry#8546
Hosting two Next.js applications
in_processstill fails. What changed is thatthe failures are now named rather than opaque:
_onUserlandLoadedis a Node bootstrap internal, so there is at least one moreholder of this same class (a JS value cached across something that can run JS).
This PR removes one verified instance; #8546 stays open for the rest.
Summary by CodeRabbit