Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
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
10 changes: 10 additions & 0 deletions src/runtime/webcore/FileReader.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Comment thread
robobun marked this conversation as resolved.
}

pub fn setRawMode(this: *FileReader, flag: bool) bun.sys.Maybe(void) {
Expand Down
106 changes: 105 additions & 1 deletion test/js/bun/util/bun-file-fd-read.test.ts
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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);
});
Loading