diff --git a/DESIGN.md b/DESIGN.md index c4ff7ee..f759eb5 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -1050,3 +1050,18 @@ pathological input, by growth rate), `terminal.fuzz.test.ts` (seeded hostile out consistent screen, bounded cells, no reply echoing attacker text, nothing while replaying), `files.resolve.fuzz.test.ts` (random paths through in/out/loop symlinks never resolve or open outside), `git.hostile-config.test.ts` and `pty.tracker.diff.test.ts` (above). + +## 25. Round 16: a shrinking file could hang /api/raw + +Release v1.10.7 stopped at its test step on a timeout in the `/api/raw` shrinking-file test, which +had passed on the same commit in `ci`. It was not a slow machine: the same test failed 4 runs in 30 +on v1.10.6 and about one in four alone, because the server sometimes never ended the response. + +| Finding | Fix | +| --- | --- | +| `ctl.error` right after a chunk could be lost by Bun 1.4.4: the connection stayed open with no end of body, and `idleTimeout: 0` never reclaims it, so a client's `blob()` waited forever | Wait 50 ms inside `pull` before the error. Measured against the real file path: no hang with readers up to 400 ms per chunk or under CPU load. Erroring after `pull` returned, throwing, generator and direct streams, `server.timeout` and padding chunks all still hung | +| The test ran the race once, so it passed most of the time and blocked a release at random | 25 rounds with a watchdog per round, reported as "hang" | +| Looped at 2 MB, the test once saw a clean end on Linux CI. The server only closes cleanly after reading every byte, so the transfer had probably finished before the truncate, but the test counted no bytes and 1200 more rounds did not repeat it | A 256 MB sparse file that cannot finish first, and byte counts: a full transfer is correct, a short one fails as "short by N" | + +The delay is measured, not derived: the cause is inside Bun. The looped test is what will notice if +a Bun upgrade changes this. diff --git a/src/server/http.ts b/src/server/http.ts index 865c459..773f14c 100644 --- a/src/server/http.ts +++ b/src/server/http.ts @@ -15,6 +15,9 @@ import { Candidates, compileQuery, globsOk, createWorkerScanner, type SearchEven import pkg from "../../package.json" with { type: "json" }; +/** How long /api/raw waits before failing a body whose file shrank: see the comment there. */ +const RAW_ABORT_DELAY_MS = 50; + export const VERSION: string = pkg.version; export interface ServeOptions { @@ -416,8 +419,12 @@ export function startServer(opts: ServeOptions): { server: Server; u const { bytesRead } = chunk.length ? await fh.read(chunk, 0, chunk.length, pos) : { bytesRead: 0 }; if (bytesRead === 0) { await fh.close(); + if (pos >= stop) return ctl.close(); // The file shrank after fstat: fail the body rather than send fewer bytes than promised. - return pos < stop ? ctl.error(new Error("file changed while reading")) : ctl.close(); + // Bun 1.4.4 can drop an error raised right after a chunk and leave the connection open + // with no end of body (one in four, and idleTimeout is 0); waiting inside pull closes it. + await Bun.sleep(RAW_ABORT_DELAY_MS); + return ctl.error(new Error("file changed while reading")); } pos += bytesRead; ctl.enqueue(chunk.subarray(0, bytesRead)); diff --git a/test/http.coverage.test.ts b/test/http.coverage.test.ts index 1325b4d..6dc0cb6 100644 --- a/test/http.coverage.test.ts +++ b/test/http.coverage.test.ts @@ -511,24 +511,37 @@ test("/api/raw honours a single byte range (206/416) so media can seek, also und }); test("/api/raw fails the body instead of sending fewer bytes than content-length when the file shrinks", async () => { - const p = path.join(dir, "shrink.bin"); - await writeFile(p, new Uint8Array(2 * 1024 * 1024)); - const r = await get("/api/raw?path=shrink.bin"); - expect(r.status).toBe(200); - const reader = r.body!.getReader(); - const first = await reader.read(); - expect(first.done).toBe(false); - await truncate(p, 10); - // The transfer fails (the stream is errored): never a quiet success with a truncated body. - let failed = false; - try { - for (let c = await reader.read(); !c.done; c = await reader.read()); - } catch { - failed = true; + // Repeated: Bun can lose the error and leave the connection open, about one run in four. The file + // is large (and sparse) so that the transfer is still running when it shrinks. + const size = 256 * 1024 * 1024; + const outcomes: string[] = []; + for (let i = 0; i < 25; i++) { + const p = path.join(dir, "shrink.bin"); + await writeFile(p, ""); + await truncate(p, size); + const r = await get("/api/raw?path=shrink.bin"); + expect(r.status).toBe(200); + const reader = r.body!.getReader(); + const first = await reader.read(); + expect(first.done).toBe(false); + let bytes = first.value!.length; + await truncate(p, 10); + const drain = async () => { + try { + for (let c = await reader.read(); !c.done; c = await reader.read()) bytes += c.value.length; + return bytes === size ? "complete" : `short by ${size - bytes}`; + } catch { + return "failed"; + } + }; + outcomes.push(await Promise.race([drain(), Bun.sleep(1500).then(() => "hang")])); } - expect(failed).toBe(true); + // Never a hang and never a quiet success with a truncated body. "complete" means every byte had + // already been sent before the file shrank, which is a correct transfer. + expect(outcomes.filter((o) => o !== "failed" && o !== "complete")).toEqual([]); + expect(outcomes).toContain("failed"); expect((await get("/api/info")).status).toBe(200); -}); +}, 45_000); test("round 15: no Bun error page, own-key MIME lookups, whole-byte limits, files-only open", async () => { // A Host header that makes the URL unparseable: a JSON 400/421, never an HTML page with paths.