Use visitor.vm() in GlobalObject::visitChildren; make HTTPHeaderIdentifiers eager

src/jsc/bindings/BunClientData.cpp

@@ -125,11 +125,4 @@ void JSVMClientData::create(VM* vm, void* bunVM)
     clientData->builtinFunctions().exportNames();
 }
 
-WebCore::HTTPHeaderIdentifiers& JSVMClientData::httpHeaderIdentifiers()
-{
-    if (!m_httpHeaderIdentifiers)
-        m_httpHeaderIdentifiers.emplace();
-    return *m_httpHeaderIdentifiers;
-}
-
 } // namespace WebCore

src/jsc/bindings/BunClientData.h

@@ -111,7 +111,12 @@ class JSVMClientData : public JSC::VM::ClientData {
 
     JSC::GCClient::IsoSubspace& domBuiltinConstructorSpace() { return m_domBuiltinConstructorSpace; }
 
-    WebCore::HTTPHeaderIdentifiers& httpHeaderIdentifiers();
+    // Constructed eagerly so the concurrent GC marker
+    // (Zig::GlobalObject::visitChildrenImpl) never races the mutator on a
+    // lazy std::optional::emplace(). The ctor only calls
+    // LazyProperty::initLater ~90 times (stores a tagged function pointer),
+    // so there is no startup cost worth deferring.
+    WebCore::HTTPHeaderIdentifiers& httpHeaderIdentifiers() { return m_httpHeaderIdentifiers; }
 
     template<typename Func> void forEachOutputConstraintSpace(const Func& func)
     {
@@ -163,7 +168,7 @@ class JSVMClientData : public JSC::VM::ClientData {
     std::unique_ptr<ExtendedDOMClientIsoSubspaces> m_clientSubspaces;
     Vector<JSC::IsoSubspace*> m_outputConstraintSpaces;
 
-    std::optional<WebCore::HTTPHeaderIdentifiers> m_httpHeaderIdentifiers;
+    WebCore::HTTPHeaderIdentifiers m_httpHeaderIdentifiers;
     WeakHashSet<JSVMClientDataClient> m_clients;
 };
 

src/jsc/bindings/ZigGlobalObject.cpp

@@ -3165,7 +3165,15 @@ void GlobalObject::visitChildrenImpl(JSCell* cell, Visitor& visitor)
     FOR_EACH_GLOBALOBJECT_GC_MEMBER(VISIT_GLOBALOBJECT_GC_MEMBER)
 #undef VISIT_GLOBALOBJECT_GC_MEMBER
 
-    WebCore::clientData(thisObject->vm())->httpHeaderIdentifiers().visit<Visitor>(visitor);
+    // This runs on a concurrent GC helper thread. Fetch the VM through the
+    // visitor (AbstractSlotVisitor::vm() returns m_heap.vm(), guaranteed alive
+    // for the duration of marking) rather than thisObject->vm() which
+    // dereferences JSGlobalObject::m_vm and can read stale bytes if the cell
+    // was picked up via conservative scan mid-recycle (see the
+    // visitGlobalObjectMember(unique_ptr) guard above for the same window).
+    // A stale m_vm surfaces as a SEGV in TypeCastTraits<JSVMClientData>::isType
+    // when downcast<> calls the virtual isWebCoreJSClientData() on garbage.
+    WebCore::clientData(visitor.vm())->httpHeaderIdentifiers().template visit<Visitor>(visitor);
 
     thisObject->visitGeneratedLazyClasses<Visitor>(thisObject, visitor);
     thisObject->visitAdditionalChildrenInGCThread<Visitor>(visitor);

test/js/web/broadcastchannel/broadcast-channel-worker-gc.test.ts

@@ -185,6 +185,100 @@ test(
   timeout,
 );
 
+// SEGV in TypeCastTraits<JSVMClientData>::isType reached from
+// Zig::GlobalObject::visitChildrenImpl on a concurrent GC helper thread. The
+// visit used `clientData(thisObject->vm())` (raw JSGlobalObject::m_vm deref)
+// and `httpHeaderIdentifiers()` did an unsynchronized std::optional::emplace()
+// that both the mutator and parallel marker threads could enter. Observed in
+// production crash reports almost exclusively on Windows x64, strongly
+// correlated with worker spawn+terminate.
+//
+// This stress maximises the race surface:
+//   - extra ShadowRealm globals so multiple parallel marker helpers each visit
+//     a distinct Zig::GlobalObject and all dereference vm.clientData
+//   - continuous allocation so the concurrent collector is always active
+//   - worker spawn/terminate churn for the reported correlation
+//
+// The race window is too narrow to trip deterministically on Linux even under
+// ASAN + Malloc=1 + collectContinuously, so this test is a guard rather than
+// a fail-before proof; it is the Windows CI lane that intermittently crashed
+// with this signature on the unfixed build.
+test(
+  "GlobalObject::visitChildren does not dereference stale vm.clientData under parallel marking",
+  async () => {
+    const script = /* js */ `
+    const workerCode = \`
+      // Extra Zig::GlobalObject cells in this VM so parallel GC helper
+      // threads each get one to visit and all call clientData(vm).
+      const realms = [];
+      for (let i = 0; i < 6; i++) { try { realms.push(new ShadowRealm()); } catch {} }
+
+      const bc = new BroadcastChannel("clientdata-visit");
+      bc.onmessage = () => {};
+
+      // Keep the concurrent marker busy so visitChildrenImpl fires
+      // repeatedly on helper threads while the mutator runs.
+      const junk = [];
+      for (let i = 0; i < 2000; i++) junk.push({ i, a: [i, i] });
+      Bun.gc(true);
+      postMessage("ready");
+      for (let i = 0; i < 2000; i++) junk.push({ i });
+    \`;
+    const blobUrl = URL.createObjectURL(new Blob([workerCode], { type: "application/javascript" }));
+
+    const mainChannel = new BroadcastChannel("clientdata-visit");
+    mainChannel.onmessage = () => {};
+
+    // Extra globals on the main VM too.
+    const realms = [];
+    for (let i = 0; i < 6; i++) { try { realms.push(new ShadowRealm()); } catch {} }
+
+    for (let round = 0; round < 6; round++) {
+      const workers = [];
+      const ready = [];
+      for (let i = 0; i < 4; i++) {
+        const w = new Worker(blobUrl);
+        const { promise, resolve, reject } = Promise.withResolvers();
+        w.onmessage = () => resolve();
+        w.onerror = (e) => reject(e.message ?? String(e));
+        workers.push(w);
+        ready.push(promise);
+      }
+      await Promise.all(ready);
+
+      // Allocation pressure on main so its parallel markers are live while
+      // worker VMs tear down.
+      const junk = [];
+      for (let i = 0; i < 3000; i++) junk.push({ a: i, b: [i] });
+
+      for (const w of workers) {
+        mainChannel.postMessage("x");
+        w.terminate();
+      }
+      for (let i = 0; i < 3; i++) Bun.gc(true);
+      junk.length = 0;
+    }
+
+    mainChannel.close();
+    console.log("OK");
+  `;
+
+    await using proc = Bun.spawn({
+      cmd: [bunExe(), "-e", script],
+      env: { ...bunEnv, BUN_JSC_numberOfGCMarkers: "8", Malloc: "1" },
+      stderr: "pipe",
+      stdout: "pipe",
+    });
+
+    const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+
+    expect(filterStderr(stderr)).toBe("");
+    expect(stdout.trim()).toBe("OK");
+    expect(exitCode).toBe(0);
+  },
+  timeout,
+);
+
 test(
   "BroadcastChannel: repeated worker create/terminate stress",
   async () => {