From be3ca0b8910e467535644093a4b9746233628b35 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 12 Jun 2026 16:46:43 +0000 Subject: [PATCH 1/3] node:fs: snapshot path buffers backed by resizable ArrayBuffers for async ops Async fs operations capture a raw ptr/len into a buffer-typed path argument on the JS thread and read it later on a work-pool thread. The pin taken at parse time blocks transfer()/detach, but ArrayBuffer.prototype.resize does not consult the pin count: shrinking a resizable ArrayBuffer decommits its tail pages, so the pool thread's read of the stale snapshot faults. Copy the path bytes at parse time when the backing store is a non-shared resizable ArrayBuffer (paths are bounded by MAX_PATH_BYTES, and Node also copies buffer paths at call time). Growable SharedArrayBuffers never shrink and keep a stable data pointer, so they stay on the zero-copy pinned path. PathLike::Drop now frees the owned snapshot, and PathLike::Clone dupes an owned payload instead of borrowing it, mirroring the owned-String arm. --- src/jsc/node_path.rs | 22 ++++++++---- src/runtime/node/types.rs | 22 ++++++++++++ test/js/node/fs/fs.test.ts | 72 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 110 insertions(+), 6 deletions(-) diff --git a/src/jsc/node_path.rs b/src/jsc/node_path.rs index 3f2efd20713f..6dec33d7ac9e 100644 --- a/src/jsc/node_path.rs +++ b/src/jsc/node_path.rs @@ -114,12 +114,19 @@ 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 { + // An owned snapshot (resizable-backed async path) is freed by + // the original's `Drop`; dupe so the clone is independently + // droppable, mirroring the owned-`String` arm above. + bun_core::handle_oom(MarkedArrayBuffer::from_string(b.slice())) + } else { + 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::SliceWithUnderlyingString(s) => { // `dupe_ref()` alone leaves `utf8` empty (lib.rs:1603) — a @@ -155,6 +162,9 @@ impl Drop for PathLike { b.pinned = false; b.buffer.unpin(); } + // Frees the snapshot copy taken for resizable-backed async + // paths (`owns_buffer`); no-op for JS-owned backings. + b.destroy(); } Self::SliceWithUnderlyingString(s) | Self::ThreadsafeString(s) => { core::mem::take(s).deinit(); diff --git a/src/runtime/node/types.rs b/src/runtime/node/types.rs index cce2e751b754..29d3d1042b2d 100644 --- a/src/runtime/node/types.rs +++ b/src/runtime/node/types.rs @@ -1010,6 +1010,26 @@ pub(crate) trait PathOrFdExt { Self: Sized; } +/// An async fs task snapshots the buffer's `ptr`/`len` on the JS thread and +/// reads them later on a work-pool thread. The pin taken by +/// [`Buffer::from_js_pinned`] blocks `transfer()`/detach but not +/// `ArrayBuffer.prototype.resize`: shrinking a resizable ArrayBuffer decommits +/// its tail pages (`JSC::ArrayBuffer::resize` → `OSAllocator::protect`), so the +/// pool thread's read of the stale snapshot faults. Copy the path bytes instead +/// (Node also copies buffer paths at call time). Growable SharedArrayBuffers +/// never shrink and keep a stable data pointer, so they stay zero-copy. +fn snapshot_resizable_path_buffer(buffer: &mut Buffer, will_be_async: bool) { + if !(will_be_async && 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 @@ -1246,6 +1266,7 @@ impl PathLikeExt for PathLike { } return Err(err); } + snapshot_resizable_path_buffer(&mut buffer, arguments.will_be_async); arguments.protect_eat(); Ok(Some(Self::Buffer(buffer))) @@ -1263,6 +1284,7 @@ impl PathLikeExt for PathLike { } return Err(err); } + snapshot_resizable_path_buffer(&mut buffer, arguments.will_be_async); arguments.protect_eat(); Ok(Some(Self::Buffer(buffer))) diff --git a/test/js/node/fs/fs.test.ts b/test/js/node/fs/fs.test.ts index 8d081789ba98..11c5f59dd8c9 100644 --- a/test/js/node/fs/fs.test.ts +++ b/test/js/node/fs/fs.test.ts @@ -5231,3 +5231,75 @@ describe("fs.close on stdio descriptors", () => { expect(exitCode).toBe(0); }); }); + +it("async fs ops snapshot path buffers backed by resizable ArrayBuffers before a shrink can decommit them", async () => { + // Pinning the backing ArrayBuffer blocks transfer()/detach but not + // ArrayBuffer.prototype.resize: shrinking decommits the tail pages, so a + // work-pool thread reading the path bytes after the shrink would fault. + // The path bytes must instead be copied at call time (as Node does). + 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(); + + // A resizable-backed path arg is snapshotted at call time: shrinking + // right after the call must not affect the rename. + { + const src = path.join(dir, "real-src.txt"); + fs.writeFileSync(src, "hi"); + const srcBytes = Buffer.from(src); + const rab = new ArrayBuffer(64 * 1024, { maxByteLength: 64 * 1024 }); + new Uint8Array(rab).set(srcBytes); + const view = new Uint8Array(rab, 0, srcBytes.length); + const renamed = fs.promises.rename(view, path.join(dir, "real-dest.txt")); + rab.resize(0); + await renamed; + if (!fs.existsSync(path.join(dir, "real-dest.txt"))) throw new Error("rename did not happen"); + } + + // Uint8Array view over a resizable ArrayBuffer, shrunk while hundreds + // of renames are still queued on the work pool. + { + const srcBytes = Buffer.from(path.join(dir, "missing-view")); + const rab = new ArrayBuffer(128 * 1024, { maxByteLength: 128 * 1024 }); + new Uint8Array(rab).set(srcBytes); + const view = new Uint8Array(rab, 0, srcBytes.length); + const dest = path.join(dir, "dest-view"); + const all = []; + for (let i = 0; i < 256; i++) all.push(fs.promises.rename(view, dest).catch(err => err.code)); + rab.resize(0); + const codes = await Promise.all(all); + const bad = codes.find(c => c !== "ENOENT"); + if (bad !== undefined) throw new Error("expected ENOENT, got: " + bad); + } + + // Resizable ArrayBuffer passed directly as the path. + { + const srcBytes = Buffer.from(path.join(dir, "missing-direct")); + const rab = new ArrayBuffer(srcBytes.length, { maxByteLength: srcBytes.length }); + new Uint8Array(rab).set(srcBytes); + const dest = path.join(dir, "dest-direct"); + const all = []; + for (let i = 0; i < 256; i++) all.push(fs.promises.rename(rab, dest).catch(err => err.code)); + rab.resize(0); + const codes = await Promise.all(all); + const bad = codes.find(c => c !== "ENOENT"); + if (bad !== undefined) throw new Error("expected ENOENT, got: " + bad); + } + + 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, exitCode }).toEqual({ stdout: "done\n", exitCode: 0 }); +}); From 123994f619af5842f6c49f0a0b6151cb8f27471b Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 12 Jun 2026 17:04:28 +0000 Subject: [PATCH 2/3] Skip protect_eat for owned path snapshots, strengthen test matrix The owned snapshot no longer references the JS backing store, so rooting the original argument pairs with no release (the success path never drops the ArgumentsSlice). Plain eat() keeps the snapshot path balanced. Test: assert via every() so an unexpected resolution cannot slip through find()'s undefined result, and cover DataView as the third buffer form. --- src/runtime/node/types.rs | 16 ++++++++++++-- test/js/node/fs/fs.test.ts | 44 ++++++++++++++++++++++++-------------- 2 files changed, 42 insertions(+), 18 deletions(-) diff --git a/src/runtime/node/types.rs b/src/runtime/node/types.rs index 29d3d1042b2d..5d8fc91c6bd4 100644 --- a/src/runtime/node/types.rs +++ b/src/runtime/node/types.rs @@ -1268,7 +1268,13 @@ impl PathLikeExt for PathLike { } snapshot_resizable_path_buffer(&mut buffer, arguments.will_be_async); - arguments.protect_eat(); + if buffer.owns_buffer { + // The owned snapshot no longer references the JS backing + // store, so the original value needs no GC root. + arguments.eat(); + } else { + arguments.protect_eat(); + } Ok(Some(Self::Buffer(buffer))) } @@ -1286,7 +1292,13 @@ impl PathLikeExt for PathLike { } snapshot_resizable_path_buffer(&mut buffer, arguments.will_be_async); - arguments.protect_eat(); + if buffer.owns_buffer { + // The owned snapshot no longer references the JS backing + // store, so the original value needs no GC root. + arguments.eat(); + } else { + arguments.protect_eat(); + } Ok(Some(Self::Buffer(buffer))) } diff --git a/test/js/node/fs/fs.test.ts b/test/js/node/fs/fs.test.ts index 11c5f59dd8c9..ae9eafcb93bc 100644 --- a/test/js/node/fs/fs.test.ts +++ b/test/js/node/fs/fs.test.ts @@ -5259,20 +5259,38 @@ it("async fs ops snapshot path buffers backed by resizable ArrayBuffers before a if (!fs.existsSync(path.join(dir, "real-dest.txt"))) throw new Error("rename did not happen"); } - // Uint8Array view over a resizable ArrayBuffer, shrunk while hundreds - // of renames are still queued on the work pool. + // Issues 256 renames of a missing source, then shrinks the backing + // store while they are still queued on the work pool. Every rename must + // reject with ENOENT; a resolution or any other code means the path + // bytes were not snapshotted correctly. + async function expectAllEnoent(pathArg, rab, dest) { + const all = []; + for (let i = 0; i < 256; i++) { + all.push(fs.promises.rename(pathArg, dest).then(() => "resolved", err => err.code)); + } + rab.resize(0); + const codes = await Promise.all(all); + if (!codes.every(c => c === "ENOENT")) { + throw new Error("expected all ENOENT, got: " + JSON.stringify([...new Set(codes)])); + } + } + + // Uint8Array view over a resizable ArrayBuffer. { const srcBytes = Buffer.from(path.join(dir, "missing-view")); const rab = new ArrayBuffer(128 * 1024, { maxByteLength: 128 * 1024 }); new Uint8Array(rab).set(srcBytes); const view = new Uint8Array(rab, 0, srcBytes.length); - const dest = path.join(dir, "dest-view"); - const all = []; - for (let i = 0; i < 256; i++) all.push(fs.promises.rename(view, dest).catch(err => err.code)); - rab.resize(0); - const codes = await Promise.all(all); - const bad = codes.find(c => c !== "ENOENT"); - if (bad !== undefined) throw new Error("expected ENOENT, got: " + bad); + await expectAllEnoent(view, rab, path.join(dir, "dest-view")); + } + + // DataView over a resizable ArrayBuffer. + { + const srcBytes = Buffer.from(path.join(dir, "missing-dataview")); + const rab = new ArrayBuffer(128 * 1024, { maxByteLength: 128 * 1024 }); + new Uint8Array(rab).set(srcBytes); + const view = new DataView(rab, 0, srcBytes.length); + await expectAllEnoent(view, rab, path.join(dir, "dest-dataview")); } // Resizable ArrayBuffer passed directly as the path. @@ -5280,13 +5298,7 @@ it("async fs ops snapshot path buffers backed by resizable ArrayBuffers before a const srcBytes = Buffer.from(path.join(dir, "missing-direct")); const rab = new ArrayBuffer(srcBytes.length, { maxByteLength: srcBytes.length }); new Uint8Array(rab).set(srcBytes); - const dest = path.join(dir, "dest-direct"); - const all = []; - for (let i = 0; i < 256; i++) all.push(fs.promises.rename(rab, dest).catch(err => err.code)); - rab.resize(0); - const codes = await Promise.all(all); - const bad = codes.find(c => c !== "ENOENT"); - if (bad !== undefined) throw new Error("expected ENOENT, got: " + bad); + await expectAllEnoent(rab, rab, path.join(dir, "dest-direct")); } console.log("done"); From ce0eb65f17ef84146e66495b3b25f5f71246269d Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 12 Jun 2026 19:37:36 +0000 Subject: [PATCH 3/3] Assert empty stderr in resizable path buffer test Surfaces the child's crash report or fixture error in the failure diff instead of only the exit code. --- test/js/node/fs/fs.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/js/node/fs/fs.test.ts b/test/js/node/fs/fs.test.ts index ae9eafcb93bc..0d86a232afaa 100644 --- a/test/js/node/fs/fs.test.ts +++ b/test/js/node/fs/fs.test.ts @@ -5313,5 +5313,5 @@ it("async fs ops snapshot path buffers backed by resizable ArrayBuffers before a stderr: "pipe", }); const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - expect({ stdout, exitCode }).toEqual({ stdout: "done\n", exitCode: 0 }); + expect({ stdout, stderr, exitCode }).toEqual({ stdout: "done\n", stderr: "", exitCode: 0 }); });