diff --git a/src/runtime/webcore/FileReader.zig b/src/runtime/webcore/FileReader.zig index 0d26e3f3d2e2..efb8aa065fc3 100644 --- a/src/runtime/webcore/FileReader.zig +++ b/src/runtime/webcore/FileReader.zig @@ -622,6 +622,16 @@ pub fn onReaderError(this: *FileReader, err: bun.sys.Error) void { this.pending.result = .{ .err = .{ .Error = err } }; this.pending.run(); + + // onStart() increments the parent refcount and sets waiting_for_onReaderDone + // so the reader stays alive until I/O completes. PosixBufferedReader.onError + // only invokes this callback and does not follow up with done()/onReaderDone, + // so without this decrement the refcount never reaches 0 after JS finalize + // and the FileReader (plus its fd/poll) leaks. + if (this.waiting_for_onReaderDone) { + this.waiting_for_onReaderDone = false; + _ = this.parent().decrementCount(); + } } pub fn setRawMode(this: *FileReader, flag: bool) bun.sys.Maybe(void) { diff --git a/test/js/bun/util/bun-file-fd-read.test.ts b/test/js/bun/util/bun-file-fd-read.test.ts index fe1e53191e31..de928117f9fd 100644 --- a/test/js/bun/util/bun-file-fd-read.test.ts +++ b/test/js/bun/util/bun-file-fd-read.test.ts @@ -1,6 +1,6 @@ import { describe, expect, test } from "bun:test"; import { closeSync, openSync } from "fs"; -import { isWindows, tempDir } from "harness"; +import { bunEnv, bunExe, isLinux, isMacOS, isWindows, tempDir } from "harness"; import { join } from "path"; // Reading a Bun.file() backed by a file descriptor goes through @@ -50,3 +50,107 @@ describe.skipIf(isWindows)("Bun.file(fd) read", () => { expect((await withFd(path, fd => Bun.file(fd).arrayBuffer())).byteLength).toBe(0); }); }); + +// FileReader.onStart() increments the parent refcount and sets +// waiting_for_onReaderDone=true so the native source outlives the JS wrapper +// until I/O completes. On POSIX, when a read syscall fails, +// PosixBufferedReader.onError only invokes onReaderError and does not follow +// up with done()/onReaderDone. FileReader.onReaderError previously rejected +// the pending pull but never cleared waiting_for_onReaderDone or called +// decrementCount(), so after JS finalize the refcount sat at 1, deinit never +// ran, and the dup'd fd/poll leaked forever. +// +// Gated to Linux/macOS (not isPosix) because the fixture's countFds relies on +// /proc/self/fd or /dev/fd, and procfs is not mounted by default on FreeBSD. +test.skipIf(!isLinux && !isMacOS)("Bun.file(fd).stream() does not leak fds when read fails (ECONNRESET)", async () => { + // Run in a subprocess so fd counting is not polluted by the test runner's + // own descriptors and GC finalization is deterministic at exit. + const fixture = /* js */ ` + import net from "node:net"; + import fs from "node:fs"; + + const countFds = () => + fs.readdirSync(process.platform === "darwin" ? "/dev/fd" : "/proc/self/fd").length; + + // Bun.listen socket.terminate() closes with SO_LINGER{1,0} so the peer's + // next recv() returns ECONNRESET, which is the read-syscall error path + // (PosixBufferedReader.onError -> FileReader.onReaderError) we need. + const server = Bun.listen({ + hostname: "127.0.0.1", + port: 0, + socket: { + data(socket) { socket.terminate(); }, + open() {}, close() {}, error() {}, + }, + }); + + async function once() { + const sock = new net.Socket(); + await new Promise((resolve, reject) => { + sock.once("connect", resolve); + sock.once("error", reject); + sock.connect(server.port, "127.0.0.1"); + }); + const fd = sock._handle.fd; + + // getReader() triggers FileReader.onStart() synchronously: dups fd, + // registers epoll poll, increments refcount. + const reader = Bun.file(fd).stream().getReader(); + + // Wake the server so it RSTs us. + sock.write("x"); + + let caught; + try { + while (true) { + const { done } = await reader.read(); + if (done) break; + } + } catch (e) { + caught = e; + } + sock.destroy(); + return caught; + } + + // Warm up so any one-time allocations (epoll fd, etc.) don't count as leaks. + await once(); + Bun.gc(true); + const before = countFds(); + + let errored = 0; + const iterations = 50; + for (let i = 0; i < iterations; i++) { + if (await once()) errored++; + } + + Bun.gc(true); + await Bun.sleep(0); + Bun.gc(true); + const after = countFds(); + + console.log(JSON.stringify({ before, after, leaked: after - before, errored, iterations })); + server.stop(true); + // Leaked polls keep the event loop alive on the buggy build; exit + // explicitly so the fd-count assertion above is what fails, not a timeout. + process.exit(0); + `; + + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", fixture], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect(stderr).toBe(""); + const result = JSON.parse(stdout.trim()); + // The read must actually fail with ECONNRESET; if it doesn't, the test + // isn't exercising the onReaderError path and would pass vacuously. + expect(result.errored).toBeGreaterThan(result.iterations / 2); + // Allow a little slack for incidental fds, but a per-iteration leak would + // show ~iterations extra fds here. + expect(result.leaked).toBeLessThan(5); + expect(exitCode).toBe(0); +});