From bd9720a7d0d0eefd2dc543f9b6acb7542b0b716d Mon Sep 17 00:00:00 2001 From: robobun Date: Fri, 26 Jun 2026 19:59:20 +0000 Subject: [PATCH 1/2] AbortSignal: don't prune the source-signal set from GC marker threads JSAbortSignalOwner::isReachableFromOpaqueRoots() runs on JSC's parallel marker threads. For a dependent (AbortSignal.any) signal with an abort listener it probed the source set with WeakListHashSet::isEmptyIgnoringNullReferences(), which clear()s the set once every source signal has died. That destroys WeakPtrs whose impls use a single-threaded refcount, so assert-enabled builds crash with "m_creationThread == currentThreadID()" and release builds race the JS thread (node frees and WeakPtrImpl derefs off-thread). Replace the probe with a read-only one (begin() != end()); pruning keeps happening on the JS thread via the container's amortized cleanup. --- src/jsc/bindings/webcore/AbortSignal.h | 5 ++ .../bindings/webcore/JSAbortSignalCustom.cpp | 4 +- .../abort/abort-controller-gc-reason.test.ts | 55 +++++++++++++++++++ 3 files changed, 63 insertions(+), 1 deletion(-) diff --git a/src/jsc/bindings/webcore/AbortSignal.h b/src/jsc/bindings/webcore/AbortSignal.h index 1442edfe8806..7eae1b2f3ba6 100644 --- a/src/jsc/bindings/webcore/AbortSignal.h +++ b/src/jsc/bindings/webcore/AbortSignal.h @@ -123,6 +123,11 @@ class AbortSignal final : public RefCounted, public EventTargetWith const AbortSignalSet& sourceSignals() const { return m_sourceSignals; } AbortSignalSet& sourceSignals() { return m_sourceSignals; } + // Read-only emptiness probe for GC marker threads (JSAbortSignalOwner::isReachableFromOpaqueRoots). + // WeakListHashSet::isEmptyIgnoringNullReferences() prunes dead entries, destroying WeakPtrs whose + // single-threaded impls (and the nodes holding them) may only be released on the owning thread. + bool hasAliveSourceSignals() const { return m_sourceSignals.begin() != m_sourceSignals.end(); } + // https://github.com/oven-sh/bun/issues/4517 void incrementPendingActivityCount() { ++pendingActivityCount; } void decrementPendingActivityCount() { --pendingActivityCount; } diff --git a/src/jsc/bindings/webcore/JSAbortSignalCustom.cpp b/src/jsc/bindings/webcore/JSAbortSignalCustom.cpp index 664732cf494b..c0666c2af62f 100644 --- a/src/jsc/bindings/webcore/JSAbortSignalCustom.cpp +++ b/src/jsc/bindings/webcore/JSAbortSignalCustom.cpp @@ -55,7 +55,9 @@ bool JSAbortSignalOwner::isReachableFromOpaqueRoots(JSC::Handle ha return true; } if (abortSignal.isDependent()) { - if (!abortSignal.sourceSignals().isEmptyIgnoringNullReferences()) { + // This runs on GC marker threads, so it must not mutate the signal: + // sourceSignals().isEmptyIgnoringNullReferences() prunes dead entries. + if (abortSignal.hasAliveSourceSignals()) { if (reason) [[unlikely]] *reason = "Has Source Signals And Abort Event Listener"_s; return true; diff --git a/test/js/web/abort/abort-controller-gc-reason.test.ts b/test/js/web/abort/abort-controller-gc-reason.test.ts index 75edf2586473..16ba7bd231b2 100644 --- a/test/js/web/abort/abort-controller-gc-reason.test.ts +++ b/test/js/web/abort/abort-controller-gc-reason.test.ts @@ -99,6 +99,61 @@ describe("AbortController GC", () => { expect(exitCode).toBe(0); }); + // AbortSignal.any(): JSAbortSignalOwner::isReachableFromOpaqueRoots probed the dependent + // signal's source set with WeakListHashSet::isEmptyIgnoringNullReferences(), which prunes + // dead entries. It runs on JSC's parallel marker threads, so once every source controller + // had been collected, a HeapHelper thread destroyed WeakPtrImpls owned by the JS thread + // ("ASSERTION FAILED: m_creationThread == currentThreadID()"; a data race in release). + test("AbortSignal.any() dependent signals survive parallel GC after their sources are collected", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const noop = () => {}; + function makeBatch(n) { + const out = []; + for (let i = 0; i < n; i++) { + // The controllers (and their signals) are dropped, so every source of the + // dependent signal is collected, leaving only dead weak references behind. + const a = new AbortController(); + const b = new AbortController(); + const c = new AbortController(); + const dep = AbortSignal.any([a.signal, b.signal, c.signal]); + dep.addEventListener("abort", noop); + out.push(dep); + } + return out; + } + const keep = []; + for (let round = 0; round < 24; round++) { + keep.push(makeBatch(100)); + if (keep.length > 6) keep.shift(); + Bun.gc(true); + } + console.log("PASS"); + `, + ], + env: { + ...bunEnv, + // `bun -e` defaults to a single GC marker; the weak-handle visit has to happen on + // the parallel marker threads to reach the cross-thread mutation. + BUN_JSC_numberOfGCMarkers: "8", + }, + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + // stderr is only reported for diagnostics; debug/ASAN builds may emit benign warnings. + expect({ stdout: stdout.trim(), exitCode, stderr }).toEqual({ + stdout: "PASS", + exitCode: 0, + stderr: expect.any(String), + }); + }); + test("signal.reason survives GC with many controllers", async () => { await using proc = Bun.spawn({ cmd: [ From c34c700ea96ede2f4d1a8de3171f3516da9a0b34 Mon Sep 17 00:00:00 2001 From: robobun Date: Fri, 26 Jun 2026 20:19:44 +0000 Subject: [PATCH 2/2] test: condense regression test comment to three lines --- test/js/web/abort/abort-controller-gc-reason.test.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/test/js/web/abort/abort-controller-gc-reason.test.ts b/test/js/web/abort/abort-controller-gc-reason.test.ts index 16ba7bd231b2..aad0eb9af074 100644 --- a/test/js/web/abort/abort-controller-gc-reason.test.ts +++ b/test/js/web/abort/abort-controller-gc-reason.test.ts @@ -99,11 +99,9 @@ describe("AbortController GC", () => { expect(exitCode).toBe(0); }); - // AbortSignal.any(): JSAbortSignalOwner::isReachableFromOpaqueRoots probed the dependent - // signal's source set with WeakListHashSet::isEmptyIgnoringNullReferences(), which prunes - // dead entries. It runs on JSC's parallel marker threads, so once every source controller - // had been collected, a HeapHelper thread destroyed WeakPtrImpls owned by the JS thread - // ("ASSERTION FAILED: m_creationThread == currentThreadID()"; a data race in release). + // JSAbortSignalOwner::isReachableFromOpaqueRoots runs on JSC's parallel marker threads and + // must not prune the dependent signal's source set there: doing so destroys WeakPtrImpls + // owned by the JS thread once every source controller has been collected. test("AbortSignal.any() dependent signals survive parallel GC after their sources are collected", async () => { await using proc = Bun.spawn({ cmd: [