fix(runtime): Set/Map forEach — fused array-receiver reroute + delete-during-iteration

crates/perry-runtime/src/array/iter_methods.rs

@@ -118,6 +118,26 @@ pub extern "C" fn js_array_forEach(arr: *const ArrayHeader, callback: *const Clo
         );
         return;
     }
+    // #5989: `.forEach` on an unknown-typed receiver is statically fused to
+    // this array entry point, but the receiver may be a native Set/Map —
+    // react-server-dom iterates `request.abortableTasks` (a Set read back off
+    // the request object) exactly this way. Treating a SetHeader as an
+    // ArrayHeader feeds hash-table internals to the callback as elements and
+    // segfaults on the first property read. `forEach` is the ONLY method name
+    // the fused array methods share with Set/Map, so this single reroute —
+    // mirroring the typed-array reroute above — covers the hazard class.
+    {
+        let cb_value = f64::from_bits(crate::value::JSValue::pointer(callback as *const u8).bits());
+        let undef = f64::from_bits(crate::value::TAG_UNDEFINED);
+        if crate::set::is_registered_set(arr as usize) {
+            crate::set::js_set_foreach(arr as *mut crate::set::SetHeader, cb_value, undef);
+            return;
+        }
+        if crate::map::is_registered_map(arr as usize) {
+            crate::map::js_map_foreach(arr as *mut crate::map::MapHeader, cb_value, undef);
+            return;
+        }
+    }
     unsafe {
         let length = (*arr).length;
         let scope = crate::gc::RuntimeHandleScope::new();

crates/perry-runtime/src/map.rs

@@ -1944,6 +1944,9 @@ fn js_map_foreach_impl(
             let entries = entries_ptr(map);
             let key = ptr::read(entries.add(i * 2));
             let value = ptr::read(entries.add(i * 2 + 1));
+            // Root the visited key so the post-callback slot comparison below
+            // stays valid across a GC move during the callback.
+            let key_handle = scope.root_nanbox_f64(key);
             let args = [value, key, map_value];
             let cb = callback_handle.get_nanbox_f64();
             let this_v = this_handle.get_nanbox_f64();
@@ -1953,7 +1956,19 @@ fn js_map_foreach_impl(
             let prev_this = crate::object::js_implicit_this_set(this_v);
             let _ = crate::closure::js_native_call_value(cb, args.as_ptr(), args.len());
             crate::object::js_implicit_this_set(prev_this);
-            i += 1;
+            // Deleting an entry compacts the backing vector (later entries
+            // shift left). If the callback deleted the just-visited entry (or
+            // an earlier one), slot `i` now holds the NEXT unvisited entry —
+            // advancing would skip it (ECMA-262 visits every not-yet-deleted
+            // entry; mirrors the `js_set_foreach_impl` fix). Only advance when
+            // slot `i` still holds the key just visited.
+            let map = map_handle.get_raw_const_ptr::<MapHeader>();
+            if i < (*map).size as usize {
+                let now_key = ptr::read(entries_ptr(map).add(i * 2));
+                if now_key.to_bits() == key_handle.get_nanbox_f64().to_bits() {
+                    i += 1;
+                }
+            }
         }
     }
 }

crates/perry-runtime/src/set.rs

@@ -1348,13 +1348,28 @@ fn js_set_foreach_impl(
             };
             let elements = elements_ptr(set);
             let value = ptr::read(elements.add(i));
+            // Root the visited value so the post-callback slot comparison below
+            // stays valid across a GC move during the callback.
+            let value_handle = scope.root_nanbox_f64(value);
             let args = [value, value, set_value];
             let cb = callback_handle.get_nanbox_f64();
             let this_v = this_handle.get_nanbox_f64();
             let prev_this = crate::object::js_implicit_this_set(this_v);
             let _ = crate::closure::js_native_call_value(cb, args.as_ptr(), args.len());
             crate::object::js_implicit_this_set(prev_this);
-            i += 1;
+            // Deleting an entry compacts the backing vector (later entries
+            // shift left). If the callback deleted the just-visited entry (or
+            // an earlier one), slot `i` now holds the NEXT unvisited value —
+            // advancing would skip it (react-server-dom's task sweeps delete
+            // while iterating; ECMA-262 visits every not-yet-deleted entry).
+            // Only advance when slot `i` still holds the value just visited.
+            let set = set_handle.get_raw_const_ptr::<SetHeader>();
+            if i < (*set).size as usize {
+                let now = ptr::read(elements_ptr(set).add(i));
+                if now.to_bits() == value_handle.get_nanbox_f64().to_bits() {
+                    i += 1;
+                }
+            }
         }
     }
 }

test-files/test_gap_set_map_foreach_fused_receiver.ts

@@ -0,0 +1,80 @@
+// `.forEach` on an unknown-typed receiver is statically fused to the ARRAY
+// forEach entry point. When the receiver is actually a native Set or Map —
+// e.g. a collection stored on an object and read back as a property — the
+// fused call treated the SetHeader as an ArrayHeader, feeding hash-table
+// internals to the callback as elements and segfaulting on the first property
+// read. react-server-dom hits exactly this: `request.abortableTasks` is a Set
+// it iterates via `.forEach`, reading `.status` off each task — this crashed
+// every Next.js App Router dynamic route once the RSC flight started flowing
+// (#5989).
+//
+// `forEach` is the only method name the fused array methods share with
+// Set/Map, so the runtime reroute (mirroring the existing typed-array reroute)
+// covers the hazard class.
+//
+// Validated byte-for-byte against `node --experimental-strip-types`.
+
+// (1) the flight shape: Set stored on an object, read back, forEach'd
+const req: any = { abortableTasks: new Set() };
+req.abortableTasks.add({ status: 10 });
+req.abortableTasks.add({ status: 11 });
+req.abortableTasks.add({ status: 12 });
+const got: any[] = [];
+req.abortableTasks.forEach((t: any) => got.push(t.status));
+console.log(JSON.stringify(got), req.abortableTasks.size);
+
+// (2) Map stored on an object — callback receives (value, key, map)
+const holder: any = { cache: new Map() };
+holder.cache.set("a", { n: 1 });
+holder.cache.set("b", { n: 2 });
+const pairs: string[] = [];
+holder.cache.forEach((v: any, k: any) => pairs.push(`${k}=${v.n}`));
+console.log(pairs.join(","));
+
+// (3) Set forEach argument order: (value, valueAgain, set)
+const s: any = { s: new Set(["x"]) };
+s.s.forEach((v: any, v2: any, theSet: any) =>
+  console.log(v === v2, theSet.has("x"), theSet.size),
+);
+
+// (4) plain arrays through the same fused path stay correct
+const arrHolder: any = { list: [7, 8] };
+const items: string[] = [];
+arrHolder.list.forEach((v: any, i: any, a: any) => items.push(`${i}:${v}:${a.length}`));
+console.log(items.join(","));
+
+// (5) delete during Set.forEach (React deletes tasks while sweeping): the
+// backing vector compacts on delete, so naive index advancement skipped the
+// shifted-in next entry.
+const req2: any = { tasks: new Set() };
+const t1 = { id: 1 };
+const t2 = { id: 2 };
+req2.tasks.add(t1);
+req2.tasks.add(t2);
+const seen: number[] = [];
+req2.tasks.forEach((t: any) => {
+  seen.push(t.id);
+  req2.tasks.delete(t);
+});
+console.log(JSON.stringify(seen), req2.tasks.size);
+
+// (6) delete during Map.forEach — same compaction hazard
+const m6: any = { m: new Map() };
+m6.m.set("a", 1);
+m6.m.set("b", 2);
+m6.m.set("c", 3);
+const seen6: string[] = [];
+m6.m.forEach((v: any, k: any) => {
+  seen6.push(`${k}:${v}`);
+  m6.m.delete(k);
+});
+console.log(JSON.stringify(seen6), m6.m.size);
+
+// (7) delete an EARLIER entry during iteration (must not skip or re-visit)
+const s7 = new Set(["p", "q", "r"]);
+const seen7: string[] = [];
+s7.forEach((v: any) => {
+  seen7.push(v);
+  if (v === "q") s7.delete("p");
+});
+console.log(JSON.stringify(seen7), s7.size);