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
94 changes: 49 additions & 45 deletions src/install/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -562,13 +562,6 @@ impl RunCommand {
}
};

#[cfg(bun_debug)]
{
// Debug-only cleanup; failures are ignored. The EEXIST branch
// below already handles a stale dir.
let _ = bun_sys::delete_tree_absolute(Self::BUN_NODE_DIR.as_bytes());
}

const NODE_LINK: &ZStr = {
const B: &[u8] = concatcp!(RunCommand::BUN_NODE_DIR, "/node\0").as_bytes();
// SAFETY: literal ends in NUL; len excludes it.
Expand Down Expand Up @@ -615,6 +608,7 @@ impl RunCommand {
// would make every `--bun` child of the SECOND
// binary silently exec the FIRST. Verify the target
// before reusing; replace it once if stale.
// Never remove a matching link: concurrent runs' scripts exec it.
let mut buf = bun_paths::PathBuffer::uninit();
let matches = bun_sys::readlink(dest, &mut buf)
.map(|n| &buf[..n] == argv0_z.as_bytes())
Expand Down Expand Up @@ -686,56 +680,66 @@ impl RunCommand {
target_path_buffer[prefix.len() + len..][..dir_name.len()].copy_from_slice(dir_name);
let dir_slice_len = prefix.len() + len + dir_name.len();

#[cfg(bun_debug)]
{
// Debug builds wipe and recreate the bun-node temp dir so the
// ALREADY_EXISTS short-circuit below never reuses a stale
// hardlink at a previous debug binary.
//
// The wipe does not always leave the path absent:
// `bun-run.test.ts` uses
// `describe.concurrent`, so multiple debug processes race on
// this shared dir and `make_dir` can legitimately observe
// `PathAlreadyExists` after a sibling re-created it. Swallow
// the error — the `CreateHardLinkW` retry below already
// re-mkdirs on failure, so a lost race here is harmless.
let dir_slice_u8 = bun_core::strings::to_utf8_alloc_with_type(
&target_path_buffer[..dir_slice_len],
);
let _ = bun_sys::delete_tree_absolute(&dir_slice_u8);
let _ = bun_sys::Dir::cwd().make_dir(&dir_slice_u8);
}

let image_path = win::exe_path_w();
// Hard links have no target to read back; compare file identity instead.
let image_stat = bun_sys::File::openat_os_path(
bun_sys::Fd::cwd(),
image_path,
bun_sys::O::RDONLY,
0,
)
.and_then(|image| image.stat())
.ok();
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved

for name in [strings::w!("\\node.exe\0"), strings::w!("\\bun.exe\0")] {
target_path_buffer[dir_slice_len..][..name.len()].copy_from_slice(name);
// `target_path_buffer` is mutated in place between FFI calls
// (the dir-NUL/backslash toggle below).
// Under Stacked Borrows a `*const` derived via `Deref::deref`
// is invalidated by the intervening `&mut` from `IndexMut`, so
// re-derive `as_ptr()` at each FFI call site instead of caching.
if win::CreateHardLinkW(target_path_buffer.as_ptr(), image_path.as_ptr(), None) == 0
{
// `name` includes its NUL terminator; the link path ends before it.
let link_len = dir_slice_len + name.len() - 1;
let mut made_dir = false;
let mut replaced = false;
loop {
// `as_ptr()` stays inline: the buffer is mutated below (Stacked Borrows).
if win::CreateHardLinkW(target_path_buffer.as_ptr(), image_path.as_ptr(), None)
!= 0
{
break;
}
match win::Win32Error::get() {
win::Win32Error::ALREADY_EXISTS => {}
_ => {
win::Win32Error::ALREADY_EXISTS => {
// As in the POSIX EEXIST branch; a rebuilt binary is a different file.
let keep = image_stat.as_ref().is_none_or(|image| {
bun_sys::openat_windows(
bun_sys::Fd::cwd(),
&target_path_buffer[..link_len],
bun_sys::O::RDONLY | bun_sys::O::NOFOLLOW,
0,
)
.map(bun_sys::File::from_fd)
.and_then(|link| link.stat())
.is_ok_and(|link| {
link.st_dev == image.st_dev && link.st_ino == image.st_ino
})
});
if keep || replaced {
break;
}
let _ = win::DeleteFileBun(
&target_path_buffer[..link_len],
win::DeleteFileOptions::default(),
);
replaced = true;
}
_ if !made_dir => {
target_path_buffer[dir_slice_len] = 0;
// SAFETY: `dir_slice_len` is in-bounds; the byte at
// `dir_slice_len` was just set to NUL.
let dir_w =
bun_core::WStr::from_buf(&target_path_buffer[..], dir_slice_len);
let _ = bun_sys::mkdir_w(dir_w);
target_path_buffer[dir_slice_len] = b'\\' as u16;

if win::CreateHardLinkW(
target_path_buffer.as_ptr(),
image_path.as_ptr(),
None,
) == 0
{
return Ok(());
}
made_dir = true;
}
_ => return Ok(()),
}
}
}
Expand Down
14 changes: 2 additions & 12 deletions test/cli/install/bun-install-lifecycle-scripts.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3635,20 +3635,15 @@ for (const forceWaiterThread of isLinux ? [false, true] : [false]) {
}),
);

const originalPath = env.PATH;
env.PATH = "";

let { stderr, exited } = spawn({
cmd: [bunExe(), "install"],
cwd: packageDir,
stdout: "pipe",
stdin: "ignore",
stderr: "pipe",
env: testEnv,
env: { ...testEnv, PATH: "" },
});

env.PATH = originalPath;

let err = await stderr.text();
expect(err).toContain("No packages! Deleted empty lockfile");
expect(err).not.toContain("not found");
Expand Down Expand Up @@ -3676,20 +3671,15 @@ for (const forceWaiterThread of isLinux ? [false, true] : [false]) {
}),
);

const originalPath = env.PATH;
env.PATH = "";

let { stderr, exited } = spawn({
cmd: [bunExe(), "install"],
cwd: packageDir,
stdout: "pipe",
stderr: "pipe",
stdin: "ignore",
env,
env: { ...testEnv, PATH: "" },
});

env.PATH = originalPath;

let err = await stderr.text();
expect(err).toContain("No packages! Deleted empty lockfile");
expect(err).not.toContain("not found");
Expand Down
77 changes: 76 additions & 1 deletion test/cli/install/bun-run.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { $ } from "bun";
import { describe, expect, it } from "bun:test";
import { chmodSync } from "fs";
import { bunEnv as bunEnv_, bunExe, isWindows, tempDir, tempDirWithFiles } from "harness";
import { bunEnv as bunEnv_, bunExe, forEachLine, isWindows, tempDir, tempDirWithFiles } from "harness";
import { join } from "path";

const bunEnv = {
Expand Down Expand Up @@ -1119,4 +1119,79 @@ describe.concurrent("bun run", () => {
exitCode: 0,
});
});

// The node/bun shims live in one directory shared by every bun process of
// the same build (and, for debug builds, by all debug builds). A bun that is
// starting up may add the links but must never remove ones that already
// point at it: a script spawned by an earlier process can exec them at any
// moment and would fail with `node: not found` or fall through to a real
// node. (Debug builds used to wipe the whole directory on every start.)
it("concurrent --bun runs do not remove the node shim other scripts are using", async () => {
using dir = tempDir("bun-run-shim-race", {
"package.json": JSON.stringify({
name: "shim-race",
scripts: {
watch: "node watch.js",
probe: "node probe.js",
},
}),
// Runs as `node` through the shim, reports what `node` resolves to, then
// keeps re-resolving it (what a script's PATH lookup does) until the test
// writes `stop`. A busy loop on purpose: a process that removes and
// recreates the shim leaves it missing for only a few syscalls, so sample
// as fast as possible. Deliberately import-free: in debug builds, loading
// node:fs alone takes most of a second. (`await using` below kills this
// process if the test fails before writing `stop`.)
"watch.js": `
const shim = Bun.which("node");
console.log(shim);
while (Bun.file("stop").size === 0) {
if (Bun.which("node") !== shim) {
console.log("missing");
process.exit(1);
}
}
console.log("ok");
`,
"probe.js": `process.exit(typeof Bun === "object" ? 0 : 3);`,
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
});
const dirStr = String(dir);

await using watcher = Bun.spawn({
cmd: [bunExe(), "run", "--bun", "watch"],
cwd: dirStr,
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
const watcherLines = forEachLine(watcher.stdout);
const { value: shim } = await watcherLines.next();
expect(shim).toMatch(/[\\/]bun-node[^\\/]*[\\/]node(\.exe)?$/);

// Every sibling sets up the shim directory on startup and then runs
// `node` through it, all while the watcher's script is still using it.
const siblings = await Promise.all(
Array.from({ length: 4 }, async () => {
await using proc = Bun.spawn({
cmd: [bunExe(), "run", "--bun", "probe"],
cwd: dirStr,
env: bunEnv,
stdout: "ignore",
stderr: "pipe",
});
const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]);
return { stderr, exitCode };
}),
);

await Bun.write(join(dirStr, "stop"), "stop");
const [stdout, stderr, exitCode] = await Promise.all([
Array.fromAsync(watcherLines),
watcher.stderr.text(),
watcher.exited,
]);

expect(siblings.filter(({ exitCode }) => exitCode !== 0)).toEqual([]);
expect({ stdout, stderr, exitCode }).toEqual({ stdout: ["ok"], stderr: "$ node watch.js\n", exitCode: 0 });
});
});
10 changes: 10 additions & 0 deletions test/harness.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,16 @@ for (let key in bunEnv) {
delete bunEnv[key];
delete process.env[key];
}

// `bun run <script>` exports these to the script, so a test run started through a
// package.json script (`bun bd test`) inherits them while CI, which starts `bun test`
// directly, does not. Spawned buns honor them: NODE/npm_node_execpath decide whether
// the node shim gets created, and most npm_* values (npm_config_user_agent,
// npm_package_name, ...) are only set when absent.
if (key === "NODE" || key.startsWith("npm_")) {
delete ciEnv[key];
delete bunEnv[key];
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
}

delete bunEnv.BUN_INSPECT_CONNECT_TO;
Expand Down
Loading