Skip to content
28 changes: 22 additions & 6 deletions test/js/web/workers/worker.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -407,16 +407,32 @@
});

// A worker posting faster than the parent can deserialize must not pin the
// parent inside one drain: its timers and I/O still get their turn.
// parent inside one drain: a drain task delivers a bounded batch and posts the
// rest to the next loop iteration, so timers and I/O get their turn in between.
test("a message flood from a worker does not starve the parent's event loop", async () => {
const src = `const p = { s: Buffer.alloc(200, "x").toString(), a: [1, 2, 3], n: 0 };
(function burst() { for (let i = 0; i < 2000; i++) { p.n++; postMessage(p) } setImmediate(burst) })()`;
const total = 1500; // more than one drain task's budget (drainBatchLimit in WorkerMessagingProxy.cpp)
const flag = new Int32Array(new SharedArrayBuffer(4));
const src = `onmessage = ({ data: flag }) => {
for (let i = 0; i < ${total}; i++) postMessage(i);
Atomics.store(flag, 0, 1); Atomics.notify(flag, 0);
}`;
const w = new Worker(URL.createObjectURL(new Blob([src])));
let received = 0;
w.onmessage = () => received++;
// Three timer turns while the flood is running is the property; not the timing.
for (let i = 0; i < 3; i++) await new Promise<void>(r => setTimeout(r, 10));
const delivered = Promise.withResolvers<void>();
w.onmessage = () => {
if (++received === total) delivered.resolve();
};
w.postMessage(flag);
// Block until the whole flood sits in the parent's inbox, so the first drain task
// starts out with more than its budget no matter how fast either thread is.
expect(Atomics.wait(flag, 0, 0, 30_000)).toBe("ok");

Check warning on line 428 in test/js/web/workers/worker.test.ts

View check run for this annotation

Claude / Claude Code Review

Atomics.wait assertion is over-strict: "not-equal" is also a success outcome

`Atomics.wait(flag, 0, 0, 30_000)` returns `"not-equal"` (not `"ok"`) if `flag[0]` is already 1 at entry — and in that case the precondition (all 1500 messages in the inbox) is *already satisfied*, since the worker only stores 1 after posting every message. If the parent thread is preempted between `w.postMessage(flag)` and `Atomics.wait` long enough for the worker to finish (a narrow but nonzero window on an oversubscribed machine), the test spuriously fails. Since the goal here is timing-indep
Comment thread
claude[bot] marked this conversation as resolved.
Outdated
// Resumes inside the first drain task. An immediate armed there runs as soon as
// that task ends, before the continuation it posted delivers the next batch.
await once(w, "message");
await new Promise<void>(r => setImmediate(r));
expect(received).toBeGreaterThan(0);

Check warning on line 433 in test/js/web/workers/worker.test.ts

View check run for this annotation

Claude / Claude Code Review

Vacuous assertion reintroduced: toBeGreaterThan(0) cannot fail after await once(w, "message")

The d2a836ed rewrite reintroduces the vacuous assertion the earlier (resolved) review thread removed in 82687bc: `w.onmessage` is registered before `once(w, "message")`, so by the time that await resumes `received >= 1` is structurally guaranteed and `toBeGreaterThan(0)` cannot fail. Drop line 433 — line 434 (`toBeLessThan(total)`) is the load-bearing check. The PR description also still describes the 82687bc state ("the received counter went away with the > 0 assertion"), which no longer matche
Comment thread
claude[bot] marked this conversation as resolved.
Outdated
expect(received).toBeLessThan(total);
await delivered.promise;
w.terminate();
await once(w, "close");
});
Expand Down