Skip to content

Commit b4654ac

Browse files
proggeramlugRalph Küpper
andauthored
fix(gc): clear the raw-handle debt regression and ratchet 1006 → 1003 (Layer 3) (#7455)
* fix(gc): clear the raw-handle debt regression and ratchet to 1003 The Layer 3 debt ratchet has been RED on main since #7424, which added two get_raw_mut_ptr reads to the proxy splice path: 1006 -> 1008, and the baseline may only move DOWN, so the gate could not be satisfied by re-pinning. Both #7424 sites are correct — they re-derive the receiver after a trap that can move it. The ratchet counts them because they spell that as a manual call/re-read pair rather than the combinator the RFC prescribes. One converts exactly; the other is a terminal read with no allocating call to wrap. Converted five call/re-read pairs to across_{mut,const}: array/push_pop.rs proxy splice: trap then re-derive promise/then.rs (x2) js_promise_new_with_parent then re-read object/async_generator_queue.rs js_promise_new then re-read string/append.rs js_string_from_bytes_with_capacity then re-read Each is semantically identical — across_* runs the call and returns the post-collection address — but the ordering becomes structural instead of conventional, which is the whole point of #7341: every fix in that family was an ordering bug, not a missing root. 1008 -> 1003; baseline locked at 1003. 1705 lib tests pass. * docs: changelog fragment for #7455 --------- Co-authored-by: Ralph Küpper <ralph@skelpo.com>
1 parent c060070 commit b4654ac

6 files changed

Lines changed: 27 additions & 13 deletions

File tree

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
- **The Layer 3 raw-handle debt ratchet is green again, at a lower number.** It had been red on `main` since #7424 took it 1006 → 1008, and its baseline may only move *down*, so re-pinning was not an option. Both #7424 sites are correct — they re-derive a receiver after a proxy trap that can move it — but they spell that as a manual call/re-read pair rather than the `RuntimeHandle::across_*` combinator the RFC prescribes. Five such pairs are now converted (`array/push_pop.rs`, `promise/then.rs` ×2, `object/async_generator_queue.rs`, `string/append.rs`); each is semantically identical, but the ordering becomes structural instead of conventional — which is the point of #7341, where every fix in the family was an ordering bug rather than a missing root. 1008 → 1003, baseline locked at 1003. (#7455)

crates/perry-runtime/src/array/push_pop.rs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -443,10 +443,14 @@ pub(super) fn proxy_array_mutator(
443443
for k in 0..actual_delete_count {
444444
let from = (actual_start + k).to_string();
445445
if proxy_has_str_key(p(), from.as_bytes()) {
446-
let v = proxy_get_str_key(p(), from.as_bytes());
447-
// Re-derive `removed` from its handle: the traps above
448-
// run arbitrary JS, which can move it.
449-
let removed = removed_handle.get_raw_mut_ptr::<ArrayHeader>();
446+
// The trap runs arbitrary JS and can move `removed`, so
447+
// its address is only valid after the call. `across_mut`
448+
// is that pattern as one combinator: it runs the call and
449+
// hands back the post-collection address, so a stale
450+
// pointer is never bound in between (#7341).
451+
let (v, removed) = removed_handle.across_mut::<ArrayHeader, _>(|| {
452+
proxy_get_str_key(p(), from.as_bytes())
453+
});
450454
let elems = (removed as *mut u8).add(std::mem::size_of::<ArrayHeader>())
451455
as *mut f64;
452456
// GC_STORE_AUDIT(BARRIERED): note_array_slot re-stores

crates/perry-runtime/src/object/async_generator_queue.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -268,8 +268,10 @@ fn async_generator_request(closure: *const ClosureHeader, arg: f64, kind: Reques
268268
let scope = crate::gc::RuntimeHandleScope::new();
269269
let original_handle = scope.root_raw_const_ptr(original);
270270
let arg_handle = scope.root_nanbox_f64(arg);
271-
let promise = js_promise_new();
272-
let original = original_handle.get_raw_const_ptr::<ClosureHeader>();
271+
// `across_const` pairs the allocating call with the re-read, so the
272+
// closure pointer cannot be bound stale in between (#7341).
273+
let (promise, original) =
274+
original_handle.across_const::<ClosureHeader, _>(|| js_promise_new());
273275
let arg = arg_handle.get_nanbox_f64();
274276
STATES.with(|states| {
275277
if let Some(state) = states.borrow_mut().get_mut(state_id - 1) {

crates/perry-runtime/src/promise/then.rs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -488,8 +488,10 @@ pub extern "C" fn js_promise_then(
488488
let promise_handle = scope.root_raw_mut_ptr(promise);
489489
let on_fulfilled_handle = scope.root_raw_const_ptr(on_fulfilled);
490490
let on_rejected_handle = scope.root_raw_const_ptr(on_rejected);
491-
let next = js_promise_new_with_parent(promise);
492-
let promise = promise_handle.get_raw_mut_ptr::<Promise>();
491+
// `across_mut` runs the allocating call and returns the post-collection
492+
// address, so the receiver is never bound stale in between (#7341).
493+
let (next, promise) =
494+
promise_handle.across_mut::<Promise, _>(|| js_promise_new_with_parent(promise));
493495
let on_fulfilled = on_fulfilled_handle.get_raw_const_ptr::<crate::closure::ClosureHeader>();
494496
let on_rejected = on_rejected_handle.get_raw_const_ptr::<crate::closure::ClosureHeader>();
495497

@@ -764,8 +766,10 @@ pub extern "C" fn js_promise_finally(
764766
let scope = crate::gc::RuntimeHandleScope::new();
765767
let promise_handle = scope.root_raw_mut_ptr(promise);
766768
let on_finally_handle = scope.root_raw_const_ptr(on_finally);
767-
let next = js_promise_new_with_parent(promise);
768-
let promise = promise_handle.get_raw_mut_ptr::<Promise>();
769+
// See the sibling in `then`: the allocating call and the re-read are one
770+
// combinator so the order cannot drift apart (#7341).
771+
let (next, promise) =
772+
promise_handle.across_mut::<Promise, _>(|| js_promise_new_with_parent(promise));
769773
let on_finally = on_finally_handle.get_raw_const_ptr::<crate::closure::ClosureHeader>();
770774
let next_i64 = next as i64;
771775

crates/perry-runtime/src/string/append.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,11 @@ pub extern "C" fn js_string_append(
3939
let scope = crate::gc::RuntimeHandleScope::new();
4040
let src_handle = scope.root_string_ptr(src);
4141
let src_blen = unsafe { (*src).byte_len };
42-
let new_ptr = js_string_from_bytes_with_capacity(ptr::null(), 0, src_blen);
43-
let src = src_handle.get_raw_const_ptr::<StringHeader>();
42+
// `across_const` pairs the allocating call with the re-read, so the
43+
// source pointer cannot be bound stale in between (#7341).
44+
let (new_ptr, src) = src_handle.across_const::<StringHeader, _>(|| {
45+
js_string_from_bytes_with_capacity(ptr::null(), 0, src_blen)
46+
});
4447
if is_valid_string_ptr(src) {
4548
unsafe {
4649
let src_data = string_data(src);
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
1006
1+
1003

0 commit comments

Comments
 (0)