Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/jsc/bindings/webcore/AbortSignal.h
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,11 @@ class AbortSignal final : public RefCounted<AbortSignal>, 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; }
Expand Down
4 changes: 3 additions & 1 deletion src/jsc/bindings/webcore/JSAbortSignalCustom.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,9 @@ bool JSAbortSignalOwner::isReachableFromOpaqueRoots(JSC::Handle<JSC::Unknown> 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;
Expand Down
53 changes: 53 additions & 0 deletions test/js/web/abort/abort-controller-gc-reason.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,59 @@ describe("AbortController GC", () => {
expect(exitCode).toBe(0);
});

// 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: [
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: [
Expand Down
Loading