From a6b5e63daf209af5116ea534711a4b32d31dbc3d Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Fri, 10 Jul 2026 02:56:27 +0000 Subject: [PATCH 1/5] install: never delete a concurrently published cache entry on Windows When two bun install processes share the package cache on Windows and both extract the same package, the loser's rename into the final cache path fails because the winner already published it. The loser then renamed the winner's entry out of the cache and deleted it, leaving a window with no cache entry, so the winner's own install failed with ENOENT. Cache entries are keyed by package identity, so an existing destination is an equivalent entry: accept it, delete only our own temp dir, and succeed. Keep the bounded backoff retry only for a destination that is not yet openable. This mirrors the POSIX arm, which already has these semantics via an atomic exchange. Fixes #28062 --- src/install/extract_tarball.rs | 71 +++++++++++----------------------- 1 file changed, 22 insertions(+), 49 deletions(-) diff --git a/src/install/extract_tarball.rs b/src/install/extract_tarball.rs index b8d3a8b091b5..04efc51ad438 100644 --- a/src/install/extract_tarball.rs +++ b/src/install/extract_tarball.rs @@ -573,59 +573,32 @@ impl ExtractTarball { true, ) { bun_sys::Result::Err(err) => { - if retries < MAX_RETRIES { - match err.get_errno() { - sys::Errno::NOTEMPTY + let _ = sys::close(dir_to_move); + if matches!( + err.get_errno(), + sys::Errno::NOTEMPTY | sys::Errno::PERM | sys::Errno::BUSY - | sys::Errno::EXIST => { - // before we attempt to delete the destination, let's close the source dir. - let _ = sys::close(dir_to_move); - - // We tried to move the folder over - // but it didn't work! - // so instead of just simply deleting the folder - // we rename it back into the temp dir - // and then delete that temp dir - // The goal is to make it more difficult for an application to reach this folder - let mut tempdest_buf = PathBuffer::uninit(); - tempdest_buf[0..tmpname.len()] - .copy_from_slice(tmpname.as_bytes()); - tempdest_buf[tmpname.len()..][0..4] - .copy_from_slice(&[b't', b'm', b'p', 0]); - let tempdest = - ZStr::from_buf(&tempdest_buf, tmpname.len() + 3); - let mut folder_name_z_buf = PathBuffer::uninit(); - folder_name_z_buf[0..folder_name.len()] - .copy_from_slice(folder_name); - folder_name_z_buf[folder_name.len()] = 0; - let folder_name_z = - ZStr::from_buf(&folder_name_z_buf, folder_name.len()); - match sys::renameat( - Fd::from_std_dir(cache_dir), - folder_name_z, - Fd::from_std_dir(tmpdir), - tempdest, - ) { - bun_sys::Result::Err(_) => {} - bun_sys::Result::Ok(_) => { - let _ = tmpdir.delete_tree(tempdest.as_bytes()); - } - } - retries += 1; - // 10ms, 20ms, 40ms, 80ms — long enough - // for a concurrent close to land, - // short enough to not slow a legit - // failure noticeably. - std::thread::sleep(std::time::Duration::from_millis( - 10u64 << (retries - 1), - )); - continue; - } - _ => {} + | sys::Errno::EXIST + ) { + // The cache path is keyed by the package identity, so an existing + // destination is an equivalent entry that a concurrent `bun install` + // published first. Never delete it — accept it and drop our copy. + if cache_dir.open_at(folder_name).is_ok() { + let _ = tmpdir.delete_tree(tmpname.as_bytes()); + break; + } + if retries < MAX_RETRIES { + retries += 1; + // 10ms, 20ms, 40ms, 80ms — long enough for a + // concurrent close to land, short enough to not + // slow a legitimate failure noticeably. + std::thread::sleep(std::time::Duration::from_millis( + 10u64 << (retries - 1), + )); + continue; } } - let _ = sys::close(dir_to_move); log.add_error_fmt( None, bun_ast::Loc::EMPTY, From 3b5cf848c831663cdb158fcf64ba89bd0b0033f2 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 2 Aug 2026 03:52:20 +0000 Subject: [PATCH 2/5] install(windows): keep stale-cache eviction, add concurrent publish regression test The accept-existing path must only accept a destination that actually has a package.json. package_missing_from_cache() triggers re-extraction precisely because the cache dir exists without one, and NtSetInformationFile cannot replace a non-empty directory, so the fresh extraction was being discarded and the stale entry kept. That broke the existing 'it should invalid cached package if package.json is missing' test on Windows. Now on collision: if the destination has package.json, accept it (a concurrent install published it); otherwise move the stale entry aside into the temp dir and retry. That matches what the POSIX arm gets from RENAME_EXCHANGE. The new test/regression/issue/28062.test.ts runs four parallel installs over a shared cache for eight rounds on Windows. Without the fix it fails on the first round with ENOENT opening cache/package/version; with the fix all 32 installs succeed. --- src/install/extract_tarball.rs | 57 ++++++++++++++--- test/regression/issue/28062.test.ts | 94 +++++++++++++++++++++++++++++ 2 files changed, 143 insertions(+), 8 deletions(-) create mode 100644 test/regression/issue/28062.test.ts diff --git a/src/install/extract_tarball.rs b/src/install/extract_tarball.rs index 04efc51ad438..83bbf5b170b3 100644 --- a/src/install/extract_tarball.rs +++ b/src/install/extract_tarball.rs @@ -581,14 +581,55 @@ impl ExtractTarball { | sys::Errno::BUSY | sys::Errno::EXIST ) { - // The cache path is keyed by the package identity, so an existing - // destination is an equivalent entry that a concurrent `bun install` - // published first. Never delete it — accept it and drop our copy. - if cache_dir.open_at(folder_name).is_ok() { - let _ = tmpdir.delete_tree(tmpname.as_bytes()); - break; - } - if retries < MAX_RETRIES { + // The cache path is keyed by the package identity, so an + // existing destination with a package.json is an equivalent + // entry that a concurrent `bun install` published first. Accept + // it and drop our copy instead of deleting theirs, which would + // leave a window where neither exists. + // + // A destination without a package.json is a stale entry that + // `package_missing_from_cache()` already rejected. Nothing is + // reading from it as a valid cache entry, so move it aside and + // retry. This mirrors the POSIX arm's RENAME_EXCHANGE, which + // atomically puts the fresh extraction at the cache path. + if let Ok(dest) = cache_dir.open_at(folder_name) { + if sys::exists_at( + dest.fd(), + ZStr::from_static(b"package.json\0"), + ) { + drop(dest); + let _ = tmpdir.delete_tree(tmpname.as_bytes()); + break; + } + drop(dest); + if retries < MAX_RETRIES { + let mut tempdest_buf = PathBuffer::uninit(); + tempdest_buf[0..tmpname.len()] + .copy_from_slice(tmpname.as_bytes()); + tempdest_buf[tmpname.len()..][0..4] + .copy_from_slice(&[b't', b'm', b'p', 0]); + let tempdest = + ZStr::from_buf(&tempdest_buf, tmpname.len() + 3); + let mut folder_name_z_buf = PathBuffer::uninit(); + folder_name_z_buf[0..folder_name.len()] + .copy_from_slice(folder_name); + folder_name_z_buf[folder_name.len()] = 0; + let folder_name_z = + ZStr::from_buf(&folder_name_z_buf, folder_name.len()); + if sys::renameat( + Fd::from_std_dir(cache_dir), + folder_name_z, + Fd::from_std_dir(tmpdir), + tempdest, + ) + .is_ok() + { + let _ = tmpdir.delete_tree(tempdest.as_bytes()); + } + retries += 1; + continue; + } + } else if retries < MAX_RETRIES { retries += 1; // 10ms, 20ms, 40ms, 80ms — long enough for a // concurrent close to land, short enough to not diff --git a/test/regression/issue/28062.test.ts b/test/regression/issue/28062.test.ts new file mode 100644 index 000000000000..94264222e92c --- /dev/null +++ b/test/regression/issue/28062.test.ts @@ -0,0 +1,94 @@ +// https://github.com/oven-sh/bun/issues/28062 +// Windows: two `bun install` processes sharing BUN_INSTALL_CACHE_DIR race to +// publish the same cache entry. The loser must accept the winner's entry and +// never delete it; otherwise the winner's install fails with ENOENT opening +// the cache dir it just published. +import { afterAll, beforeAll, expect, test } from "bun:test"; +import { rm, mkdir, writeFile } from "fs/promises"; +import { bunEnv, bunExe, isWindows, tempDir, VerdaccioRegistry } from "harness"; +import { join } from "path"; + +let verdaccio: VerdaccioRegistry; + +beforeAll(async () => { + verdaccio = new VerdaccioRegistry(); + await verdaccio.start(); +}); + +afterAll(() => { + verdaccio.stop(); +}); + +// The destructive rename-out-and-delete lived in the #[cfg(windows)] publish +// path; POSIX uses an atomic RENAME_EXCHANGE and never had the ENOENT window. +test.skipIf(!isWindows)( + "concurrent installs sharing a cache dir do not delete each other's cache entries", + async () => { + const dependencies = { + "no-deps": "1.0.0", + "a-dep": "1.0.1", + "basic-1": "1.0.0", + "what-bin": "1.0.0", + "one-dep": "1.0.0", + "two-range-deps": "1.0.0", + "dep-with-tags": "1.0.0", + "dep-loop-entry": "1.0.0", + }; + const pkg = JSON.stringify({ name: "cache-race", private: true, dependencies }); + + using root = tempDir("bun-install-cache-race", {}); + const cache = join(String(root), "shared-cache"); + const bunfig = `[install]\nregistry = "${verdaccio.registryUrl()}"\n`; + + const projects: string[] = []; + for (let i = 0; i < 4; i++) { + const dir = join(String(root), `p${i}`); + await mkdir(dir, { recursive: true }); + await writeFile(join(dir, "package.json"), pkg); + await writeFile(join(dir, "bunfig.toml"), bunfig); + projects.push(dir); + } + + const env = { + ...bunEnv, + BUN_INSTALL_CACHE_DIR: cache, + }; + + const install = async (cwd: string) => { + await using proc = Bun.spawn({ + cmd: [bunExe(), "install", "--ignore-scripts"], + cwd, + env, + stdout: "pipe", + stderr: "pipe", + stdin: "ignore", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + return { cwd, stdout, stderr, exitCode }; + }; + + // Before the fix, the loser's rename-out + delete of the winner's cache + // entry created a guaranteed ENOENT window (>=10ms of backoff) every time + // two processes collided on the same package, so a handful of rounds with + // a fresh cache is enough to hit it reliably. + for (let round = 0; round < 8; round++) { + await rm(cache, { recursive: true, force: true }); + for (const dir of projects) { + await rm(join(dir, "node_modules"), { recursive: true, force: true }); + await rm(join(dir, "bun.lock"), { force: true }); + } + + const results = await Promise.all(projects.map(install)); + const failed = results.filter(r => r.exitCode !== 0); + if (failed.length) { + const detail = failed.map(r => `cwd=${r.cwd}\nstderr:\n${r.stderr}\nstdout:\n${r.stdout}`).join("\n---\n"); + expect(detail).toBe(""); + } + for (const r of results) { + expect(r.stderr).not.toContain("ENOENT"); + expect(r.exitCode).toBe(0); + } + } + }, + 120_000, +); From 7310b818cb87271f10c8457095f2b99695338302 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Sun, 2 Aug 2026 03:54:49 +0000 Subject: [PATCH 3/5] [autofix.ci] apply automated fixes --- test/regression/issue/28062.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/regression/issue/28062.test.ts b/test/regression/issue/28062.test.ts index 94264222e92c..843e51b67d21 100644 --- a/test/regression/issue/28062.test.ts +++ b/test/regression/issue/28062.test.ts @@ -4,7 +4,7 @@ // never delete it; otherwise the winner's install fails with ENOENT opening // the cache dir it just published. import { afterAll, beforeAll, expect, test } from "bun:test"; -import { rm, mkdir, writeFile } from "fs/promises"; +import { mkdir, rm, writeFile } from "fs/promises"; import { bunEnv, bunExe, isWindows, tempDir, VerdaccioRegistry } from "harness"; import { join } from "path"; From 8353d5b0cb853e630d8e6c7b0e3b07a42667af03 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 2 Aug 2026 04:34:21 +0000 Subject: [PATCH 4/5] install(windows): evict stale cache entry by handle, mirror package_missing_from_cache() validity Open the destination with DELETE access and evict via move_opened_file_at on that handle instead of a path-based renameat, so the directory whose package.json was checked is the one moved aside; a concurrently published fresh entry at the same path is never touched. Only gate on package.json for ResolutionTag::Npm. package_missing_from_cache() uses directory_exists_at for Github/LocalTarball/RemoteTarball, and Github entries are allowed to have no package.json, so accept any openable destination for those tags. Keep the backoff sleep before retry when the eviction rename fails (sharing violation from AV/indexer); only skip it when the stale entry was actually moved aside. --- src/install/extract_tarball.rs | 132 ++++++++++++++++++++------------- 1 file changed, 79 insertions(+), 53 deletions(-) diff --git a/src/install/extract_tarball.rs b/src/install/extract_tarball.rs index 83bbf5b170b3..0fd75aeae14f 100644 --- a/src/install/extract_tarball.rs +++ b/src/install/extract_tarball.rs @@ -581,63 +581,89 @@ impl ExtractTarball { | sys::Errno::BUSY | sys::Errno::EXIST ) { - // The cache path is keyed by the package identity, so an - // existing destination with a package.json is an equivalent - // entry that a concurrent `bun install` published first. Accept - // it and drop our copy instead of deleting theirs, which would - // leave a window where neither exists. - // - // A destination without a package.json is a stale entry that - // `package_missing_from_cache()` already rejected. Nothing is - // reading from it as a valid cache entry, so move it aside and - // retry. This mirrors the POSIX arm's RENAME_EXCHANGE, which - // atomically puts the fresh extraction at the cache path. - if let Ok(dest) = cache_dir.open_at(folder_name) { - if sys::exists_at( - dest.fd(), - ZStr::from_static(b"package.json\0"), - ) { - drop(dest); - let _ = tmpdir.delete_tree(tmpname.as_bytes()); - break; - } - drop(dest); - if retries < MAX_RETRIES { - let mut tempdest_buf = PathBuffer::uninit(); - tempdest_buf[0..tmpname.len()] - .copy_from_slice(tmpname.as_bytes()); - tempdest_buf[tmpname.len()..][0..4] - .copy_from_slice(&[b't', b'm', b'p', 0]); - let tempdest = - ZStr::from_buf(&tempdest_buf, tmpname.len() + 3); - let mut folder_name_z_buf = PathBuffer::uninit(); - folder_name_z_buf[0..folder_name.len()] - .copy_from_slice(folder_name); - folder_name_z_buf[folder_name.len()] = 0; - let folder_name_z = - ZStr::from_buf(&folder_name_z_buf, folder_name.len()); - if sys::renameat( - Fd::from_std_dir(cache_dir), - folder_name_z, - Fd::from_std_dir(tmpdir), - tempdest, - ) - .is_ok() - { - let _ = tmpdir.delete_tree(tempdest.as_bytes()); + // The cache path is keyed by package identity, so a valid + // destination is an equivalent entry a concurrent `bun install` + // published first: accept it and drop our copy instead of + // deleting theirs, which would leave a window where neither + // exists. "Valid" here mirrors `package_missing_from_cache()`: + // Npm requires `package.json`; other tags only require the + // directory. An Npm destination without `package.json` is a + // stale entry nothing reads from; move it aside by handle (not + // by path, so a concurrently-published fresh entry is never + // touched) and retry, like the POSIX arm's RENAME_EXCHANGE. + match sys::open_dir_at_windows_a( + Fd::from_std_dir(cache_dir), + folder_name, + sys::WindowsOpenDirOptions { + can_rename_or_delete: true, + iterable: false, + ..Default::default() + }, + ) { + Ok(dest) => { + let valid = self.resolution.tag != ResolutionTag::Npm + || sys::exists_at( + dest, + ZStr::from_static(b"package.json\0"), + ); + if valid { + let _ = sys::close(dest); + let _ = tmpdir.delete_tree(tmpname.as_bytes()); + break; + } + let evicted = if retries < MAX_RETRIES { + let mut tempdest_buf = PathBuffer::uninit(); + tempdest_buf[0..tmpname.len()] + .copy_from_slice(tmpname.as_bytes()); + tempdest_buf[tmpname.len()..][0..3] + .copy_from_slice(b"tmp"); + let tempdest = + &tempdest_buf[..tmpname.len() + 3]; + let mut tempdest_w_buf = WPathBuffer::uninit(); + let tempdest_w = strings::to_wpath_normalized( + &mut tempdest_w_buf, + tempdest, + ); + let r = bun_sys::windows::move_opened_file_at( + dest, + Fd::from_std_dir(tmpdir), + tempdest_w, + true, + ); + let _ = sys::close(dest); + if r.is_ok() { + let _ = tmpdir.delete_tree(tempdest); + true + } else { + false + } + } else { + let _ = sys::close(dest); + false + }; + if retries < MAX_RETRIES { + retries += 1; + if !evicted { + std::thread::sleep( + std::time::Duration::from_millis( + 10u64 << (retries - 1), + ), + ); + } + continue; } + } + Err(_) if retries < MAX_RETRIES => { retries += 1; + // 10ms, 20ms, 40ms, 80ms — long enough for a + // concurrent close to land, short enough to not + // slow a legitimate failure noticeably. + std::thread::sleep(std::time::Duration::from_millis( + 10u64 << (retries - 1), + )); continue; } - } else if retries < MAX_RETRIES { - retries += 1; - // 10ms, 20ms, 40ms, 80ms — long enough for a - // concurrent close to land, short enough to not - // slow a legitimate failure noticeably. - std::thread::sleep(std::time::Duration::from_millis( - 10u64 << (retries - 1), - )); - continue; + Err(_) => {} } } log.add_error_fmt( From 45ed0f691af0f2e6e59a962c5900f97f0f472832 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 2 Aug 2026 05:13:41 +0000 Subject: [PATCH 5/5] test(28062): skip verdaccio fork on non-Windows, raise beforeAll timeout File-scope beforeAll runs even when every test is skipped, so non-Windows lanes were forking verdaccio for a test that never runs. Guard the hook body on isWindows and give it a 60s ceiling for cold debug VMs. --- test/regression/issue/28062.test.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/test/regression/issue/28062.test.ts b/test/regression/issue/28062.test.ts index 843e51b67d21..8a3729e6f8f6 100644 --- a/test/regression/issue/28062.test.ts +++ b/test/regression/issue/28062.test.ts @@ -8,15 +8,16 @@ import { mkdir, rm, writeFile } from "fs/promises"; import { bunEnv, bunExe, isWindows, tempDir, VerdaccioRegistry } from "harness"; import { join } from "path"; -let verdaccio: VerdaccioRegistry; +let verdaccio: VerdaccioRegistry | undefined; beforeAll(async () => { + if (!isWindows) return; verdaccio = new VerdaccioRegistry(); await verdaccio.start(); -}); +}, 60_000); afterAll(() => { - verdaccio.stop(); + verdaccio?.stop(); }); // The destructive rename-out-and-delete lived in the #[cfg(windows)] publish @@ -38,7 +39,7 @@ test.skipIf(!isWindows)( using root = tempDir("bun-install-cache-race", {}); const cache = join(String(root), "shared-cache"); - const bunfig = `[install]\nregistry = "${verdaccio.registryUrl()}"\n`; + const bunfig = `[install]\nregistry = "${verdaccio!.registryUrl()}"\n`; const projects: string[] = []; for (let i = 0; i < 4; i++) {