diff --git a/changelog.d/9570-release-gates.md b/changelog.d/9570-release-gates.md new file mode 100644 index 0000000000..f30c91a66a --- /dev/null +++ b/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. diff --git a/changelog.d/PENDINGIGN-ignore-preexisting.md b/changelog.d/PENDINGIGN-ignore-preexisting.md deleted file mode 100644 index d721d1f51c..0000000000 --- a/changelog.d/PENDINGIGN-ignore-preexisting.md +++ /dev/null @@ -1,26 +0,0 @@ -**test: ignore two pre-existing `cargo-test-perry` failures (#9377, #9378)** - -`full-suite-gate` requires `cargo-test-perry`, and `failure`/`cancelled` both -fail it — so two long-standing bugs were blocking every release cut despite -having nothing to do with the release. - -Both were confirmed pre-existing with **clean builds** (an incremental target dir -across checkouts produces false verdicts here, so every data point is -`cargo clean` first): - -| test | Aug-31 pin `83754818ea` | current pin | + #9372/#9375 | -|---|---|---|---| -| `degenerate_then_chain_survives_combinator` | FAILED | FAILED | FAILED | -| `native_compile_skips_link_on_identical_second_build` | FAILED | FAILED | FAILED | - -Both reproduce on macOS as well as `ubuntu-latest`, and both are independent of -#9226 (the source of the two regressions fixed in #9372 and #9375). - -They went unnoticed because these shards run **only in the full tier** — never -on `main` — and in the Aug-31 full tier six of eight shards died early (shard 7 -ran 3 of 35 test binaries), so many tests produced no verdict at all. An absent -result reads as health. - -Each `#[ignore]` names its issue and states the defect, so re-enabling is a -one-line change once fixed. This is a deliberate, reversible coverage trade to -unblock a release from bugs that predate it — not a fix. diff --git a/crates/perry-runtime/src/promise/combinators.rs b/crates/perry-runtime/src/promise/combinators.rs index eb0a4afb6e..c88c4429ca 100644 --- a/crates/perry-runtime/src/promise/combinators.rs +++ b/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}; diff --git a/crates/perry/tests/native_link_cache.rs b/crates/perry/tests/native_link_cache.rs index c4b93ee5dd..b4a58cef23 100644 --- a/crates/perry/tests/native_link_cache.rs +++ b/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); diff --git a/crates/perry/tests/promise_reaction_slot_overflow.rs b/crates/perry/tests/promise_reaction_slot_overflow.rs index 5f0540ab99..67480cd3a4 100644 --- a/crates/perry/tests/promise_reaction_slot_overflow.rs +++ b/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" + ); +}