Skip to content
Open
Show file tree
Hide file tree
Changes from 2 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
23 changes: 20 additions & 3 deletions src/runtime/server/FileResponseStream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -545,6 +545,12 @@ impl FileResponseStream {
(self.on_complete.get())(self.ctx.get(), resp);
}

// An abort can finish the stream while a read is still parked on the
// poll (e.g. a FIFO with no writer); `on_reader_done`/`on_reader_error`
// will never fire to adopt the in-flight read ref, so release it here.
// No-op on the reader-callback paths, which already took it.
Comment thread
robobun marked this conversation as resolved.
Outdated
drop(self.take_read_ref());
Comment thread
robobun marked this conversation as resolved.

// Release the owner ref from `heap::into_raw` in `start()`. Every entry
// point that can reach here holds its own ref, so the free lands on
// that guard's drop, not here.
Expand Down Expand Up @@ -600,9 +606,20 @@ bun_io::impl_buffered_reader_parent! {
impl Drop for FileResponseStream {
fn drop(&mut self) {
bun_output::scoped_log!(FileResponseStream, "deinit");
// `self.reader` (BufferedReader) is torn down by its own `Drop` as a
// field — closes the poll handle. `bun.destroy(this)` is owned by
// `heap::take` in `deref`, not here.
// `start()` cleared CLOSE_HANDLE (auto_close owns the fd), so the
// reader's own `Drop` skips the handle. Unregister and free the
// FilePoll explicitly — it holds the event loop's active ref, which
// otherwise keeps the process alive forever — without closing the fd
// (same idiom as the shell `IOReader`). On Windows the reader's `Drop`
// hands its libuv source back to the loop itself.
Comment thread
robobun marked this conversation as resolved.
Outdated
#[cfg(unix)]
self.reader.with_mut(|reader| {
if matches!(reader.handle, bun_io::pipes::PollOrFd::Poll(_)) {
reader
.handle
.close_impl(None, None::<fn(*mut c_void)>, false);
}
});
if self.auto_close.get() {
#[cfg(windows)]
Closer::close(self.fd.get(), bun_sys::windows::libuv::Loop::get());
Expand Down
127 changes: 127 additions & 0 deletions test/js/bun/http/bun-serve-file.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1162,6 +1162,133 @@ test.skipIf(isWindows)("Response(Bun.file(FIFO)) frames the body as chunked, not
}
});

// Aborting a FIFO file response while the server's read is parked on its poll
// (pipe drained, no EOF) used to leak the stream: the in-flight read ref was
// only released by the reader callbacks, which never fire once nothing will
// ever write to the pipe again, and the armed FilePoll kept the event loop
// referenced. The process then never exited. The client receiving the first
// body chunk proves the server has drained the pipe and parked the read; the
// abort must tear the stream down and let the process exit on its own.
test.skipIf(isWindows)(
"process exits after a FIFO file response is aborted while its read is parked",
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
async () => {
using dir = tempDir("serve-fifo-abort-exit", {
"fixture.ts": `
import { openSync, writeSync, closeSync } from "node:fs";

const fifoPath = process.argv[2];
// r+ so open() never blocks and the server's reads EAGAIN (no EOF) after
// draining what we wrote.
const writerFd = openSync(fifoPath, "r+");
writeSync(writerFd, "first-chunk");

const server = Bun.serve({
port: 0,
hostname: "127.0.0.1",
fetch() {
return new Response(Bun.file(fifoPath));
},
});

const controller = new AbortController();
const res = await fetch("http://127.0.0.1:" + server.port + "/", { signal: controller.signal });
const reader = res.body.getReader();
// The server writes the chunk and then parks its read on the poll in the same
// synchronous read loop, so once this resolves the park has happened.
await reader.read();
controller.abort();

server.stop(true);
closeSync(writerFd);
console.log("aborted");
Comment thread
claude[bot] marked this conversation as resolved.
`,
});

const fifoPath = join(String(dir), "body.fifo");
mkfifo(fifoPath);

await using proc = Bun.spawn({
cmd: [bunExe(), "fixture.ts", fifoPath],
env: bunEnv,
cwd: String(dir),
stdout: "pipe",
stderr: "pipe",
});

const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);

expect(stdout.trim()).toBe("aborted");
expect(stderr).toBe("");
expect(exitCode).toBe(0);
},
15_000,
);

// Completing a FIFO file response also used to keep the process alive: the
// stream cleared the reader's CLOSE_HANDLE flag (it owns the fd itself), and
// the reader's own teardown skips the FilePoll in that mode, so the poll's
// event-loop active ref leaked even after the response finished at EOF.
test.skipIf(isWindows)(
"process exits after a FIFO file response completes at EOF",
async () => {
using dir = tempDir("serve-fifo-eof-exit", {
"fixture.ts": `
import { openSync, writeSync, closeSync } from "node:fs";

const fifoPath = process.argv[2];
const writerFd = openSync(fifoPath, "r+");
writeSync(writerFd, "fifo-body");

const server = Bun.serve({
port: 0,
hostname: "127.0.0.1",
fetch() {
return new Response(Bun.file(fifoPath));
},
});

const res = await fetch("http://127.0.0.1:" + server.port + "/");
const reader = res.body.getReader();
// First chunk proves the server opened the pipe and drained what we wrote;
// closing the last writer afterwards delivers EOF to the server's reader.
const first = await reader.read();
const body = Buffer.from(first.value).toString();
closeSync(writerFd);
// Drain to the end. How the server terminates the response body on a late
// empty EOF is a separate concern; this test only cares that the process
// exits, so a client-side framing error just ends the drain.
try {
for (;;) {
const { done } = await reader.read();
if (done) break;
}
} catch {}

server.stop(true);
console.log(body);
`,
});

const fifoPath = join(String(dir), "body.fifo");
mkfifo(fifoPath);

await using proc = Bun.spawn({
cmd: [bunExe(), "fixture.ts", fifoPath],
env: bunEnv,
cwd: String(dir),
stdout: "pipe",
stderr: "pipe",
});

const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);

expect(stdout.trim()).toBe("fifo-body");
expect(stderr).toBe("");
expect(exitCode).toBe(0);
},
15_000,
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
);

// A request that declares a body arms the request-body (onData) callback on
// the uWS response before the fetch handler runs. uWS keeps a single shared
// userdata slot per response, so when the handler returns a file response
Expand Down