diff --git a/src/jsc/bindings/JSCTaskScheduler.cpp b/src/jsc/bindings/JSCTaskScheduler.cpp index 4f6b8ceae48f..5e91d3cc9d17 100644 --- a/src/jsc/bindings/JSCTaskScheduler.cpp +++ b/src/jsc/bindings/JSCTaskScheduler.cpp @@ -35,17 +35,14 @@ class JSCDeferredWorkTask { // Drop `ticket` from whichever pending set holds it. Caller holds m_lock; the // event-loop ref is balanced after the caller releases the lock. +// Must stay O(1): ~VM -> WaiterListManager::unregister reaches this once per +// pending Atomics.waitAsync ticket under the process-global waiter-lists lock. static bool dropPendingTicketLocked(Bun::JSCTaskScheduler& scheduler, Ticket* ticket) WTF_REQUIRES_LOCK(scheduler.m_lock) { - bool isKeepingEventLoopAlive = scheduler.m_pendingTicketsKeepingEventLoopAlive.removeIf([ticket](auto pendingTicket) { - return pendingTicket.ptr() == ticket; - }); + bool isKeepingEventLoopAlive = scheduler.m_pendingTicketsKeepingEventLoopAlive.remove(ticket); // -- At this point, ticket may be an invalid pointer. - if (!isKeepingEventLoopAlive) { - scheduler.m_pendingTicketsOther.removeIf([ticket](auto pendingTicket) { - return pendingTicket.ptr() == ticket; - }); - } + if (!isKeepingEventLoopAlive) + scheduler.m_pendingTicketsOther.remove(ticket); return isKeepingEventLoopAlive; } diff --git a/test/js/web/atomics.test.ts b/test/js/web/atomics.test.ts index a36b5cf80cd2..f18bb3640ba4 100644 --- a/test/js/web/atomics.test.ts +++ b/test/js/web/atomics.test.ts @@ -1,4 +1,5 @@ import { describe, expect, test } from "bun:test"; +import { bunEnv, bunExe, tempDir } from "harness"; describe("Atomics", () => { describe("basic operations", () => { @@ -307,3 +308,58 @@ describe("Atomics", () => { }); }); }); + +describe("Atomics.waitAsync worker teardown", () => { + // A worker exiting with N pending Atomics.waitAsync waiters used to tear down in + // O(N^2): ~VM cancels each waiter's ticket, and each cancellation scanned the whole + // pending-ticket set. The scan also ran while JSC held the process-global + // waiter-lists lock, stalling Atomics.notify/waitAsync on every other thread. + // With N = 50000 the quadratic teardown took 38s+ in release; linear teardown is + // well under a second. The 15s bound leaves wide margin for slow debug/ASAN CI. + test("worker with many pending waiters exits quickly", async () => { + using dir = tempDir("atomics-waiter-teardown", { + "waiter-teardown-fixture.mjs": ` + import { Worker, isMainThread, parentPort, workerData } from "node:worker_threads"; + if (!isMainThread) { + const i32 = new Int32Array(workerData.sab); + for (let k = 0; k < 50000; k++) Atomics.waitAsync(i32, k % 1024, 0); + parentPort.postMessage("armed"); + setInterval(() => {}, 1e6); + } else { + const sab = new SharedArrayBuffer(4096); + const w = new Worker(new URL(import.meta.url), { workerData: { sab } }); + w.on("message", () => { + const start = Date.now(); + w.terminate(); + w.on("exit", () => { + console.log(JSON.stringify({ exitAfterMs: Date.now() - start })); + process.exit(0); + }); + }); + } + `, + }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "waiter-teardown-fixture.mjs"], + env: bunEnv, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + + // Fail fast instead of waiting out a quadratic teardown (minutes in debug). + let timer: Timer | undefined; + const deadline = new Promise<"deadline">(resolve => { + timer = setTimeout(() => resolve("deadline"), 60_000); + }); + const first = await Promise.race([proc.exited, deadline]); + clearTimeout(timer); + if (first === "deadline") proc.kill("SIGKILL"); + expect(first).not.toBe("deadline"); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stderr, exitCode }).toEqual({ stderr: "", exitCode: 0 }); + const { exitAfterMs } = JSON.parse(stdout); + expect(exitAfterMs).toBeLessThan(15_000); + }, 90_000); +});