diff --git a/src/install/lib.rs b/src/install/lib.rs index 11258ad52a84..47429682b4c6 100644 --- a/src/install/lib.rs +++ b/src/install/lib.rs @@ -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. @@ -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()) @@ -686,39 +680,56 @@ 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(); + 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. @@ -726,16 +737,9 @@ impl RunCommand { 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(()), } } } diff --git a/test/cli/install/bun-install-lifecycle-scripts.test.ts b/test/cli/install/bun-install-lifecycle-scripts.test.ts index 73b280bdb58c..428758c0f58f 100644 --- a/test/cli/install/bun-install-lifecycle-scripts.test.ts +++ b/test/cli/install/bun-install-lifecycle-scripts.test.ts @@ -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"); @@ -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"); diff --git a/test/cli/install/bun-run.test.ts b/test/cli/install/bun-run.test.ts index f9045468b6bc..5923ad29ba94 100644 --- a/test/cli/install/bun-run.test.ts +++ b/test/cli/install/bun-run.test.ts @@ -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 = { @@ -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);`, + }); + 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 }); + }); }); diff --git a/test/harness.ts b/test/harness.ts index a6c07623ec17..b9a619a88f41 100644 --- a/test/harness.ts +++ b/test/harness.ts @@ -112,6 +112,16 @@ for (let key in bunEnv) { delete bunEnv[key]; delete process.env[key]; } + + // `bun run