From 65a8a715451c094aca669bfed11272972c442783 Mon Sep 17 00:00:00 2001 From: devswha Date: Thu, 29 Oct 2026 07:46:57 +0900 Subject: [PATCH 1/2] test(files): open the read-held thread's FIFO before it says it is reading (#2296) The file-pool test that holds a thread inside a read could hang the unit run: the fake worker's readGate notified "ran" before its blocking FIFO open, and the test, after ending the thread, opened the writer blocking on its main thread. A thread ended in between never opened the reader, so the main thread blocked forever where Bun's timeout cannot fire. readGate now opens the FIFO read-write before it notifies and blocks in a read of it, as authorizeGate does; the test releases it with a non-blocking write. --- changelog.d/file-pool-test-hang.md | 4 ++++ server/file-pool.test.ts | 6 ++++-- server/file-worker.fake.ts | 11 +++++++---- 3 files changed, 15 insertions(+), 6 deletions(-) create mode 100644 changelog.d/file-pool-test-hang.md diff --git a/changelog.d/file-pool-test-hang.md b/changelog.d/file-pool-test-hang.md new file mode 100644 index 0000000000..b03e4e6886 --- /dev/null +++ b/changelog.d/file-pool-test-hang.md @@ -0,0 +1,4 @@ +### Maintenance +- The file-thread test that holds a thread inside a read can no longer hang the unit run: its + stand-in thread opens the FIFO it waits on before saying it is reading, and the test releases it + with a write that never waits, so a thread ended in between no longer blocks the test forever. diff --git a/server/file-pool.test.ts b/server/file-pool.test.ts index bbaacc1e00..28b076159b 100644 --- a/server/file-pool.test.ts +++ b/server/file-pool.test.ts @@ -189,8 +189,10 @@ describe("file threads", () => { const other = openSync(big, "r"); expect(other).not.toBe(fd); closeSync(other); - // the read returns, the thread is gone, and only then is its descriptor closed - closeSync(openSync(fifo, "w")); + // the read returns (a write that never waits: the thread opened the FIFO before it said it ran), the thread is gone, and only then is its descriptor closed + const writer = openSync(fifo, fsConstants.O_WRONLY | fsConstants.O_NONBLOCK); + writeSync(writer, "x"); + closeSync(writer); await fileThreadsClosed(); expect(() => fstatSync(fd)).toThrow(); }); diff --git a/server/file-worker.fake.ts b/server/file-worker.fake.ts index cce6cd615c..b098edfc23 100644 --- a/server/file-worker.fake.ts +++ b/server/file-worker.fake.ts @@ -7,9 +7,11 @@ * task opened (`Atomics.notify` on it; -2 when it opened none) and answers only once cell 0 is * set, as a call that returns late would; * - with a `readGate` (two Int32 cells and a FIFO's path), stops inside the file read itself: it - * records the descriptor being read in cell 1 (`Atomics.notify`), then opens the FIFO for reading, - * a real system call that blocks until a writer opens it (which ending the thread cannot cut - * short), and then reads, as a read on a stalled disk would; + * opens the FIFO (read and write, which never waits), records the descriptor being read in cell 1 + * (`Atomics.notify`), then blocks in a read of the FIFO, a real system call that ending the thread + * cannot cut short, and then reads the file, as a read on a stalled disk would. Opened before + * cell 1 is set, the FIFO always has this reader once the test sees it, so the test's release (a + * write that does not wait) never waits for this thread, even one ended in the meantime; * - with an `authorizeGate` (the same shape), stops once the file is authorized, before the task * answers, as a check on a stalled disk would: it opens the FIFO (read and write, which never * waits), records the descriptor the task named in its cell (`opened`) in cell 1, then blocks in a @@ -32,9 +34,10 @@ self.onmessage = (event: MessageEvent { const cells = new Int32Array(readGate.cells); + const fifo = openSync(readGate.fifo, constants.O_RDWR); Atomics.store(cells, 1, fd); Atomics.notify(cells, 1); - closeSync(openSync(readGate.fifo, "r")); + try { readSync(fifo, new Uint8Array(1), 0, 1, null); } finally { closeSync(fifo); } return readSync(fd, buffer, offset, length, position); }; } From 80b8cb2503c311a74cc28fb2ccc351fd1d29331e Mon Sep 17 00:00:00 2001 From: devswha Date: Thu, 29 Oct 2026 07:59:55 +0900 Subject: [PATCH 2/2] test(files): close the FIFO gates' descriptor once their ended thread is gone (#2296) A file thread ended while it blocks in a FIFO gate's read never runs its finally (on Bun a terminated worker stops as the read returns, checked 5 of 5), so both FIFO gates left their FIFO descriptor open in the test process: 2 per run of server/file-pool.test.ts. The gates now name that descriptor in a third cell, and the test closes it once the thread is gone; whichever side takes it from the cell first closes it. --- server/file-pool.test.ts | 10 +++++++++- server/file-worker.fake.ts | 30 +++++++++++++++++------------- 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/server/file-pool.test.ts b/server/file-pool.test.ts index 28b076159b..a60d14ba70 100644 --- a/server/file-pool.test.ts +++ b/server/file-pool.test.ts @@ -13,8 +13,9 @@ const waitAsync = (Atomics as unknown as { waitAsync: (cells: Int32Array, index: /** A held answer: the thread runs its task, then waits until `release`. */ const gate = () => { - const cells = new Int32Array(new SharedArrayBuffer(8)); + const cells = new Int32Array(new SharedArrayBuffer(12)); cells[1] = -1; + cells[2] = -1; return { buffer: cells.buffer as SharedArrayBuffer, release: () => { Atomics.store(cells, 0, 1); Atomics.notify(cells, 0); }, @@ -24,6 +25,11 @@ const gate = () => { if ((waited.async ? await waited.value : waited.value) === "timed-out") throw new Error("the held thread never ran its task"); return cells[1]!; }, + /** The FIFO a FIFO gate's thread, ended inside its read, never closed: closed once that thread is gone. */ + closeFifo: () => { + const fd = Atomics.exchange(cells, 2, -1); + if (fd >= 0) closeSync(fd); + }, }; }; const held = (task: FileTask, buffer: SharedArrayBuffer): FileTask & { gate: SharedArrayBuffer } => ({ ...task, gate: buffer }); @@ -171,6 +177,7 @@ describe("file threads", () => { closeSync(writer); await fileThreadsClosed(); expect(() => fstatSync(fd)).toThrow(); + hold.closeFifo(); }); it("keeps a descriptor its thread is reading open until that thread is gone, when the last server stops", async () => { @@ -195,6 +202,7 @@ describe("file threads", () => { closeSync(writer); await fileThreadsClosed(); expect(() => fstatSync(fd)).toThrow(); + hold.closeFifo(); }); it("ends its threads, a busy one too, when the last server holding them stops", async () => { diff --git a/server/file-worker.fake.ts b/server/file-worker.fake.ts index b098edfc23..c505dfc284 100644 --- a/server/file-worker.fake.ts +++ b/server/file-worker.fake.ts @@ -6,7 +6,7 @@ * - with a `gate` (a SharedArrayBuffer of two Int32 cells), records in cell 1 the descriptor the * task opened (`Atomics.notify` on it; -2 when it opened none) and answers only once cell 0 is * set, as a call that returns late would; - * - with a `readGate` (two Int32 cells and a FIFO's path), stops inside the file read itself: it + * - with a `readGate` (three Int32 cells and a FIFO's path), stops inside the file read itself: it * opens the FIFO (read and write, which never waits), records the descriptor being read in cell 1 * (`Atomics.notify`), then blocks in a read of the FIFO, a real system call that ending the thread * cannot cut short, and then reads the file, as a read on a stalled disk would. Opened before @@ -17,6 +17,10 @@ * waits), records the descriptor the task named in its cell (`opened`) in cell 1, then blocks in a * read of the FIFO. Opened before cell 1 is set, the FIFO always has this reader once the test * sees it, so the test's release (a write that does not wait) never waits for this thread. + * + * Both FIFO gates name their FIFO descriptor in cell 2: a thread ended inside its read never runs its + * `finally` (Bun stops it as the read returns), so the test closes that descriptor once the thread + * is gone. Whichever side takes it from the cell first (`Atomics.exchange`) closes it. */ import { closeSync, constants, openSync, readSync } from "node:fs"; @@ -27,29 +31,29 @@ declare const self: Worker; type Gate = { cells: SharedArrayBuffer; fifo: string }; +/** Opens the gate's FIFO (which never waits), names it in cell 2 and `fd` in cell 1 (`Atomics.notify`), then blocks in a read of it. */ +const holdOnFifo = (gate: Gate, fd: number): void => { + const cells = new Int32Array(gate.cells); + const fifo = openSync(gate.fifo, constants.O_RDWR); + Atomics.store(cells, 2, fifo); + Atomics.store(cells, 1, fd); + Atomics.notify(cells, 1); + try { readSync(fifo, new Uint8Array(1), 0, 1, null); } finally { if (Atomics.exchange(cells, 2, -1) >= 0) closeSync(fifo); } +}; + self.onmessage = (event: MessageEvent) => { const task = event.data; if ("path" in task && (task.path.endsWith("/stuck") || (task.op === "open" && task.path.endsWith("/held-open")))) return; const readGate = task.readGate; if (readGate !== undefined) { pageFileHooks.readSync = (fd, buffer, offset, length, position) => { - const cells = new Int32Array(readGate.cells); - const fifo = openSync(readGate.fifo, constants.O_RDWR); - Atomics.store(cells, 1, fd); - Atomics.notify(cells, 1); - try { readSync(fifo, new Uint8Array(1), 0, 1, null); } finally { closeSync(fifo); } + holdOnFifo(readGate, fd); return readSync(fd, buffer, offset, length, position); }; } const authorizeGate = task.authorizeGate; if (authorizeGate !== undefined) { - pageFileHooks.afterAuthorize = () => { - const cells = new Int32Array(authorizeGate.cells); - const fifo = openSync(authorizeGate.fifo, constants.O_RDWR); - Atomics.store(cells, 1, "opened" in task && task.opened !== undefined ? task.opened[0]! : -2); - Atomics.notify(cells, 1); - try { readSync(fifo, new Uint8Array(1), 0, 1, null); } finally { closeSync(fifo); } - }; + pageFileHooks.afterAuthorize = () => holdOnFifo(authorizeGate, "opened" in task && task.opened !== undefined ? task.opened[0]! : -2); } let result; try { result = runFileTask(task); } finally { delete pageFileHooks.readSync; delete pageFileHooks.afterAuthorize; }