fix(gc): explicit gc() forces the conservative native-stack scan (#4977)

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

@@ -229,17 +229,10 @@ pub fn gc_build_v8_heap_snapshot_json() -> String {
     // Approximate the live set: collect first so unreachable malloc
     // objects are freed and fully-dead nursery blocks are reset before
     // the walk picks the population (Node's writeHeapSnapshot also
-    // forces a full GC). Force the conservative native-stack scan for
-    // this collection: in the default `auto` mode a full collect can
-    // miss top-level locals held only on the native stack and reclaim
-    // live objects (pre-existing explicit-`gc()` bug, see #4977 —
-    // repro: `const k = {nested:{deep:"s"}}; gc();` corrupts
-    // `k.nested.deep`). A diagnostics API must never make that worse.
-    let prev = super::roots::set_conservative_stack_scan_override(Some(
-        super::roots::ConservativeStackScanMode::Full,
-    ));
+    // forces a full GC). `js_gc_collect` forces the conservative
+    // native-stack scan when no per-thread override is pinned (#4977),
+    // so top-level locals held only on the native stack survive.
     js_gc_collect();
-    super::roots::set_conservative_stack_scan_override(prev);
 
     // Free-list slots are dead-but-unreclaimed space inside live
     // blocks; exclude them from the dump.

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

@@ -552,10 +552,7 @@ pub(super) fn flush_deferred_gc_request() {
             if manual_gc_blocked_by_unsafe_zone() {
                 return;
             }
-            crate::weakref::clear_pending_finalization_jobs();
-            gc_collect_inner_with_trigger(GcTriggerSnapshot::capture(GcTriggerKind::Manual))
-                .emit_after_current();
-            crate::weakref::queue_pending_finalization_callbacks_after_gc();
+            manual_gc_collect_now();
         }
         DeferredGcRequest::Collect(kind) => {
             if gc_blocked_by_unsafe_zone() {
@@ -1551,6 +1548,14 @@ pub extern "C" fn js_gc_collect() {
     if defer_gc_request(DeferredGcRequest::Collect(GcTriggerKind::Manual)) {
         return;
     }
+    manual_gc_collect_now();
+}
+
+/// Run an explicit (`gc()`) full collection. The `gc()` callsite may hold live
+/// module-init/top-level locals only on the native stack, so the collection
+/// forces the conservative native-stack scan (#4977); see `ManualGcScanGuard`.
+fn manual_gc_collect_now() {
+    let _scan = super::roots::ManualGcScanGuard::force_full_scan();
     crate::weakref::clear_pending_finalization_jobs();
     gc_collect_inner_with_trigger(GcTriggerSnapshot::capture(GcTriggerKind::Manual))
         .emit_after_current();

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

@@ -178,7 +178,7 @@ thread_local! {
     /// set this to `Auto` (skip) inside their controlled-root scopes so a forced
     /// collection still reclaims the objects they hold only as native-stack
     /// locals; see `ScopedRootScannerRegistryGuard`.
-    static CONSERVATIVE_STACK_SCAN_OVERRIDE: std::cell::Cell<Option<ConservativeStackScanMode>> =
+    pub(super) static CONSERVATIVE_STACK_SCAN_OVERRIDE: std::cell::Cell<Option<ConservativeStackScanMode>> =
         const { std::cell::Cell::new(None) };
 }
 
@@ -190,6 +190,42 @@ pub(crate) fn set_conservative_stack_scan_override(
     CONSERVATIVE_STACK_SCAN_OVERRIDE.with(|c| c.replace(mode))
 }
 
+/// Scoped guard forcing the conservative native-stack scan for an explicit
+/// `gc()` collection (#4977). In the default `Auto` mode a full collection
+/// skips the native scan, but at a `gc()` callsite live module-init/top-level
+/// locals may be held only on the native stack — neither the precise
+/// shadow-stack roots nor the module-var scanners cover them — so the
+/// collector reclaimed live object graphs and later field reads returned
+/// dangling-pointer garbage. An already-pinned per-thread override wins (the
+/// GC unit tests pin `Auto` so a forced collection still reclaims objects they
+/// hold only as native-stack locals), and an explicit
+/// `PERRY_CONSERVATIVE_STACK_SCAN` env value beats any override either way,
+/// so the bisection escape hatch keeps working.
+pub(super) struct ManualGcScanGuard {
+    engaged: bool,
+}
+
+impl ManualGcScanGuard {
+    pub(super) fn force_full_scan() -> Self {
+        let engaged = CONSERVATIVE_STACK_SCAN_OVERRIDE.with(|c| {
+            if c.get().is_some() {
+                return false;
+            }
+            c.set(Some(ConservativeStackScanMode::Full));
+            true
+        });
+        Self { engaged }
+    }
+}
+
+impl Drop for ManualGcScanGuard {
+    fn drop(&mut self) {
+        if self.engaged {
+            CONSERVATIVE_STACK_SCAN_OVERRIDE.with(|c| c.set(None));
+        }
+    }
+}
+
 pub(super) fn conservative_stack_scan_mode() -> ConservativeStackScanMode {
     // Explicit env var wins (so the dedicated mode tests and ops bisection keep
     // working), then a per-thread override, then the build-time default.

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

@@ -805,3 +805,38 @@ fn test_shadow_stack_restore_drops_orphaned_bound_slots() {
     js_shadow_frame_pop(run_frame);
     assert_eq!(shadow_stack_depth(), 0);
 }
+
+#[test]
+fn manual_gc_scan_guard_forces_full_scan_only_when_unpinned() {
+    use crate::gc::roots::{ManualGcScanGuard, CONSERVATIVE_STACK_SCAN_OVERRIDE};
+
+    let prev = set_conservative_stack_scan_override(None);
+
+    // Unpinned: the guard engages a Full override for its lifetime (#4977 —
+    // explicit gc() must see top-level locals held only on the native stack).
+    {
+        let _scan = ManualGcScanGuard::force_full_scan();
+        assert_eq!(
+            CONSERVATIVE_STACK_SCAN_OVERRIDE.with(|c| c.get()),
+            Some(ConservativeStackScanMode::Full)
+        );
+    }
+    assert_eq!(CONSERVATIVE_STACK_SCAN_OVERRIDE.with(|c| c.get()), None);
+
+    // Pinned (the GC unit-test guards pin Auto so forced collections still
+    // reclaim native-stack locals): the guard must not replace the override.
+    set_conservative_stack_scan_override(Some(ConservativeStackScanMode::Auto));
+    {
+        let _scan = ManualGcScanGuard::force_full_scan();
+        assert_eq!(
+            CONSERVATIVE_STACK_SCAN_OVERRIDE.with(|c| c.get()),
+            Some(ConservativeStackScanMode::Auto)
+        );
+    }
+    assert_eq!(
+        CONSERVATIVE_STACK_SCAN_OVERRIDE.with(|c| c.get()),
+        Some(ConservativeStackScanMode::Auto)
+    );
+
+    set_conservative_stack_scan_override(prev);
+}

test-files/test_issue_4977_gc_toplevel_locals.ts

@@ -0,0 +1,16 @@
+// #4977: explicit gc() in default auto stack-scan mode reclaimed live
+// top-level locals — string fields read back as dangling-pointer garbage.
+class Widget {
+  x = 1;
+  constructor(public name: string) {}
+}
+
+const keep = { nested: { deep: "leaf-string-4916" } };
+const w = new Widget("widget-name-4977");
+
+gc();
+
+console.log(keep.nested.deep.length);
+console.log(keep.nested.deep);
+console.log(w.name);
+console.log(w.x);