Merge train: #9570 #9571 (release gates re-enabled; honest #9552 fixture)

changelog.d/9565-cross-thread-promise-pin.md

@@ -0,0 +1,53 @@
+### 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. The
+  gap test hangs 4/4 on the unfixed tree (an env-gated trace shows two of its
+  three fetch promises freed by malloc sweeps, then `js_promise_resolve` on a
+  reused slot) and prints node's output 5/5 fixed; `perry-runtime`'s suite is
+  3005 passed / 0 failed single-threaded.

changelog.d/9570-release-gates.md

@@ -0,0 +1,6 @@
+### Fixed
+
+- `Promise.all` now preserves registration order when its input already has `.then()` reactions, while retaining the allocation-free fast path for reaction-free promises.
+- The native link-cache regression test now isolates dependency invalidation
+  from legitimate cross-module specialization, restoring its cache-hit
+  coverage in the full release gate.

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

@@ -68,6 +68,23 @@ pub(super) fn attach_promise_all_state(promise: *mut Promise, state: PromiseAllS
         return;
     }
     mark_rejection_handled(promise);
+
+    // The direct PromiseAllState table is allocation-free, but it is a
+    // separate queue from the promise's ordinary reaction slot. If a reaction
+    // is already registered, parking the state here would let Promise.all
+    // overtake that earlier reaction when the promise settles. Join the
+    // ordered overflow path in that uncommon case; a promise with no prior
+    // reaction keeps the direct fast path below.
+    let has_prior_reaction = unsafe {
+        !(*promise).on_fulfilled.is_null()
+            || !(*promise).on_rejected.is_null()
+            || !(*promise).next.is_null()
+    };
+    if has_prior_reaction {
+        attach_promise_all_after_prior_reaction(promise, state);
+        return;
+    }
+
     let mut queued = false;
     unsafe {
         match (*promise).state {
@@ -928,37 +945,7 @@ pub extern "C" fn js_promise_all(promises_arr: *const crate::array::ArrayHeader)
             state_arr,
             index: i,
         };
-
-        unsafe {
-            match (*promise_ptr).state {
-                PromiseState::Fulfilled => {
-                    TASK_QUEUE.with(|q| {
-                        q.borrow_mut().push_back(Task::PromiseAll(
-                            state,
-                            (*promise_ptr).value,
-                            true,
-                            context_for_promise(promise_ptr),
-                        ));
-                    });
-                }
-                PromiseState::Rejected => {
-                    TASK_QUEUE.with(|q| {
-                        q.borrow_mut().push_back(Task::PromiseAll(
-                            state,
-                            (*promise_ptr).reason,
-                            false,
-                            context_for_promise(promise_ptr),
-                        ));
-                    });
-                }
-                PromiseState::Pending => {
-                    PROMISE_ALL_STATES.with(|states| {
-                        states.borrow_mut().push(promise_ptr as usize, state);
-                    });
-                    set_promise_callback_context(promise_ptr);
-                }
-            }
-        }
+        attach_promise_all_state(promise_ptr, state);
     }
 
     let remaining = js_array_get_f64(state_arr, 0);
@@ -970,6 +957,78 @@ pub extern "C" fn js_promise_all(promises_arr: *const crate::array::ArrayHeader)
     result_promise
 }
 
+/// Attach a Promise.all element after reactions already registered on the
+/// input promise. The closures enter the same overflow list as later `.then`
+/// calls, preserving registration order without penalizing the empty-slot fast
+/// path above.
+fn attach_promise_all_after_prior_reaction(promise: *mut Promise, state: PromiseAllState) {
+    use crate::closure::{
+        js_closure_alloc, js_closure_set_capture_f64, js_closure_set_capture_ptr,
+    };
+
+    // Both closure allocations may collect. Root every pointer that is stored
+    // afterwards, including the first closure across allocation of the second.
+    let scope = crate::gc::RuntimeHandleScope::new();
+    let promise_h = scope.root_nanbox_f64(crate::value::js_nanbox_pointer(promise as i64));
+    let result_h =
+        scope.root_nanbox_f64(crate::value::js_nanbox_pointer(state.result_promise as i64));
+    let results_h =
+        scope.root_nanbox_f64(crate::value::js_nanbox_pointer(state.results_arr as i64));
+    let state_h = scope.root_nanbox_f64(crate::value::js_nanbox_pointer(state.state_arr as i64));
+    let fulfill_h = scope.root_nanbox_f64(crate::value::js_nanbox_pointer(js_closure_alloc(
+        promise_all_ordered_fulfill_handler as *const u8,
+        4,
+    ) as i64));
+    let reject_h = scope.root_nanbox_f64(crate::value::js_nanbox_pointer(js_closure_alloc(
+        promise_all_ordered_reject_handler as *const u8,
+        2,
+    ) as i64));
+
+    let ptr_of =
+        |h: &crate::gc::RuntimeHandle<'_>| crate::value::js_nanbox_get_pointer(h.get_nanbox_f64());
+    let fulfill = ptr_of(&fulfill_h) as *mut crate::closure::ClosureHeader;
+    js_closure_set_capture_ptr(fulfill, 0, ptr_of(&result_h));
+    js_closure_set_capture_ptr(fulfill, 1, ptr_of(&results_h));
+    js_closure_set_capture_ptr(fulfill, 2, ptr_of(&state_h));
+    js_closure_set_capture_f64(fulfill, 3, state.index as f64);
+
+    let reject = ptr_of(&reject_h) as *mut crate::closure::ClosureHeader;
+    js_closure_set_capture_ptr(reject, 0, ptr_of(&result_h));
+    js_closure_set_capture_ptr(reject, 1, ptr_of(&state_h));
+
+    js_promise_attach_handlers(ptr_of(&promise_h) as *mut Promise, fulfill, reject);
+}
+
+extern "C" fn promise_all_ordered_fulfill_handler(
+    closure: *const crate::closure::ClosureHeader,
+    value: f64,
+) -> f64 {
+    use crate::closure::{js_closure_get_capture_f64, js_closure_get_capture_ptr};
+    promise_all_fulfill_direct(
+        PromiseAllState {
+            result_promise: js_closure_get_capture_ptr(closure, 0) as *mut Promise,
+            results_arr: js_closure_get_capture_ptr(closure, 1) as *mut crate::array::ArrayHeader,
+            state_arr: js_closure_get_capture_ptr(closure, 2) as *mut crate::array::ArrayHeader,
+            index: js_closure_get_capture_f64(closure, 3) as u32,
+        },
+        value,
+    );
+    0.0
+}
+
+extern "C" fn promise_all_ordered_reject_handler(
+    closure: *const crate::closure::ClosureHeader,
+    reason: f64,
+) -> f64 {
+    use crate::closure::js_closure_get_capture_ptr;
+    promise_all_reject_direct(
+        js_closure_get_capture_ptr(closure, 0) as *mut Promise,
+        js_closure_get_capture_ptr(closure, 1) as *mut crate::array::ArrayHeader,
+        reason,
+    );
+    0.0
+}
+
 #[inline]
 fn promise_all_fulfill_direct(state: PromiseAllState, value: f64) {
     use crate::array::{js_array_get_f64, js_array_set_f64};

crates/perry/tests/native_link_cache.rs

@@ -97,10 +97,6 @@ fn assert_codegen_cache(
 }
 
 #[test]
-#[ignore = "#9378: a dependency change invalidates its dependents' codegen objects, so the \
-           expected 1 cache hit is 0. Pre-existing — fails identically at the Aug-31 \
-           release pin (83754818ea) on a clean build, and is unrelated to #9226. \
-           Ignored to unblock a release it does not belong to; see #9378 to re-enable."]
 fn native_compile_skips_link_on_identical_second_build() {
     let dir = tempfile::tempdir().expect("tempdir");
     let project = dir.path();
@@ -113,16 +109,12 @@ fn native_compile_skips_link_on_identical_second_build() {
         "{\"name\":\"link-cache-test\"}\n",
     )
     .unwrap();
-    fs::write(
-        src.join("util.ts"),
-        "export function answer(): number { return 41; }\n",
-    )
-    .unwrap();
-    fs::write(
-        src.join("main.ts"),
-        "import { answer } from './util';\nconsole.log(answer() + 1);\n",
-    )
-    .unwrap();
+    // Keep the changed module behind a side-effect import. A direct imported
+    // function call can be specialized into the consumer, in which case a
+    // body edit legitimately changes both modules' post-transform HIR and
+    // cannot prove the one-hit/one-miss object-cache contract below.
+    fs::write(src.join("util.ts"), "console.log('util-41');\n").unwrap();
+    fs::write(src.join("main.ts"), "import './util';\nconsole.log(42);\n").unwrap();
 
     let entry = src.join("main.ts");
     let output = dist.join("app");
@@ -132,13 +124,13 @@ fn native_compile_skips_link_on_identical_second_build() {
     assert_build_cache_miss(&first, "manifest-missing");
     assert_codegen_cache(&first, 0, 2, 0, 2, 0);
     let first_bytes = fs::read(&output).expect("first output");
-    assert_eq!(run_binary(&output).trim(), "42");
+    assert_eq!(run_binary(&output).trim(), "util-41\n42");
 
     let second = compile_json(project, &entry, &output);
     assert_skipped(&second);
     assert_build_cache_hit(&second);
     assert_eq!(fs::read(&output).expect("second output"), first_bytes);
-    assert_eq!(run_binary(&output).trim(), "42");
+    assert_eq!(run_binary(&output).trim(), "util-41\n42");
 
     fs::write(
         project.join("package.json"),
@@ -147,7 +139,7 @@ fn native_compile_skips_link_on_identical_second_build() {
     .unwrap();
     let config_changed = compile_json(project, &entry, &output);
     assert_build_cache_miss(&config_changed, "config");
-    assert_eq!(run_binary(&output).trim(), "42");
+    assert_eq!(run_binary(&output).trim(), "util-41\n42");
 
     let env_changed = compile_json_with_env(project, &entry, &output, &[("PERRY_DEBUG_INIT", "1")]);
     assert_linked(&env_changed);
@@ -156,22 +148,18 @@ fn native_compile_skips_link_on_identical_second_build() {
     let env_restored = compile_json(project, &entry, &output);
     assert_linked(&env_restored);
     assert_build_cache_miss(&env_restored, "env");
-    assert_eq!(run_binary(&output).trim(), "42");
+    assert_eq!(run_binary(&output).trim(), "util-41\n42");
 
     let warm_again = compile_json(project, &entry, &output);
     assert_skipped(&warm_again);
     assert_build_cache_hit(&warm_again);
 
-    fs::write(
-        src.join("util.ts"),
-        "export function answer(): number { return 40; }\n",
-    )
-    .unwrap();
+    fs::write(src.join("util.ts"), "console.log('util-40');\n").unwrap();
     let changed = compile_json(project, &entry, &output);
     assert_linked(&changed);
     assert_build_cache_miss(&changed, "source");
     assert_codegen_cache(&changed, 1, 1, 1, 1, 0);
-    assert_eq!(run_binary(&output).trim(), "41");
+    assert_eq!(run_binary(&output).trim(), "util-40\n42");
 
     fs::remove_file(&output).unwrap();
     let missing_output = compile_json(project, &entry, &output);

crates/perry/tests/promise_reaction_slot_overflow.rs

@@ -211,10 +211,6 @@ t.then((v) => console.log("then:", v));
 /// instead of the value (CodeRabbit finding on the initial version of this
 /// fix).
 #[test]
-#[ignore = "#9377: a bare p.then() pass-through chain resolves AFTER a later combinator \
-           (values correct, order reversed). Pre-existing — fails identically at the \
-           Aug-31 release pin (83754818ea) on a clean build. Ignored to unblock a \
-           release it does not belong to; see #9377 to re-enable."]
 fn degenerate_then_chain_survives_combinator() {
     let dir = tempfile::tempdir().expect("tempdir");
     let stdout = compile_and_run(
@@ -233,3 +229,26 @@ resolveP(9);
         "a bare p.then() pass-through chain must survive a later combinator"
     );
 }
+
+/// The inverse registration order must retain the allocation-free
+/// PromiseAllState path and still run the combinator before the later bare
+/// `.then()` chain.
+#[test]
+fn promise_all_before_degenerate_then_keeps_registration_order() {
+    let dir = tempfile::tempdir().expect("tempdir");
+    let stdout = compile_and_run(
+        dir.path(),
+        r#"
+let resolveP: any;
+const p = new Promise((r) => { resolveP = r; });
+Promise.all([p]).then(([v]) => console.log("all:", v));
+const chain = p.then();
+chain.then((v) => console.log("chain:", v));
+resolveP(9);
+"#,
+    );
+    assert_eq!(
+        stdout, "all: 9\nchain: 9\n",
+        "Promise.all registered first must keep its earlier reaction order"
+    );
+}