fix(runtime): pin cross-thread promises in their constructor until they settle (#9552) - #9565
Closed
proggeramlug wants to merge 4 commits into
Closed
fix(runtime): pin cross-thread promises in their constructor until they settle (#9552)#9565proggeramlug wants to merge 4 commits into
proggeramlug wants to merge 4 commits into
Conversation
…ey settle (#9552) A promise minted for a cross-thread settlement — every stdlib fetch/db/ws request, `spawn`, `Atomics.waitAsync` — leaves the runtime as a bare usize inside a worker future and is invisible to every root scanner until its completion is queued back. Nothing on the JS side points AT it either: the awaiting continuation hangs OFF it (`P.on_fulfilled`) and `P.next` is an edge out. The pin was the caller's job; `spawn` and `waitAsync` took it, ~110 stdlib sites (fetch among them) never did. A full collection landing in the in-flight window freed the promise, and the completion then resolved whatever the allocator had reused the slot for — in the report, a RegExp header read as the promise's `next` inside the microtask pump. `js_promise_new_cross_thread` now takes the pin itself (one flag bit on a malloc-resident object it is already writing; the young-pin latch is not consulted) and the settlement paths release it (one byte test on the promise's own cache line). A token dropped without a settlement releases it too. The caller-side pins in `spawn` and `waitAsync` are gone; the stdlib bridge helper is now exactly the constructor. Every place a raw promise address re-enters the runtime from native code (the stdlib pump, the native-async token pump, the thread-result drain) classifies the address first and aborts naming the site and the slot's occupant, so a future rooting hole fails at the boundary instead of as heap corruption cycles later. `scripts/check_cross_thread_promise_provenance.py` (lint) finds arena promises handed to a native settlement sink or captured by a spawn, self-tested with three planted shapes and three clean ones. Claude-Session: https://claude.ai/code/session_01Bok4V8wzgNGmBeE4GPf7Up
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks 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 |
added 3 commits
September 2, 2026 21:31
…lloc-count churn, slot reuse Claude-Session: https://claude.ai/code/session_01Bok4V8wzgNGmBeE4GPf7Up
proggeramlug
pushed a commit
that referenced
this pull request
Sep 2, 2026
…ise literals (#9565 follow-through)
Contributor
Author
|
Landed via merge train #9569 (rebase-merge, authorship preserved), with the three gc test-fixture Promise literals updated for native_pinned as a train fix. |
proggeramlug
pushed a commit
that referenced
this pull request
Sep 2, 2026
This was referenced Sep 2, 2026
proggeramlug
pushed a commit
that referenced
this pull request
Sep 2, 2026
…9565 The #9552 fix landed via merge train #9569 with the first draft of its gap test. That draft — `await fetch()` in a plain async function plus `gc()` — passed 6/6 on the UNFIXED runtime: the collection it forces does not free the in-flight promise, so the gate could not fail. This is the fixture validated on both arms: three consumer shapes (`.then`, async arrow, async class method) start a request from a frame that has returned, `Symbol()` churn trips the malloc-count sweep, `RegExp` headers reuse freed 80-byte slots. Unfixed it hangs 4/4 (two of the three promises freed by malloc sweeps, then a stale resolve on a reused slot); fixed it prints node's `ok,ok,ok 6` 5/5. Also lands the changelog fragment for #9565, which the train did not carry. Claude-Session: https://claude.ai/code/session_01Bok4V8wzgNGmBeE4GPf7Up
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
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.
Fixes #9552.
What was wrong
A promise minted for a cross-thread settlement — every stdlib
fetch/db/ws request,perry/thread'sspawn,Atomics.waitAsync— leaves the runtime as a bareusizeinside a worker future. No root scanner sees it until its completion is queued back, and nothing on the JS side points at it either: the awaiting continuation hangs off it (P.on_fulfilled) andP.nextis an edge out. The constructor's contract said the pin was the caller's job.spawnandwaitAsynctook it; ~110 stdlib sites (fetchamong them) never did.In the report's case (
cc -p <120,000-char argument>, not logged in) an old-generation reclaim at an allocation point ran its malloc sweep whilejs_fetch_with_options's promise was in flight. The sweep freed it (flags=0x0, never pinned, no token, stillPending); mimalloc handed the 80-byte slot to aRegExp; the stdlib pump then resolved the stale address and queued it, and the microtask pump readREGEXP_MAGICas the promise'snext(mov 0x30(%rax)atpump_protected, SIGSEGV withsi_addr=0). The nondeterminism is where the reclaim lands relative to the request. The from-space quarantine correctly reports it as an unrelated fault: the object was never in the arena.Diagnosis was by symbolized build + an env-gated trace of every promise allocation, malloc-sweep free, pin/unpin, and token event; the trace shows
new promise (malloc) → FREED by malloc sweep (state=0, not queued, token=false) → unpin → STALE js_promise_resolvefor the fetch promise, with zero pin events in its lifetime.The fix (one owner, no hot-path cost)
js_promise_new_cross_threadtakes the pin itself: one flag bit on a malloc-resident object it is already writing, viapin_object_non_youngso the copying minor's young-pin latch is never armed.js_promise_resolve/js_promise_rejecttest one byte that lives in the padding afterstate(same cache line, no field moves); an arena promise pays that predictable-branch load and nothing else.remove_token_from_registryreleases it too, so a token dropped without settling cannot leak its promise.spawnandwaitAsyncand thethread::pin_promisehelper are removed; the stdlib'sjs_promise_new_for_native_resolutionis now exactly the constructor. The pump's explicit unpin stays: it still servesspawn_for_promise*, which pins whatever the caller passed.Trust boundary
Everywhere a raw promise address re-enters the runtime from native code — the stdlib pump (both queues), the native-async token pump, the
perry/threadresult drain — now goes throughnative_promise_from_raw, which classifies the address withtry_read_gc_headerand aborts naming the site and the slot's currentobj_type. That runs once per I/O completion, never perawait. A future rooting hole of this shape fails at the boundary with an attributable message instead of as heap corruption cycles later.Scanning for siblings
scripts/check_cross_thread_promise_provenance.py(newlintstep) parses every function inperry-runtime,perry-stdliband theperry-ext-*crates, tracks promise bindings by constructor (with shadowing andlet p = promise as usizealiases), and fails on an arena promise (js_promise_new/js_promise_new_with_parent) reaching a native settlement sink (queue_*_resolution,queue_thread_result,spawn_for_promise*,spawn,spawn_blocking,std::thread::spawn).--self-testplants three bad shapes (direct, alias+spawn,with_parent+thread::spawn) and three clean ones (cross-thread ctor, unrelated usize, shadowed early-return arena promise). The tree is clean today; an early-return arena promise inspawn_implwas the one false positive the shadowing rule exists for.Tests
crates/perry-runtime/src/promise/cross_thread_pin_tests.rs: pinned at creation / released by fulfilment and by rejection; survivesjs_gc_collect()while only an XOR-hidden integer holds it (so a conservative scan cannot make the assertion vacuous); arena promises carry no pin; dropping a token without settling releases the pin;classify_native_promise_addrnames null / live / reused-slot / non-heap. All six pass; the fullperry-runtimesuite is 3005 passed, 0 failed, 4 ignored (RUST_TEST_THREADS=1),perry-stdlib'sasync_bridgetests 3/3.test-files/test_gap_9552_cross_thread_promise_survives_gc.ts: three consumer shapes (.then, async arrow, async class method) each start afetchagainst a local server that answers late, from a frame that has returned; the churn trips the malloc-count sweep (Symbol()) and reuses freed 80-byte slots (RegExpheaders). Unfixed it hangs 4/4 (the env-gated trace shows two of the three promisesFREEDby malloc sweeps atstate=0, thenSTALE js_promise_resolveon a slot now holding an object / nothing); fixed it prints node'sok,ok,ok 65/5. The first draft of this test (await fetch()in a plain async function +gc()) passed 6/6 unfixed and was replaced — a gate that cannot fail is not a gate.Validation (perrymaster,
main3b9c9dee2toolchain,cli_2.1.112.jscompiled with--enable-wasm-runtime, same compiler, only the runtime archives swapped viaPERRY_BUILD_COMMITstamping)C08_long_arg(-p x*120000)ok,ok,ok 65/5 = nodecargo check --release -p perry-runtime -p perry-stdlib --testson currentmain+ this patch: clean.scripts/run_lint_gates.shonmain+ this patch: 61 of 62 pass. The one failure,warnings(cargo check --workspace --all-targetswith-D warnings), is pre-existing on pristinemain(ed99c35cd):pthread_getattr_np/pthread_attr_getstack/pthread_attr_destroyare declared with*mut [u64; 8]ingc/roots.rsand*mut u8inerror_stack_frames.rs(feat(runtime): real function names in Error stacks — frame-pointer walk + the existing name registry (#9486) #9521), whichclashing_extern_declarationsrejects on Linux. Not touched here.https://claude.ai/code/session_01Bok4V8wzgNGmBeE4GPf7Up