Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
17 changes: 9 additions & 8 deletions src/jsc/bindings/JSCTaskScheduler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,17 +35,18 @@ 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
// still-pending Atomics.waitAsync ticket while holding the process-global
// waiter-lists lock, so a per-call scan makes a worker exiting with N waiters
// O(N^2) and stalls Atomics.notify/waitAsync on every other thread for the
// whole window.
Comment thread
robobun marked this conversation as resolved.
Outdated
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;
}

Expand Down
55 changes: 55 additions & 0 deletions test/js/web/atomics.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { describe, expect, test } from "bun:test";
import { bunEnv, bunExe, tempDir } from "harness";

describe("Atomics", () => {
describe("basic operations", () => {
Expand Down Expand Up @@ -307,3 +308,57 @@
});
});
});

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",
});

// 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 = await proc.stdout.text();
const { exitAfterMs } = JSON.parse(stdout);
expect(exitAfterMs).toBeLessThan(15_000);
expect(await proc.exited).toBe(0);

Check warning on line 362 in test/js/web/atomics.test.ts

View check run for this annotation

Claude / Claude Code Review

Subprocess test doesn't pipe stderr or follow Promise.all convention

Nit: this deviates from the harness subprocess convention (`Promise.all([stdout.text(), stderr.text(), exited])` and assert a combined `{ stdout, stderr, exitCode }`). stderr isn't piped and stdout is read only after exit — no deadlock risk here (one short JSON line, stderr inherited), but if the fixture ever crashes before printing, the test fails on `JSON.parse("")` with "Unexpected EOF" instead of surfacing the exit code and stderr in the assertion. Piping stderr and asserting `{ stdout, stde
Comment thread
robobun marked this conversation as resolved.
Outdated
}, 90_000);
});