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
26 changes: 20 additions & 6 deletions src/runtime/webcore/FileReader.zig
Original file line number Diff line number Diff line change
Expand Up @@ -320,14 +320,23 @@ pub fn onReadChunk(this: *@This(), init_buf: []const u8, state: bun.io.ReadState

if (buf.len > 0) {
if (this.max_size) |max_size| {
if (this.total_readed >= max_size) return false;
if (this.total_readed >= max_size) {
// We already delivered `max_size` bytes on a previous chunk.
// Close the reader so `isDone()` becomes true; otherwise
// `onPull` will return `.pending` forever for non-pollable
// regular files.
close = true;
return false;
}
const len = @min(max_size - this.total_readed, buf.len);
if (buf.len > len) {
buf = buf[0..len];
}
this.total_readed += len;

if (buf.len == 0) {
if (this.total_readed >= max_size) {
// This chunk satisfies the slice; treat it as the final one
// and close the reader after delivering it.
close = true;
hasMore = false;
}
Expand Down Expand Up @@ -391,7 +400,7 @@ pub fn onReadChunk(this: *@This(), init_buf: []const u8, state: bun.io.ReadState
return false;
}

const was_done = this.reader.isDone();
const was_done = this.reader.isDone() or close;

if (this.pending_view.len >= buf.len) {
@memcpy(this.pending_view[0..buf.len], buf);
Expand All @@ -411,7 +420,7 @@ pub fn onReadChunk(this: *@This(), init_buf: []const u8, state: bun.io.ReadState
}

if (bun.isSliceInBuffer(buf, reader_buffer.allocatedSlice())) {
if (this.reader.isDone()) {
if (was_done) {
bun.assert_eql(buf.ptr, reader_buffer.items.ptr);
var buffer = reader_buffer.moveToUnmanaged();
buffer.shrinkRetainingCapacity(buf.len);
Expand All @@ -424,7 +433,7 @@ pub fn onReadChunk(this: *@This(), init_buf: []const u8, state: bun.io.ReadState
}

if (!bun.isSliceInBuffer(buf, this.buffered.allocatedSlice())) {
this.pending.result = if (this.reader.isDone())
this.pending.result = if (was_done)
.{ .temporary_and_done = .fromBorrowedSliceDangerous(buf) }
else
.{ .temporary = .fromBorrowedSliceDangerous(buf) };
Expand All @@ -436,7 +445,7 @@ pub fn onReadChunk(this: *@This(), init_buf: []const u8, state: bun.io.ReadState
this.buffered = .{};
buffered.shrinkRetainingCapacity(buf.len);

this.pending.result = if (this.reader.isDone())
this.pending.result = if (was_done)
.{ .owned_and_done = .moveFromList(&buffered) }
else
.{ .owned = .moveFromList(&buffered) };
Expand All @@ -448,6 +457,11 @@ pub fn onReadChunk(this: *@This(), init_buf: []const u8, state: bun.io.ReadState
}
}

// When `close` is set we've just scheduled `this.reader.close()` via the
// defer above; returning true here would let the low-level read loop
// issue another read against the now-closed fd.
if (close) return false;

// For pipes, we have to keep pulling or the other process will block.
return this.read_inside_on_pull != .temporary and
!(this.buffered.items.len + reader_buffer.items.len >= this.highwater_mark and
Expand Down
78 changes: 78 additions & 0 deletions test/regression/issue/18192.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
// https://github.com/oven-sh/bun/issues/18192
// Bun.file(path).slice(...).stream() would hang forever when the underlying
// file was larger than 640 KiB, because FileReader.onReadChunk() stopped
// asking for more data once max_size was reached but never closed the
// underlying reader — leaving isDone() == false and the next onPull()
// parked on a pending promise that nothing would ever resolve (regular
// files are not pollable).
import { describe, expect, test } from "bun:test";
import { bunEnv, bunExe, tempDir } from "harness";

describe.concurrent("#18192 Bun.file().slice().stream() on large files", () => {
for (const fileSize of [512 * 1024, 640 * 1024, 640 * 1024 + 1, 768 * 1024, 2 * 1024 * 1024]) {
test(`slice(0, 1) on a ${fileSize}-byte file does not hang`, async () => {
using dir = tempDir("issue-18192", {
"run.js": `
import { writeFileSync } from "fs";
writeFileSync("data", Buffer.alloc(${fileSize}, 0x41));
const text = await Bun.readableStreamToText(Bun.file("data").slice(0, 1).stream());
console.log(JSON.stringify({ len: text.length, first: text }));
`,
});

await using proc = Bun.spawn({
cmd: [bunExe(), "run.js"],
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(stderr).toBe("");
expect(stdout.trim()).toBe(JSON.stringify({ len: 1, first: "A" }));
expect(exitCode).toBe(0);
});
}

test("slice(start, end) on a large file yields the correct bytes", async () => {
using dir = tempDir("issue-18192", {
"run.js": `
import { writeFileSync } from "fs";
const size = 1024 * 1024;
const buf = Buffer.alloc(size);
for (let i = 0; i < size; i++) buf[i] = i % 256;
writeFileSync("data", buf);

for (const [start, end] of [[0, 1], [5, 10], [300_000, 300_005], [0, 300_000], [700_000, 700_001]]) {
const chunks = [];
for await (const chunk of Bun.file("data").slice(start, end).stream()) {
chunks.push(chunk);
}
const got = Buffer.concat(chunks);
const want = buf.subarray(start, end);
if (!got.equals(want)) {
console.log("FAIL", start, end, "got", got.length, "bytes, want", want.length);
process.exit(1);
}
}
console.log("OK");
`,
});

await using proc = Bun.spawn({
cmd: [bunExe(), "run.js"],
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(stderr).toBe("");
expect(stdout.trim()).toBe("OK");
expect(exitCode).toBe(0);
});
});
Loading