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
15 changes: 10 additions & 5 deletions src/runtime/node/node_fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5078,7 +5078,8 @@ impl NodeFS {
if matches!(err.get_errno(), E::ENOSYS | E::EOPNOTSUPP) {
sys::copy_file::disable_copy_file_range_syscall();
}
let mut w = wrote.get();
// copy_file_range did not move the fd offsets, so the fallback starts again at 0.
let mut w = 0;
let r = Self::copy_file_using_sendfile_on_linux_with_read_write_fallback(src, dest, src_fd, dest_fd, size, &mut w);
wrote.set(w);
return r;
Expand Down Expand Up @@ -5116,7 +5117,8 @@ impl NodeFS {
if matches!(err.get_errno(), E::ENOSYS | E::EOPNOTSUPP) {
sys::copy_file::disable_copy_file_range_syscall();
}
let mut w = wrote.get();
// copy_file_range did not move the fd offsets, so the fallback starts again at 0.
let mut w = 0;
let r = Self::copy_file_using_sendfile_on_linux_with_read_write_fallback(src, dest, src_fd, dest_fd, size, &mut w);
wrote.set(w);
return r;
Expand Down Expand Up @@ -8530,7 +8532,8 @@ impl NodeFS {
if matches!(err.get_errno(), E::ENOSYS | E::EOPNOTSUPP) {
sys::copy_file::disable_copy_file_range_syscall();
}
let mut w = wrote.get();
// copy_file_range did not move the fd offsets, so the fallback starts again at 0.
let mut w = 0;
let r = Self::copy_file_using_sendfile_on_linux_with_read_write_fallback(src, dest, src_fd, dest_fd, size, &mut w);
wrote.set(w);
return r;
Comment on lines 8532 to 8539

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟣 pre-existing, not blocking: pre-existing: a caller of fs.cpSync on one file can get a spurious EINTR error on Linux, where the async twin retries. The sync copy_file_range match at node_fs.rs:8526 and :8570 has no E::EINTR => continue arm, so EINTR falls to _ => return err and the copy aborts. The async copy_file_inner twin handles it at :5076 and :5115. Fix: handle EINTR the same way in both copy_single_file_sync loops (retry the call), so sync and async copies behave alike. Same pattern at 2 sites (node_fs.rs:8531, node_fs.rs:8575).
A small fix can ride a push you are already making; otherwise a short reply is enough.

Extended reasoning...

The base branch behaves the same way; this diff edits the fallback arm of these two match blocks but leaves the EINTR gap. A user calls fs.cpSync(srcFile, destFile) with default options on Linux, which routes to copy_single_file_sync. copy_file_range is issued at node_fs.rs:8512 or :8556. The kernel's splice-based path in copy_file_range can return -ERESTARTSYS when a signal is pending before any bytes are copied in that call, which surfaces as EINTR when the handler was installed without SA_RESTART. The match at :8526-8542 (size 0 loop) and :8570-8586 (sized loop) lists only EXDEV/ENOSYS/EINVAL/EOPNOTSUPP, so EINTR hits _ => return err at :8541 / :8585. cpSync throws an EINTR error and the destination is left partially written and ftruncated to the partial wrote by the scopeguard at :8484. The async twin in copy_file_inner has E::EINTR => continue at :5076 and :5115, so fs.promises.cp / fs.copyFile retry and succeed on the same event. A correct fix adds the EINTR retry arm at both sync sites.

Verification: pre-existing. Triggering condition: a signal whose handler was installed without SA_RESTART lands on the thread while copy_file_range is in the kernel's splice-based path with zero bytes progressed in that call (splice_from_pipe_next returns -ERESTARTSYS, surfaced as EINTR), during fs.cpSync/fs.cp of a single regular file on Linux. Mechanism verified in… | pre-existing — the base commit has the…

Expand Down Expand Up @@ -8573,7 +8576,8 @@ impl NodeFS {
if matches!(err.get_errno(), E::ENOSYS | E::EOPNOTSUPP) {
sys::copy_file::disable_copy_file_range_syscall();
}
let mut w = wrote.get();
// copy_file_range did not move the fd offsets, so the fallback starts again at 0.
let mut w = 0;
let r = Self::copy_file_using_sendfile_on_linux_with_read_write_fallback(src, dest, src_fd, dest_fd, size, &mut w);
wrote.set(w);
return r;
Expand Down Expand Up @@ -8712,7 +8716,8 @@ impl NodeFS {
}
}

let mut w = wrote.get();
// copy_file_range did not move the fd offsets, so the fallback starts again at 0.
let mut w = 0;
let r = Self::copy_file_using_read_write_loop(src, dest, src_fd, dest_fd, size, &mut w);
wrote.set(w);
return r;
Expand Down
158 changes: 157 additions & 1 deletion test/js/node/fs/cp.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { describe, expect, jest, test } from "bun:test";
import { afterAll, beforeAll, describe, expect, jest, test } from "bun:test";
import fs from "fs";
import { bunEnv, bunExe, isLinux, isPosix, isWindows, tempDir } from "harness";
import { mkfifo } from "mkfifo";
Expand Down Expand Up @@ -716,3 +716,159 @@ test.skipIf(!isLinux)("fs.cp and fs.copyFile create the destination with the sou
expect(stdout).toBe("");
expect(exitCode).toBe(0);
});

// copy_file_range() can copy part of a file and then fail with an errno that
// selects the fallback (rust-lang/rust#91152 saw it on a network share). An
// LD_PRELOAD shim stands in for such a mount: the first call copies one chunk
// of at most 4096 bytes and moves the offsets like the kernel does (the
// caller's offsets, or the fd offsets when the pointers are null), and each
// later call fails with EXDEV. Bun issues copy_file_range through syscall(2),
// so the shim interposes syscall().
const cc = isLinux ? Bun.which("cc") || Bun.which("gcc") || Bun.which("clang") : null;

const COPY_FILE_RANGE_SHIM_C = /* c */ `
#define _GNU_SOURCE
#include <dlfcn.h>
#include <errno.h>
#include <stdarg.h>
#include <stdio.h>
#include <sys/syscall.h>
#include <unistd.h>

static long (*real_syscall)(long, long, long, long, long, long, long);
static int copy_file_range_calls;

static long partial_copy_file_range(int in, long long *off_in, int out, long long *off_out, size_t len) {
char buf[4096];
ssize_t n;
if (copy_file_range_calls++ > 0) {
dprintf(2, "shim: copy_file_range = EXDEV\\n");
errno = EXDEV;
return -1;
}
if (len > sizeof(buf)) len = sizeof(buf);
n = off_in ? pread(in, buf, len, *off_in) : read(in, buf, len);
if (n > 0) n = off_out ? pwrite(out, buf, (size_t)n, *off_out) : write(out, buf, (size_t)n);
if (n > 0) {
if (off_in) *off_in += n;
if (off_out) *off_out += n;
}
dprintf(2, "shim: copy_file_range = %zd\\n", n);
return n;
}

long syscall(long number, ...) {
va_list ap;
long a, b, c, d, e, f;
va_start(ap, number);
a = va_arg(ap, long);
b = va_arg(ap, long);
c = va_arg(ap, long);
d = va_arg(ap, long);
e = va_arg(ap, long);
f = va_arg(ap, long);
va_end(ap);
if (number == SYS_copy_file_range) {
return partial_copy_file_range((int)a, (long long *)b, (int)c, (long long *)d, (size_t)e);
}
if (!real_syscall) {
real_syscall = (long (*)(long, long, long, long, long, long, long))dlsym(RTLD_NEXT, "syscall");
}
return real_syscall(number, a, b, c, d, e, f);
}
`;

describe.skipIf(!isLinux || !cc)("node:fs copy when copy_file_range fails after a partial copy", () => {
let dir: ReturnType<typeof tempDir> | undefined;
let shimPath: string;

beforeAll(async () => {
const source = Buffer.alloc(10000, "0123456789").toString();
dir = tempDir("cp-copy-file-range-partial", {
"shim.c": COPY_FILE_RANGE_SHIM_C,
"known-size.bin": source,
"tree/known-size.bin": source,
"longer-dest.bin": Buffer.alloc(20000, "x").toString(),
});
shimPath = join(String(dir), "shim.so");
await using ccProc = Bun.spawn({
cmd: [cc!, "-shared", "-fPIC", "-o", shimPath, join(String(dir), "shim.c"), "-ldl"],
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
const [ccOut, ccErr, ccExit] = await Promise.all([ccProc.stdout.text(), ccProc.stderr.text(), ccProc.exited]);
if (ccExit !== 0) {
throw new Error(`shim compile failed: ${ccErr || ccOut}`);
}
});

afterAll(() => {
dir?.[Symbol.dispose]();
});

// One child process per row, so each copy gets the shim's first call.
// Columns: name, code the child runs with `src` and `dest` defined, src, dest, file to compare inside a copied directory.
test.concurrent.each([
// copy_file_range is asked for the whole file.
["cpSync(file)", "fs.cpSync(src, dest)", "known-size.bin", "cpSync.bin", ""],
["copyFileSync(file)", "fs.copyFileSync(src, dest)", "known-size.bin", "copyFileSync.bin", ""],
["promises.cp(file)", "await fs.promises.cp(src, dest)", "known-size.bin", "promises-cp.bin", ""],
["promises.copyFile(file)", "await fs.promises.copyFile(src, dest)", "known-size.bin", "promises-copyFile.bin", ""],
// On Linux the directory walker is in JS and copies each file with copyFile.
[
"cpSync(dir, { recursive: true })",
"fs.cpSync(src, dest, { recursive: true })",
"tree",
"tree-copy",
"known-size.bin",
],
// A procfs file reports st_size 0, so copy_file_range is called in page-size steps until EOF.
["cpSync(file), source reports size 0", "fs.cpSync(src, dest)", "/proc/version", "cpSync-proc-version", ""],
[
"copyFileSync(file), source reports size 0",
"fs.copyFileSync(src, dest)",
"/proc/version",
"copyFileSync-proc-version",
"",
],
// The destination is not truncated at open, so a count that is too large keeps old bytes.
["cpSync(file) over a longer destination", "fs.cpSync(src, dest)", "known-size.bin", "longer-dest.bin", ""],
])("%s", async (_name, run, srcName, destName, fileInDir) => {
const src = isAbsolute(srcName) ? srcName : join(String(dir), srcName);
const dest = join(String(dir), destName);
const script = `
const fs = require("node:fs");
const src = ${JSON.stringify(src)};
const dest = ${JSON.stringify(dest)};
${run};
`;
await using proc = Bun.spawn({
cmd: [bunExe(), "-e", script],
env: {
...bunEnv,
LD_PRELOAD: bunEnv.LD_PRELOAD ? `${shimPath}:${bunEnv.LD_PRELOAD}` : shimPath,
// A successful FICLONE returns before copy_file_range is tried.
BUN_CONFIG_DISABLE_ioctl_ficlonerange: "1",
},
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);

const source = fs.readFileSync(join(src, fileInDir));
const stderrLines = stderr.split("\n").filter(l => l.length > 0 && !l.startsWith("WARNING: ASAN interferes"));
expect(stderrLines).toEqual([
`shim: copy_file_range = ${Math.min(source.length, 4096)}`,
"shim: copy_file_range = EXDEV",
]);

const copy = fs.readFileSync(join(dest, fileInDir));
expect({ size: copy.length, equalsSource: copy.equals(source) }).toEqual({
size: source.length,
equalsSource: true,
});
expect(stdout).toBe("");
expect(exitCode).toBe(0);
});
});
Loading