fix(gc): disable evacuating minor by default pending #7154 (use-after-free on dynamically-added fields)

changelog.d/7154-gc-disable-evacuating-minor-default.md

@@ -0,0 +1,10 @@
+Disabled the evacuating (moving-loop) minor GC by default, restoring the
+non-moving minor as the default collector. The evacuating minor #7019 made
+default-on has a use-after-free (#7154): a young closure referenced from a
+dynamically-added object field is reclaimed while still live, so a later call
+dies with `TypeError: value is not a function`. This reproduced in the default
+configuration, so the shipped binary could corrupt the heap. The moving-loop
+path is unchanged and still available behind an explicit
+`PERRY_GC_MOVING_LOOP_POLLS=1` opt-in (compile and run). This is a stopgap until
+the root cause tracked in #7154 is fixed; it trades #7019's minor-GC
+RSS/throughput win for correctness.

crates/perry-codegen/src/stmt/loops.rs

@@ -5227,15 +5227,16 @@ fn lower_for_after_init_with_i32_bound(
 fn moving_safepoint_polls_enabled() -> bool {
     use std::sync::OnceLock;
     static CACHED: OnceLock<bool> = OnceLock::new();
-    // DEFAULT ON (moving-nursery flip): emit the back-edge poll, but ONLY for
-    // allocating loops (see the `loop_may_allocate` gate in
-    // `emit_gc_loop_safepoint`) so numeric/vectorizable loops stay call-free.
-    // Kill switch: PERRY_GC_MOVING_LOOP_POLLS=0/off/false. Must match the runtime
-    // `gc_moving_loop_polls_enabled` (same env) so deferrals always have a drain.
+    // DEFAULT OFF (stopgap for #7154): the runtime moving-loop minor this poll
+    // drives has a use-after-free that corrupts the heap even in the default
+    // config, so the default reverts to the non-moving minor and the poll is
+    // emitted only under an explicit PERRY_GC_MOVING_LOOP_POLLS=1/on/true opt-in.
+    // Must match the runtime `gc_moving_loop_polls_enabled` (same env) so a
+    // deferred collection always has a drain and vice versa.
     *CACHED.get_or_init(|| {
-        !matches!(
+        matches!(
             std::env::var("PERRY_GC_MOVING_LOOP_POLLS").as_deref(),
-            Ok("0") | Ok("off") | Ok("false")
+            Ok("1") | Ok("on") | Ok("true")
         )
     })
 }

crates/perry-runtime/src/gc/policy.rs

@@ -359,32 +359,52 @@ pub(crate) fn gc_incremental_enabled() -> bool {
 /// Make the moving minor PRIMARY inside loops: defer the alloc-point nursery
 /// collection to a codegen loop back-edge poll (`js_gc_loop_safepoint`) instead
 /// of collecting non-moving mid-expression, so reallocation-heavy loops evacuate
-/// (bounded RSS) instead of leaking. **DEFAULT ON** as of the moving-nursery flip
-/// — the poll is now emitted only for ALLOCATING loop bodies (`body_may_allocate`
-/// in codegen), so numeric/vectorizable loops stay call-free. Kill switch is an
-/// explicit `PERRY_GC_MOVING_LOOP_POLLS=0`/`off`/`false` (bisection / max-throughput
-/// batch). MUST match codegen `moving_safepoint_polls_enabled` (same env) so the
-/// deferral and the polls that drain it stay coherent — a runtime default-on with
-/// a codegen default-off (or vice versa) would defer collections that never drain.
+/// (bounded RSS) instead of leaking.
+///
+/// **DEFAULT OFF (stopgap for #7154).** This was flipped default-ON in #7019, but
+/// the evacuating minor it makes primary has a use-after-free: a young closure
+/// referenced from a dynamically-added object field (`field[1]`, holders built in
+/// `proxy::create_or_update_receiver_property`) is reclaimed while still live, so
+/// the field dangles and a later call dies with `TypeError: value is not a
+/// function`. This reproduces in the DEFAULT config (no env) — the shipped binary
+/// corrupts the heap. `PERRY_GC_MOVING_LOOP_POLLS=0` is confirmed to eliminate it,
+/// so until #7154 is root-caused we default OFF (restoring the previously-correct
+/// non-moving minor) and keep the moving-loop path behind an explicit
+/// `PERRY_GC_MOVING_LOOP_POLLS=1`/`on`/`true` opt-in. Reverting the default costs
+/// #7019's minor-GC RSS/throughput win but not correctness.
+///
+/// MUST match codegen `moving_safepoint_polls_enabled` (same env) so the deferral
+/// and the polls that drain it stay coherent — a runtime default that disagrees
+/// with the codegen default would defer collections that never drain (or drain
+/// collections that were never deferred).
 pub(crate) fn gc_moving_loop_polls_enabled() -> bool {
     // Test-only mode override (see `force_legacy_gc_pacing`). Consulted BEFORE
-    // the process-wide OnceLock so a single test can pin legacy (non-moving,
-    // budgeted/direct, 128 MiB-ceiling) pacing for its duration even though the
-    // process default is moving-on. Compiled out entirely in release builds.
+    // the process-wide OnceLock so a single test can pin a specific pacing mode
+    // for its duration even though the process default is off. Compiled out
+    // entirely in release builds.
     #[cfg(test)]
     if let Some(forced) = GC_MOVING_LOOP_POLLS_TEST_OVERRIDE.with(Cell::get) {
         return forced;
     }
 
     static CACHED: OnceLock<bool> = OnceLock::new();
     *CACHED.get_or_init(|| {
-        !matches!(
-            std::env::var("PERRY_GC_MOVING_LOOP_POLLS").as_deref(),
-            Ok("0") | Ok("off") | Ok("false")
+        moving_loop_polls_enabled_from_env(
+            std::env::var("PERRY_GC_MOVING_LOOP_POLLS").ok().as_deref(),
         )
     })
 }
 
+/// Pure env→enable decision for the moving-loop minor, factored out so the
+/// default is unit-testable without touching process env / the cached `OnceLock`.
+/// **Default OFF (#7154 stopgap):** an unset var (or any value other than an
+/// explicit opt-in) selects the non-evacuating minor; only `1`/`on`/`true`
+/// enables the moving-loop path. Codegen's `moving_safepoint_polls_enabled`
+/// mirrors this exactly (same env, same predicate).
+pub(crate) fn moving_loop_polls_enabled_from_env(value: Option<&str>) -> bool {
+    matches!(value, Some("1") | Some("on") | Some("true"))
+}
+
 #[cfg(test)]
 thread_local! {
     /// Test-only override for [`gc_moving_loop_polls_enabled`]. When `Some(v)`,

crates/perry-runtime/src/gc/tests/triggers.rs

@@ -429,6 +429,29 @@ fn test_effective_arena_trigger_respects_armed_values() {
     GC_TRIGGER_ARMED.with(|c| c.set(prev_armed));
 }
 
+// #7154 stopgap: the moving-loop (evacuating) minor must be OFF by default.
+// #7019 flipped it default-on, but the evacuating minor has a use-after-free
+// that corrupts the heap in the default config, so the default must select the
+// non-evacuating minor; the moving path stays reachable only via an explicit
+// PERRY_GC_MOVING_LOOP_POLLS=1/on/true opt-in.
+#[test]
+fn test_moving_loop_minor_off_by_default_7154() {
+    use super::super::policy::moving_loop_polls_enabled_from_env as enabled;
+    // Default (unset) is non-evacuating.
+    assert!(!enabled(None), "moving-loop minor must be OFF by default (#7154)");
+    // Kill-switch values remain off.
+    assert!(!enabled(Some("0")));
+    assert!(!enabled(Some("off")));
+    assert!(!enabled(Some("false")));
+    // Unknown / garbage values fall back to the safe default (off).
+    assert!(!enabled(Some("")));
+    assert!(!enabled(Some("2")));
+    // Explicit opt-in enables the moving path.
+    assert!(enabled(Some("1")));
+    assert!(enabled(Some("on")));
+    assert!(enabled(Some("true")));
+}
+
 // #6184: the OS memory-pressure entry must run a real collection when the
 // thread is at a safe point, and must lower+arm the arena trigger.
 #[test]