Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
35 changes: 16 additions & 19 deletions src/jsc/node_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,9 @@ use crate::array_buffer::MarkedArrayBuffer;
// every early return between `to_thread_safe` and the manual cleanup.
// ──────────────────────────────────────────────────────────────────────────

/// Undo the `JSValue::protect()` calls taken by [`to_thread_safe`](
/// PathLike::to_thread_safe) (or an `args::*` type's `to_thread_safe`).
/// Undo the `JSValue::protect()` calls taken by an `args::*` type's
/// `to_thread_safe` (e.g. `StringOrBuffer`, which keeps JS-backed data
/// buffers zero-copy).
///
/// Implementations release **only** the JS-GC protect refcount — owned Rust
/// payloads (Vec, `SliceWithUnderlyingString`, …) are freed by the type's own
Expand Down Expand Up @@ -203,15 +204,14 @@ impl PathLike {
}
}

/// Promote any borrowed-JS
/// payload to a thread-safe representation. For `Buffer` the variant is
/// kept and the backing JS value is `protect()`ed (paired with
/// [`Unprotect::unprotect`]); the discriminant is preserved so callers
/// matching on `Buffer` after this call see the same shape.
/// Promote any borrowed-JS payload to a representation that references
/// no JS heap cell, so the path may be read from another thread and
/// dropped anywhere — including off-thread or inside a GC finalizer
/// (a `Blob` store holds its path for the cell's lifetime).
///
/// Prefer [`Self::into_thread_safe`] which returns a [`ThreadSafe`] guard;
/// this in-place form exists for nested calls from container types'
/// `to_thread_safe`.
/// A `Buffer` is copied into an owned `String` and its pin released here,
/// on the JS thread: a path is at most `MAX_PATH_BYTES`, and a snapshot
/// also means later JS writes to the buffer can't tear the path mid-read.
pub fn to_thread_safe(&mut self) {
match self {
Self::SliceWithUnderlyingString(s) => {
Expand All @@ -220,23 +220,20 @@ impl PathLike {
*self = Self::ThreadsafeString(owned);
}
Self::Buffer(b) => {
b.buffer.value.protect();
let owned = bun_core::handle_oom(CowSlice::init_dupe(b.slice()));
// Drops the `Buffer` arm, which unpins.
*self = Self::String(owned);
}
Self::String(_) | Self::ThreadsafeString(_) | Self::EncodedSlice(_) => {}
}
}
}

impl Unprotect for PathLike {
/// JS-side half of cleanup — undo
/// the `protect()` taken by [`Self::to_thread_safe`] /
/// `ArgumentsSlice::protect_eat`. Owned payloads are released by `Drop`.
/// Nothing to release: [`Self::to_thread_safe`] copies rather than
/// `protect()`s. Kept so container `args::*` types can forward uniformly.
#[inline]
fn unprotect(&mut self) {
if let Self::Buffer(b) = self {
b.buffer.value.unprotect();
}
}
fn unprotect(&mut self) {}
}

/// `node.PathOrFileDescriptor`.
Expand Down
1 change: 0 additions & 1 deletion src/runtime/webcore/Blob.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3653,7 +3653,6 @@ impl BlobExt for Blob {
}
}

path_or_fd.to_thread_safe();
core::mem::replace(
path_or_fd,
PathOrFileDescriptor::Path(crate::webcore::node_types::PathLike::String(
Expand Down
7 changes: 5 additions & 2 deletions src/runtime/webcore/blob/Store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,6 @@ impl StoreExt for Store {
credentials: S3Credentials,
) -> Result<Box<Store>, crate::Error> {
let mut path = pathlike;
// this actually protects/refs the pathlike
path.to_thread_safe();

// Compute the extension-derived fallback before moving `path` into the
Expand All @@ -144,9 +143,13 @@ impl StoreExt for Store {
}

fn init_file(
pathlike: PathOrFileDescriptor,
mut pathlike: PathOrFileDescriptor,
mime_type: Option<MimeType>,
) -> Result<Box<Store>, crate::Error> {
// A Store is shared across threads and dropped from the Blob cell's
// GC finalizer, so it must never hold a JS-backed path.
pathlike.to_thread_safe();

// Compute the extension-derived fallback before moving `pathlike` into
// the Store so we don't need to clone the owned PathOrFileDescriptor.
let mime_type = mime_type.or_else(|| match &pathlike {
Expand Down
45 changes: 45 additions & 0 deletions test/js/bun/util/bun-file.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,51 @@ test("Bun.file().arrayBuffer() errors include async stack frames", async () => {
expect(caught.stack).toContain("at async caller");
});

test("Bun.file() with a Buffer/Uint8Array path survives GC of the blobs", async () => {
// The file store used to keep the JS buffer pinned for the Blob's lifetime
// and unpin it from the Blob's GC destructor, touching a JS cell mid-sweep.
await using dir = tempDir("bun-file-buffer-path-gc", {
"hello.txt": "hello",
"run.js": `
const { join } = require("path");
const existing = join(process.argv[2], "hello.txt");
for (let i = 0; i < 2000; i++) {
Bun.file(Buffer.from(join(process.argv[2], "missing-" + i)));
Bun.file(new TextEncoder().encode(join(process.argv[2], "missing-u8-" + i)));
}
const keep = [Bun.file(Buffer.from(existing)), Bun.file(new TextEncoder().encode(existing))];
const pathBuf = Buffer.from(existing);
const fromMutated = Bun.file(pathBuf);
pathBuf.fill(0x78); // later writes to the buffer must not change the file's path
Bun.gc(true);
Bun.gc(true);
console.log(JSON.stringify({
exists: await Promise.all(keep.map(f => f.exists())),
text: await Promise.all(keep.map(f => f.text())),
fromMutated: await fromMutated.text(),
missing: await Bun.file(Buffer.from(join(process.argv[2], "missing-0"))).exists(),
}));
`,
});

await using proc = Bun.spawn({
cmd: [bunExe(), join(dir, "run.js"), dir],
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);

expect(stderr).toBe("");
expect(JSON.parse(stdout)).toEqual({
exists: [true, true],
text: ["hello", "hello"],
fromMutated: "hello",
missing: false,
});
expect(exitCode).toBe(0);
});

test("Bun.file().json() with UTF-8 BOM does not free an interior pointer", async () => {
// When a file starts with EF BB BF, the BOM is stripped before parsing and
// the temporary read buffer is freed. Previously the *post-strip* slice was
Expand Down