Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,16 @@ jobs:
python3 scripts/gc_pin_sites.py --self-test
python3 scripts/gc_pin_sites.py

# #9552. A promise handed to native code as a bare address is invisible
# to every root scanner until its completion is queued back. The
# cross-thread constructor pins it for that window; the arena constructor
# cannot. This finds arena promises reaching a native settlement sink.
- name: Cross-thread promise provenance (#9552)
if: ${{ !cancelled() }}
run: |
python3 scripts/check_cross_thread_promise_provenance.py --self-test
python3 scripts/check_cross_thread_promise_provenance.py

# #7231. A runtime-side table holding a GC pointer IS a root, and nothing
# static could see that class before: gc_root_dominance_check.py reads
# emitted LLVM IR and a thread_local is not in it. #7226, #7239, #7268 and
Expand Down
49 changes: 49 additions & 0 deletions changelog.d/9565-cross-thread-promise-pin.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
### Fixed

- **A promise handed to native code is now rooted until it settles; `fetch`
(and ~110 other stdlib hand-offs) could be freed mid-flight (#9552).**
`claude -p <120,000-char argument>` compiled with perry died with
SIGSEGV in the microtask pump (`pump_protected`, `si_addr=0`) where node
exits 1; nondeterministic, ~50% of runs.

A promise minted by `js_promise_new_cross_thread` leaves the runtime as a
bare `usize` inside a worker future, which no root scanner visits, and
nothing on the JS side points **at** a pending promise whose only consumer
is an `await` — `P.on_fulfilled` and `P.next` are edges out of it. The
constructor's contract made the pin the caller's job; `spawn` and
`Atomics.waitAsync` took it, the stdlib's `fetch`/db/ws sites never did.
An old-generation reclaim at an allocation point ran its malloc sweep while
`js_fetch_with_options`'s promise was in flight and freed it (never
pinned, no token, still pending); mimalloc gave the 80-byte slot to a
`RegExp`; the stdlib pump resolved the stale address and the pump then
read `REGEXP_MAGIC` as the promise's `next`. The from-space quarantine
reports it as an unrelated fault because the object was never in the
arena. Diagnosed with a symbolized build and an env-gated trace of every
promise allocation, malloc-sweep free, pin and token event.

The constructor now owns the invariant: `js_promise_new_cross_thread`
pins the (malloc-resident, non-moving) promise itself — one flag bit,
through `pin_object_non_young` so the copying minor's young-pin latch is
never armed — and `js_promise_resolve` / `js_promise_reject` release it
with one byte test on a field in the padding after `state` (no other
field moves; an arena promise pays a predictable-branch load and nothing
else). `remove_token_from_registry` releases it too, so a native-async
token dropped without settling cannot leak its promise. The caller-side
pins in `spawn` and `waitAsync` are gone.

Every place a raw promise address re-enters the runtime from native code
— the stdlib pump, the native-async token pump, the `perry/thread`
result drain — now classifies it (`native_promise_from_raw`) and aborts
naming the site and the slot's current occupant, once per I/O completion,
never per `await`. `scripts/check_cross_thread_promise_provenance.py` is
a new `lint` step: it fails on an **arena** promise reaching a native
settlement sink or a spawned closure (shadowing- and alias-aware,
self-tested with three planted and three clean shapes). Unit coverage in
`promise/cross_thread_pin_tests.rs` (pinned at creation, released by both
settlements, survives `js_gc_collect()` while only an XOR-hidden integer
holds it, token-drop release, address classification); gap test
`test_gap_9552_cross_thread_promise_survives_gc.ts` fetches from a local
server that answers late while collections are forced in between.

Validation on the report's binary (`cli_2.1.112.js`, `--enable-wasm-runtime`,
same compiler, only the runtime archives swapped): 4/5 crashes → 0/12.
6 changes: 2 additions & 4 deletions crates/perry-runtime/src/atomics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -768,11 +768,9 @@ pub extern "C" fn js_atomics_wait_async(
// Cross-thread variant: referenced only by a raw usize in the pending
// results queue until drained — must not live in the copying nursery
// (the from-space flip ignores pins that no scanner reaches).
// #9552: the cross-thread constructor pins the promise until it settles;
// this only has to keep the event loop alive until the result lands.
let promise = crate::promise::js_promise_new_cross_thread();
// Pin the promise + keep the event loop alive until the async result lands.
unsafe {
crate::thread::pin_promise(promise);
}
crate::thread::thread_job_begin();
let promise_usize = promise as usize;
// #6185: the promise belongs to the agent calling `waitAsync`. The futex
Expand Down
2 changes: 1 addition & 1 deletion crates/perry-runtime/src/gc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ mod pin;
pub(crate) use pin::test_reset_young_pin_latch;
pub use pin::{
copied_minor_preflight_skips, copied_minor_preflight_walks, pin_object, pin_object_non_young,
unpin_object,
pin_user_ptr_non_young, unpin_object, unpin_user_ptr,
};
use pin::{note_preflight_skipped, note_preflight_walked, young_pin_latch_armed};
/// Software prefetch helpers for the collector's pointer-chasing loops
Expand Down
25 changes: 24 additions & 1 deletion crates/perry-runtime/src/gc/pin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@
use std::cell::Cell;
use std::sync::atomic::{AtomicBool, AtomicU64, Ordering};

use super::types::{GcHeader, GC_FLAG_ARENA, GC_FLAG_PINNED};
use super::types::{GcHeader, GC_FLAG_ARENA, GC_FLAG_PINNED, GC_HEADER_SIZE};

crate::perry_thread_local! {
static COPYING_WALK_PHASE: Cell<Option<&'static str>> =
Expand Down Expand Up @@ -266,6 +266,29 @@ pub(crate) unsafe fn pin_constrains_copying_minor_for_tests(header: *mut GcHeade
/// # Safety
///
/// As [`pin_object`].
/// Pin the non-young object whose USER pointer is `user_ptr` (#9552).
///
/// The address arithmetic lives here, next to the flag it serves, so callers
/// that hold a `*mut Promise` (or any other user pointer) do not each grow a
/// bare `GcHeader` cast. Malloc-resident and old-arena objects only: the
/// young-pin latch is deliberately not consulted (see `pin_object_non_young`).
#[inline]
pub unsafe fn pin_user_ptr_non_young(user_ptr: *mut u8) {
if user_ptr.is_null() {
return;
}
pin_object_non_young(user_ptr.sub(GC_HEADER_SIZE) as *mut GcHeader);
}

/// Release the pin on the object whose USER pointer is `user_ptr` (#9552).
#[inline]
pub unsafe fn unpin_user_ptr(user_ptr: *mut u8) {
if user_ptr.is_null() {
return;
}
unpin_object(user_ptr.sub(GC_HEADER_SIZE) as *mut GcHeader);
}

#[inline]
pub unsafe fn unpin_object(header: *mut GcHeader) {
if header.is_null() {
Expand Down
1 change: 1 addition & 0 deletions crates/perry-runtime/src/gc/tests/alloc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -640,6 +640,7 @@ fn alloc_malloc_kind_test_object(obj_type: u8) -> *mut u8 {
ptr as *mut crate::promise::Promise,
crate::promise::Promise {
state: crate::promise::PromiseState::Pending,
native_pinned: 0,
value: 0.0,
reason: 0.0,
on_fulfilled: std::ptr::null(),
Expand Down
2 changes: 2 additions & 0 deletions crates/perry-runtime/src/gc/tests/support.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ pub(super) unsafe fn alloc_old_test_promise() -> *mut crate::promise::Promise {
ptr,
crate::promise::Promise {
state: crate::promise::PromiseState::Pending,
native_pinned: 0,
value: 0.0,
reason: 0.0,
on_fulfilled: std::ptr::null(),
Expand Down Expand Up @@ -785,6 +786,7 @@ pub(super) fn allocate_dead_malloc_churn_headers(per_type: usize) -> Vec<usize>
ptr,
crate::promise::Promise {
state: crate::promise::PromiseState::Pending,
native_pinned: 0,
value: 0.0,
reason: 0.0,
on_fulfilled: std::ptr::null(),
Expand Down
128 changes: 128 additions & 0 deletions crates/perry-runtime/src/promise/cross_thread_pin_tests.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
//! #9552 — a promise whose address leaves the runtime as a bare `usize` (a
//! worker future, a pending-result queue, a native async token) is pinned by
//! `js_promise_new_cross_thread` and released by its settlement. These pin the
//! constructor/settlement contract and the trust boundary that classifies a
//! returning address.

use super::native_async::{
js_native_async_completion_new, js_native_async_completion_promise,
js_native_async_drop_promise_token, native_async_promise_has_token, test_native_async_lock,
test_reset_native_async_registry,
};
use super::{
classify_native_promise_addr, js_promise_new, js_promise_new_cross_thread, js_promise_reject,
js_promise_resolve, NativePromiseAddr, Promise,
};
use crate::value::addr_class::try_read_gc_header;

fn gc_flags(promise: *mut Promise) -> u8 {
unsafe { try_read_gc_header(promise as usize) }
.expect("a freshly minted promise is a tracked heap object")
.gc_flags
}

fn pinned(promise: *mut Promise) -> bool {
gc_flags(promise) & crate::gc::GC_FLAG_PINNED != 0
}

#[test]
fn cross_thread_promise_is_pinned_at_creation_and_released_by_fulfilment() {
let _guard = test_native_async_lock();
let promise = js_promise_new_cross_thread();
assert_eq!(
gc_flags(promise) & crate::gc::GC_FLAG_ARENA,
0,
"cross-thread promises are malloc-resident"
);
assert!(pinned(promise), "#9552: the constructor takes the pin");
assert_eq!(unsafe { (*promise).native_pinned }, 1);

js_promise_resolve(promise, 1.0);
assert!(!pinned(promise), "settlement releases the pin");
assert_eq!(unsafe { (*promise).native_pinned }, 0);

// A second settlement is a no-op on an already-settled promise and must
// not touch the pin state.
js_promise_reject(promise, 2.0);
assert!(!pinned(promise));
}

#[test]
fn rejection_releases_the_pin_too() {
let _guard = test_native_async_lock();
let promise = js_promise_new_cross_thread();
assert!(pinned(promise));
js_promise_reject(promise, 2.0);
assert!(!pinned(promise));
assert_eq!(unsafe { (*promise).native_pinned }, 0);
}

#[test]
fn cross_thread_promise_survives_a_full_collection_while_only_native_code_holds_it() {
let _guard = test_native_async_lock();
// Hold the address the way a worker future does: as a bare integer no
// root scanner visits. XOR-hide it so a conservative stack scan (if one
// were to run) cannot keep the object alive by accident and make the
// assertion vacuous.
const MASK: usize = 0x5555_5555_5555_5555;
let hidden = (js_promise_new_cross_thread() as usize) ^ MASK;
crate::gc::js_gc_collect();
let raw = hidden ^ MASK;
match classify_native_promise_addr(raw) {
NativePromiseAddr::Live(promise) => {
assert!(pinned(promise), "still pinned while in flight");
js_promise_resolve(promise, 3.0);
assert!(!pinned(promise));
}
other => panic!("#9552: in-flight promise did not survive the collection: {other:?}"),
}
}

#[test]
fn arena_promises_carry_no_pin() {
let _guard = test_native_async_lock();
let promise = js_promise_new();
assert!(!pinned(promise));
assert_eq!(unsafe { (*promise).native_pinned }, 0);
js_promise_resolve(promise, 1.0);
assert!(!pinned(promise));
}

#[test]
fn dropping_a_token_without_settling_releases_the_pin() {
let _guard = test_native_async_lock();
test_reset_native_async_registry();
let token = js_native_async_completion_new(0);
let promise = js_native_async_completion_promise(token);
assert!(pinned(promise), "token promises are cross-thread promises");
assert!(native_async_promise_has_token(promise));
// The token was the promise's root; once it is gone the pin must not keep
// a never-settling promise alive forever.
js_native_async_drop_promise_token(promise);
assert!(!native_async_promise_has_token(promise));
assert!(!pinned(promise));
assert_eq!(unsafe { (*promise).native_pinned }, 0);
}

#[test]
fn classify_native_promise_addr_names_null_live_and_reused_slots() {
let _guard = test_native_async_lock();
assert_eq!(classify_native_promise_addr(0), NativePromiseAddr::Null);
let promise = js_promise_new_cross_thread();
assert_eq!(
classify_native_promise_addr(promise as usize),
NativePromiseAddr::Live(promise)
);
// A malloc-resident object of another type where a promise used to be.
let occupant = crate::gc::gc_malloc(64, crate::gc::GC_TYPE_STRING) as usize;
assert_eq!(
classify_native_promise_addr(occupant),
NativePromiseAddr::WrongType(crate::gc::GC_TYPE_STRING)
);
// Not a heap object at all.
assert_eq!(
classify_native_promise_addr(0x10),
NativePromiseAddr::NotAHeapObject
);
js_promise_resolve(promise, 0.0);
}
72 changes: 72 additions & 0 deletions crates/perry-runtime/src/promise/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ pub mod assimilate;
pub mod async_step;
pub mod checked_dispatch;
pub mod combinators;
#[cfg(test)]
mod cross_thread_pin_tests;
pub(crate) mod keyed_table;
pub mod microtasks;
pub mod native_async;
Expand Down Expand Up @@ -532,6 +534,12 @@ pub type ClosurePtr = *const crate::closure::ClosureHeader;
pub struct Promise {
/// Current state of the promise
pub(crate) state: PromiseState,
/// #9552 — non-zero while this promise holds the cross-thread pin taken by
/// `js_promise_new_cross_thread`. Lives in the padding after `state`, so no
/// other field moves. Cleared, and the pin released, by the settlement
/// paths (`js_promise_resolve` / `js_promise_reject`) and by
/// `remove_token_from_registry` for a token dropped without settling.
pub(crate) native_pinned: u8,
/// The resolved value (if fulfilled)
pub(crate) value: f64,
/// The rejection reason (if rejected)
Expand Down Expand Up @@ -565,6 +573,7 @@ impl Promise {
pub(crate) fn new() -> Self {
Promise {
state: PromiseState::Pending,
native_pinned: 0,
value: 0.0,
reason: 0.0,
on_fulfilled: ptr::null(),
Expand Down Expand Up @@ -1264,3 +1273,66 @@ pub extern "C" fn js_microtasks_pending() -> i32 {
}
TASK_QUEUE.with(|q| if q.borrow().is_empty() { 0 } else { 1 })
}

/// #9552 — what a raw promise address handed back by native code names.
#[derive(Debug, PartialEq, Eq)]
pub enum NativePromiseAddr {
/// A null hand-off (a caller that never minted a promise).
Null,
/// A live promise.
Live(*mut Promise),
/// Not a tracked heap object at all (freed and unmapped, or never one).
NotAHeapObject,
/// A heap object of another type: the promise was freed and its slot
/// reused. The payload is the occupant's `obj_type`.
WrongType(u8),
}

/// Classify `addr` without dereferencing anything the heap does not vouch
/// for. Pure, so the abort policy in [`native_promise_from_raw`] is testable.
pub fn classify_native_promise_addr(addr: usize) -> NativePromiseAddr {
if addr == 0 {
return NativePromiseAddr::Null;
}
match unsafe { crate::value::addr_class::try_read_gc_header(addr) } {
None => NativePromiseAddr::NotAHeapObject,
Some(header) if header.obj_type == crate::gc::GC_TYPE_PROMISE => {
NativePromiseAddr::Live(addr as *mut Promise)
}
Some(header) => NativePromiseAddr::WrongType(header.obj_type),
}
}

/// The trust boundary for a promise address that left the runtime as a bare
/// `usize` (a worker future, a pending-result queue, a native async token) and
/// is now coming back to be settled (#9552).
///
/// A stale address here is a use-after-free in the making: `js_promise_resolve`
/// would write a state byte and a value into whatever the allocator has since
/// put in the slot, and the corruption surfaces cycles later in an unrelated
/// object (the #9552 report was a RegExp header read as a promise's `next`).
/// Aborting at the boundary names the site and the occupant instead. This runs
/// once per native completion — never per `await` — so it is not on any hot
/// path.
pub fn native_promise_from_raw(addr: usize, site: &str) -> *mut Promise {
match classify_native_promise_addr(addr) {
NativePromiseAddr::Null => ptr::null_mut(),
NativePromiseAddr::Live(promise) => promise,
NativePromiseAddr::NotAHeapObject => {
eprintln!(
"[perry] FATAL (#9552): {site} handed back promise address {addr:#x}, which is \
not a tracked heap object — the promise was freed while native code still \
held its address. It was not rooted across its in-flight window."
);
std::process::abort()
}
NativePromiseAddr::WrongType(obj_type) => {
eprintln!(
"[perry] FATAL (#9552): {site} handed back promise address {addr:#x}, but the \
object there now has obj_type={obj_type} — the promise was freed while native \
code still held its address and the slot was reused."
);
std::process::abort()
}
}
}
Loading
Loading