Skip to content
Open
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
16 changes: 10 additions & 6 deletions src/jsc/node_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,12 +114,15 @@ impl Clone for PathLike {
} else {
s.borrow()
}),
Self::Buffer(b) => Self::Buffer(MarkedArrayBuffer {
buffer: b.buffer,
// The clone borrows the JS-owned backing store; only the
// original (if any) owns the allocation.
owns_buffer: false,
pinned: false,
Self::Buffer(b) => Self::Buffer(if b.owns_buffer {
// Owned snapshot: dupe so the clone's `Drop` is independent.
bun_core::handle_oom(MarkedArrayBuffer::from_string(b.slice()))
} else {
MarkedArrayBuffer {
buffer: b.buffer,
owns_buffer: false,
pinned: false,
}
}),
Self::SliceWithUnderlyingString(s) => {
// `dupe_ref()` alone leaves `utf8` empty (lib.rs:1603) — a
Expand Down Expand Up @@ -155,6 +158,7 @@ impl Drop for PathLike {
b.pinned = false;
b.buffer.unpin();
}
b.destroy();
}
Self::SliceWithUnderlyingString(s) | Self::ThreadsafeString(s) => {
core::mem::take(s).deinit();
Expand Down
30 changes: 28 additions & 2 deletions src/runtime/node/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -915,6 +915,22 @@ pub(crate) trait PathOrFdExt {
Self: Sized;
}

/// `pin()` blocks `transfer()` but not `ArrayBuffer.prototype.resize`, which
/// decommits tail pages: a later `slice_z` (after an option getter) or a
/// work-pool read faults. Copy resizable non-shared paths; growable SABs only
/// grow in place so stay borrowed.
Comment thread
robobun marked this conversation as resolved.
fn snapshot_resizable_path_buffer(buffer: &mut Buffer) {
if !(buffer.buffer.resizable && !buffer.buffer.shared) {
return;
}
let copy = bun_core::handle_oom(Buffer::from_string(buffer.slice()));
if buffer.pinned {
buffer.pinned = false;
buffer.buffer.unpin();
}
*buffer = copy;
}

impl PathLikeExt for PathLike {
// Const-generics can't change return mutability, so this always returns
// `&ZStr`. A future force=true caller that needs `&mut ZStr` will need a
Expand Down Expand Up @@ -1151,8 +1167,13 @@ impl PathLikeExt for PathLike {
}
return Err(err);
}
snapshot_resizable_path_buffer(&mut buffer);

arguments.protect_eat();
if buffer.owns_buffer {
arguments.eat();
} else {
arguments.protect_eat();
}
Ok(Some(Self::Buffer(buffer)))
}

Expand All @@ -1168,8 +1189,13 @@ impl PathLikeExt for PathLike {
}
return Err(err);
}
snapshot_resizable_path_buffer(&mut buffer);

arguments.protect_eat();
if buffer.owns_buffer {
arguments.eat();
} else {
arguments.protect_eat();
}
Ok(Some(Self::Buffer(buffer)))
}

Expand Down
115 changes: 115 additions & 0 deletions test/js/node/fs/fs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5742,6 +5742,121 @@ it("fs.promises.writeFile keeps a buffer path argument attached while options ar
expect(readFileSync(file, "utf8")).toBe("hello world");
});

it("fs path buffers backed by a resizable ArrayBuffer are snapshotted before option getters run", async () => {
// Pinning the backing ArrayBuffer blocks transfer()/detach but not
// ArrayBuffer.prototype.resize: shrinking decommits the tail pages, so a
// later read of the captured (ptr, len) faults. That later read can be a
// sync op's own option getter (writeFileSync reading `flag`) or a work-pool
// thread for an async op. The path bytes must instead be copied at capture
// time. Growable SharedArrayBuffers never shrink so they stay zero-copy.
using dir = tempDir("fs-rab-path", {
"rab-path-fixture.js": String.raw`
import fs from "node:fs";
import path from "node:path";

const dir = process.cwd();

function resizablePath(p) {
const bytes = Buffer.from(p);
const rab = new ArrayBuffer(bytes.length, { maxByteLength: 64 * 1024 });
new Uint8Array(rab).set(bytes);
return { rab, view: new Uint8Array(rab, 0, bytes.length) };
}

// sync: writeFileSync — Uint8Array path, getter on the options object
// resizes the backing store to 0 between capture and use.
{
const file = path.join(dir, "w.txt");
const { rab, view } = resizablePath(file);
fs.writeFileSync(view, "sync-write", {
get flag() { rab.resize(0); return "w"; },
});
if (fs.readFileSync(file, "utf8") !== "sync-write") throw new Error("writeFileSync lost path");
}

// sync: readFileSync — DataView path over a resizable ArrayBuffer.
{
const file = path.join(dir, "r.txt");
fs.writeFileSync(file, "sync-read");
const bytes = Buffer.from(file);
const rab = new ArrayBuffer(bytes.length, { maxByteLength: 64 * 1024 });
new Uint8Array(rab).set(bytes);
const dv = new DataView(rab, 0, bytes.length);
const got = fs.readFileSync(dv, {
get encoding() { rab.resize(0); return "utf8"; },
});
if (got !== "sync-read") throw new Error("readFileSync lost path: " + JSON.stringify(got));
}

// sync: mkdirSync — resizable ArrayBuffer passed directly as the path.
{
const target = path.join(dir, "made");
const bytes = Buffer.from(target);
const rab = new ArrayBuffer(bytes.length, { maxByteLength: bytes.length });
new Uint8Array(rab).set(bytes);
fs.mkdirSync(rab, {
get recursive() { rab.resize(0); return true; },
});
if (!fs.existsSync(target)) throw new Error("mkdirSync lost path");
}

// async: rename — shrinking right after the call must not affect the
// work-pool thread's read of the path bytes.
{
const src = path.join(dir, "src.txt");
fs.writeFileSync(src, "hi");
const { rab, view } = resizablePath(src);
const renamed = fs.promises.rename(view, path.join(dir, "dst.txt"));
rab.resize(0);
await renamed;
if (!fs.existsSync(path.join(dir, "dst.txt"))) throw new Error("rename lost path");
}

// Growable SharedArrayBuffer: stays zero-copy (only grows in place), so
// the path is read from the live backing and the write still lands.
{
const file = path.join(dir, "sab.txt");
const bytes = Buffer.from(file);
const sab = new SharedArrayBuffer(bytes.length, { maxByteLength: 64 * 1024 });
new Uint8Array(sab).set(bytes);
fs.writeFileSync(new Uint8Array(sab, 0, bytes.length), "sab-write", {
get flag() { sab.grow(bytes.length + 16); return "w"; },
});
if (fs.readFileSync(file, "utf8") !== "sab-write") throw new Error("SAB path failed");
}

// Bun.file(): captures the path via the same PathLike funnel (owned
// snapshot for resizable). .text() clones the stored PathLike on the JS
// thread and again on the worker; the owned-buffer Clone arm must dupe
// so both clones are independently droppable.
{
const file = path.join(dir, "bunfile.txt");
fs.writeFileSync(file, "bun-file");
const { rab, view } = resizablePath(file);
const f = Bun.file(view);
rab.resize(0);
const got = await f.text();
if (got !== "bun-file") throw new Error("Bun.file().text() lost path: " + JSON.stringify(got));
// Second read reuses the stored owned PathLike via a fresh clone; the
// original must not have been freed by the first.
if ((await f.text()) !== "bun-file") throw new Error("Bun.file().text() second read failed");
}

console.log("done");
`,
});

await using proc = Bun.spawn({
cmd: [bunExe(), "rab-path-fixture.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({ stdout, stderr, exitCode }).toEqual({ stdout: "done\n", stderr: "", exitCode: 0 });
});

describe("fs.close on stdio descriptors", () => {
it.skipIf(isWindows)("closeSync(2) actually closes fd 2 and allows redirect", async () => {
using dir = tempDir("fs-close-stdio", {
Expand Down
Loading