fix(gc): prune DATA_VIEW_REGISTRY and SHARED_ARRAY_BUFFER_REGISTRY on buffer death (#6337)

crates/perry-runtime/src/buffer/header.rs

@@ -161,6 +161,19 @@ pub fn is_data_view(addr: usize) -> bool {
     DATA_VIEW_REGISTRY.with(|r| r.borrow().contains(&addr))
 }
 
+/// Live entry counts for the two registries the GC buffer sweep prunes (#6337).
+/// Test-only: the leak regression asserts these DRAIN after the owning buffers
+/// are collected, which a per-address `is_*` probe cannot show.
+#[cfg(test)]
+pub(crate) fn test_data_view_registry_len() -> usize {
+    DATA_VIEW_REGISTRY.with(|r| r.borrow().len())
+}
+
+#[cfg(test)]
+pub(crate) fn test_shared_array_buffer_registry_len() -> usize {
+    SHARED_ARRAY_BUFFER_REGISTRY.with(|r| r.borrow().len())
+}
+
 /// Register a buffer pointer in the thread-local registry
 pub fn register_buffer(ptr: *const BufferHeader) {
     BUFFER_REGISTRY.with(|r| r.borrow_mut().insert(ptr as usize));
@@ -455,16 +468,49 @@ pub(crate) fn collect_dead_registered_buffers_post_trace(full_trace: bool) -> Ve
     if !full_trace {
         return Vec::new();
     }
+    // Lock the process-global SAB registry ONCE for the whole scan rather than
+    // once per registered buffer (see `registered_buffer_is_dead_post_trace`).
+    // `None` — nearly every process — means no SAB was ever allocated.
+    let shared_sabs = crate::shared_sab::snapshot_shared_sabs();
     BUFFER_REGISTRY.with(|r| {
         r.borrow()
             .iter()
             .copied()
-            .filter(|&addr| unsafe { registered_buffer_is_dead_post_trace(addr) })
+            .filter(|&addr| unsafe {
+                registered_buffer_is_dead_post_trace(addr, shared_sabs.as_ref())
+            })
             .collect()
     })
 }
 
-unsafe fn registered_buffer_is_dead_post_trace(addr: usize) -> bool {
+unsafe fn registered_buffer_is_dead_post_trace(
+    addr: usize,
+    shared_sabs: Option<&std::collections::HashSet<usize>>,
+) -> bool {
+    // A process-global `SharedArrayBuffer` backing is NOT a GC allocation:
+    // `shared_sab::alloc_shared_sab` takes it straight from `alloc_zeroed`, it
+    // carries no `GcHeader`, and it is never freed (#4913 — that is what lets
+    // the same bytes alias across `perry/thread` agents). But
+    // `js_shared_array_buffer_new` DOES `register_buffer` it, so it lands in
+    // `BUFFER_REGISTRY` and reaches this scan on every full trace.
+    //
+    // `try_read_gc_header` below would then read the 8 bytes BEFORE the malloc
+    // block — the allocator's own metadata — and interpret them as a `GcHeader`:
+    // one arbitrary byte compared against `GC_TYPE_BUFFER` (10), the next
+    // against the mark/pin/forward bits. A chance match declares a LIVE,
+    // never-freed SAB dead, and `finalize_collected_dead_buffer` then runs on
+    // it — including `view::remove_entries_for_dead_buffer`, which retains on
+    // `info.backing != addr` and so unregisters EVERY live typed-array view
+    // over that SAB. Those views are exactly how cross-agent `Atomics`
+    // wait/notify resolve their absolute slot addresses.
+    //
+    // So: veto first, and never sniff a header the object does not have. The
+    // set is snapshotted once per scan by the caller and is `None` for the
+    // processes that never allocate a SAB — nearly all of them — so the common
+    // path here is a single null check.
+    if shared_sabs.is_some_and(|sabs| sabs.contains(&addr)) {
+        return false;
+    }
     let Some(header) = crate::value::addr_class::try_read_gc_header(addr) else {
         return false;
     };
@@ -487,6 +533,34 @@ pub(crate) fn finalize_collected_dead_buffer(addr: usize) {
     ARRAY_BUFFER_REGISTRY.with(|r| {
         r.borrow_mut().remove(&addr);
     });
+    // #6337: the two sibling buffer-identity registries were missing from this
+    // list — they had no `.remove`/`.retain` site anywhere in the tree. Like
+    // the three above they are plain address-keyed sets that never rooted the
+    // `BufferHeader`, so a collected view left its entry behind forever:
+    //
+    //  * an unbounded leak — one permanent entry per `DataView` (and per
+    //    SAB-flagged buffer) ever created;
+    //  * the #6080 ABA class this function exists to prevent —
+    //    `arena_reset_empty_blocks` resets a fully-empty block's offset to 0
+    //    while KEEPING its base pointer, so a reset block re-issues the same
+    //    addresses. A recycled address then inherits the dead view's identity:
+    //    `is_data_view`/`is_shared_array_buffer` gate `util.types.isDataView`/
+    //    `isSharedArrayBuffer`, `ArrayBuffer.isView`, the `[object DataView]`
+    //    tag, and the structuredClone/`.slice()` re-marking above — an
+    //    unrelated fresh Buffer landing there would answer to all of them.
+    //
+    // Only GC-heap buffers reach here. A process-global SAB backing is never
+    // freed and is vetoed as a dead candidate in
+    // `registered_buffer_is_dead_post_trace`, so the entries pruned from
+    // SHARED_ARRAY_BUFFER_REGISTRY are the arena-allocated SAB-flagged copies
+    // (`SharedArrayBuffer.prototype.slice`, structuredClone) — the ones that
+    // genuinely die and whose addresses genuinely get recycled.
+    SHARED_ARRAY_BUFFER_REGISTRY.with(|r| {
+        r.borrow_mut().remove(&addr);
+    });
+    DATA_VIEW_REGISTRY.with(|r| {
+        r.borrow_mut().remove(&addr);
+    });
     BUFFER_AB_ALIAS.with(|r| {
         r.borrow_mut().remove(&addr);
     });

crates/perry-runtime/src/buffer/mod.rs

@@ -43,6 +43,8 @@ pub use header::{
 pub(crate) use header::{
     collect_dead_registered_buffers_post_trace, finalize_collected_dead_buffer,
 };
+#[cfg(test)]
+pub(crate) use header::{test_data_view_registry_len, test_shared_array_buffer_registry_len};
 
 // ---- Re-exports: ArrayBuffer detach / transfer (ES2024) ----
 // `detach_array_buffer` dereferences the raw address it is given, so it stays

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

@@ -0,0 +1,236 @@
+//! Death pruning for the buffer-identity side tables (#6337).
+//!
+//! `finalize_collected_dead_buffer` exists to drop "every registry/side-table
+//! entry keyed by a dead buffer's address" — its own doc comment cites
+//! preventing the #6080 ABA class. But `DATA_VIEW_REGISTRY` and
+//! `SHARED_ARRAY_BUFFER_REGISTRY` were never pruned anywhere in the tree: they
+//! had zero `.remove`/`.retain` sites. That leaked one permanent entry per
+//! `DataView` / SAB-flagged buffer ever created, and let a recycled address
+//! (`arena_reset_empty_blocks` resets a fully-empty block's offset to 0 while
+//! KEEPING its base pointer) inherit a dead view's identity.
+//!
+//! The SharedArrayBuffer side has a wrinkle the DataView side does not: a
+//! *process-global* SAB backing (`shared_sab::alloc_shared_sab`) is not a GC
+//! allocation at all and is never freed, so it must be vetoed as a dead
+//! candidate rather than pruned — see
+//! `test_seeded_shared_sab_is_never_a_dead_buffer_candidate`.
+
+use super::super::*;
+use super::support::*;
+
+fn full_gc() {
+    let _ =
+        gc_collect_full_mark_sweep_with_trigger(GcTriggerSnapshot::capture(GcTriggerKind::Direct));
+}
+
+/// Drops a test-seeded `shared_sab` entry even if the test panics — the
+/// registry is process-global and never cleared, so a leaked seed would make
+/// every later test in this binary treat that address as a live SAB backing.
+struct SeededSharedSabGuard(usize);
+
+impl Drop for SeededSharedSabGuard {
+    fn drop(&mut self) {
+        crate::shared_sab::test_unseed_shared_sab(self.0);
+    }
+}
+
+/// A dead `DataView`'s `DATA_VIEW_REGISTRY` entry must go. `is_data_view` gates
+/// `util.types.isDataView`, `ArrayBuffer.isView`, the `[object DataView]` tag,
+/// and the structuredClone / `.slice()` re-marking paths — a fresh Buffer
+/// landing on the recycled address would answer to all of them.
+#[test]
+fn test_dead_data_view_registry_entry_pruned_on_full_gc() {
+    let _guard = GcTestIsolationGuard::new();
+
+    let addr = crate::buffer::buffer_alloc(32) as usize;
+    crate::buffer::mark_as_data_view(addr);
+    assert!(
+        crate::buffer::is_data_view(addr),
+        "test premise: the view is registered"
+    );
+
+    // No roots: dead at the full trace. (Buffers are TENURED old-gen residents,
+    // so only a FULL trace can prove them dead.)
+    full_gc();
+
+    assert!(
+        !crate::buffer::is_data_view(addr),
+        "dead buffer must not keep its DataView identity — a recycled address \
+         would answer to util.types.isDataView / ArrayBuffer.isView"
+    );
+}
+
+/// The SAB-flagged buffers that actually die are the arena-allocated copies:
+/// `SharedArrayBuffer.prototype.slice` (`object/buffer_dispatch.rs`) and
+/// structuredClone (`builtins/globals.rs`) both `buffer_alloc` a fresh
+/// `BufferHeader` and re-`mark_as_shared_array_buffer` it. Those are ordinary
+/// GC-heap objects whose addresses get recycled.
+#[test]
+fn test_dead_shared_array_buffer_flag_pruned_on_full_gc() {
+    let _guard = GcTestIsolationGuard::new();
+
+    let addr = crate::buffer::buffer_alloc(32) as usize;
+    crate::buffer::mark_as_shared_array_buffer(addr);
+    assert!(
+        crate::buffer::is_shared_array_buffer(addr),
+        "test premise: the SAB flag is registered"
+    );
+
+    full_gc();
+
+    assert!(
+        !crate::buffer::is_shared_array_buffer(addr),
+        "dead SAB-flagged buffer must not keep its identity — a recycled \
+         address would answer to util.types.isSharedArrayBuffer"
+    );
+}
+
+/// The safety inverse: a LIVE (rooted) view keeps its flag. Full mark-sweep is
+/// non-moving, so the rooted buffers keep their addresses.
+///
+/// `CopyingNurseryTestGuard::new(2)` — not `GcTestIsolationGuard` — because
+/// only it pushes the shadow frame that makes `js_shadow_slot_set` an actual
+/// root. Under the plain isolation guard the slot writes land in no frame, the
+/// buffers stay unreachable, and the test would "pass" for the wrong reason.
+#[test]
+fn test_live_data_view_and_shared_array_buffer_flags_survive_full_gc() {
+    let _guard = CopyingNurseryTestGuard::new(2);
+
+    let view = crate::buffer::buffer_alloc(32) as usize;
+    crate::buffer::mark_as_data_view(view);
+    let sab = crate::buffer::buffer_alloc(32) as usize;
+    crate::buffer::mark_as_shared_array_buffer(sab);
+
+    js_shadow_slot_set(0, ptr_bits(view));
+    js_shadow_slot_set(1, ptr_bits(sab));
+
+    full_gc();
+
+    assert!(
+        crate::buffer::is_data_view(view),
+        "a live (rooted) DataView must keep its registry entry"
+    );
+    assert!(
+        crate::buffer::is_shared_array_buffer(sab),
+        "a live (rooted) SAB-flagged buffer must keep its registry entry"
+    );
+}
+
+/// The leak regression: N views, all references dropped, one full collection,
+/// and both registries must DRAIN. A per-address `is_*` probe cannot show this
+/// — before the fix the tables grew monotonically for the life of the process,
+/// one permanent entry per `DataView` / SAB-flagged buffer ever created.
+#[test]
+fn test_data_view_and_sab_registries_drain_after_owners_die() {
+    let _guard = GcTestIsolationGuard::new();
+
+    const N: usize = 4096;
+    let base_views = crate::buffer::test_data_view_registry_len();
+    let base_sabs = crate::buffer::test_shared_array_buffer_registry_len();
+
+    for _ in 0..N {
+        let view = crate::buffer::buffer_alloc(16) as usize;
+        crate::buffer::mark_as_data_view(view);
+        let sab = crate::buffer::buffer_alloc(16) as usize;
+        crate::buffer::mark_as_shared_array_buffer(sab);
+    }
+
+    assert_eq!(
+        crate::buffer::test_data_view_registry_len(),
+        base_views + N,
+        "test premise: every DataView registered"
+    );
+    assert_eq!(
+        crate::buffer::test_shared_array_buffer_registry_len(),
+        base_sabs + N,
+        "test premise: every SAB-flagged buffer registered"
+    );
+
+    // Every one of them is unreachable — no shadow slot, no global root.
+    full_gc();
+
+    assert_eq!(
+        crate::buffer::test_data_view_registry_len(),
+        base_views,
+        "DATA_VIEW_REGISTRY must drain when its buffers are swept (it had no \
+         .remove site at all before #6337)"
+    );
+    assert_eq!(
+        crate::buffer::test_shared_array_buffer_registry_len(),
+        base_sabs,
+        "SHARED_ARRAY_BUFFER_REGISTRY must drain when its buffers are swept"
+    );
+}
+
+/// A process-global `SharedArrayBuffer` backing must NEVER be treated as a
+/// collectable GC object.
+///
+/// `alloc_shared_sab` takes the block straight from `alloc_zeroed`: no
+/// `GcHeader`, never freed (that is what lets the bytes alias across
+/// `perry/thread` agents, #4913). But `js_shared_array_buffer_new` DOES
+/// `register_buffer` it, so it reaches the dead-buffer scan on every full
+/// trace, where `try_read_gc_header` would sniff the 8 bytes BEFORE the malloc
+/// block and read the allocator's own metadata as a header — one arbitrary byte
+/// against `GC_TYPE_BUFFER` (10), the next against the mark/pin/forward bits.
+///
+/// A chance match declares a LIVE SAB dead, and `finalize_collected_dead_buffer`
+/// then runs `view::remove_entries_for_dead_buffer`, which retains on
+/// `info.backing != addr` — silently unregistering EVERY live typed-array view
+/// over that SAB, i.e. exactly the views cross-agent `Atomics` wait/notify
+/// resolve their absolute slot addresses through.
+///
+/// That coincidence cannot be forced from a test without writing outside the
+/// allocation, so seed an ordinary GC buffer — whose real header genuinely says
+/// "dead" — into the SAB registry. It reproduces the same decision
+/// deterministically: without the veto this buffer is reported dead.
+#[test]
+fn test_seeded_shared_sab_is_never_a_dead_buffer_candidate() {
+    let _guard = GcTestIsolationGuard::new();
+
+    let addr = crate::buffer::buffer_alloc(32) as usize;
+    crate::buffer::mark_as_shared_array_buffer(addr);
+    crate::shared_sab::test_seed_shared_sab(addr);
+    let _seeded = SeededSharedSabGuard(addr);
+
+    // Unrooted, and its real GcHeader is unmarked — the scan would list it.
+    let dead = crate::buffer::collect_dead_registered_buffers_post_trace(true);
+
+    assert!(
+        !dead.contains(&addr),
+        "a process-global SAB backing must never be reported dead: it has no \
+         GcHeader and is never freed, and finalizing it would unregister every \
+         live typed-array view over it (breaking cross-agent Atomics)"
+    );
+}
+
+/// End-to-end companion to the veto test: a real `new SharedArrayBuffer(n)`
+/// backing survives a full collection with no roots at all, and stays
+/// recognisable through every predicate — the thread-local flag, the buffer
+/// registry, and the process-global registry the cross-thread serializer and
+/// `Atomics` futex keying depend on.
+#[test]
+fn test_process_global_sab_backing_survives_full_gc_unrooted() {
+    let _guard = GcTestIsolationGuard::new();
+
+    let buf = crate::buffer::js_shared_array_buffer_new(64);
+    let addr = buf as usize;
+    assert!(crate::shared_sab::is_shared_sab(addr));
+
+    // Deliberately unrooted. The backing is never freed, so this must be a
+    // no-op for it — a SAB is reachable from other agents, not from this
+    // thread's roots.
+    full_gc();
+
+    assert!(
+        crate::shared_sab::is_shared_sab(addr),
+        "the process-global SAB registry must still recognise the backing"
+    );
+    assert!(
+        crate::buffer::is_shared_array_buffer(addr),
+        "the SAB must still answer util.types.isSharedArrayBuffer"
+    );
+    assert!(
+        crate::buffer::is_registered_buffer(addr),
+        "the SAB must still answer as a registered buffer"
+    );
+}

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

@@ -1,6 +1,7 @@
 mod alloc;
 mod barrier;
 mod budgeted_step_api;
+mod buffer_side_tables;
 mod contract;
 mod copying;
 mod copying_side_tables;