From 63069cc0d7648514d8d31ad802f162a5d6a30cce Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 25 Jul 2026 06:35:38 +0000 Subject: [PATCH 1/8] install(windows): extend EPERM retry when moving extracted package into cache When antivirus / Search Indexer / MDM agents open a freshly extracted file for scanning without FILE_SHARE_DELETE, NTFS fails the rename of the containing directory with STATUS_ACCESS_DENIED. The existing retry handled this case but with only 150ms of total backoff, which is shorter than a typical scanner hold. Extend the retry to 10 attempts with linear backoff (25ms increments, ~1.4s total), matching SQLite's winIoerrRetry which is tuned for exactly this class of interference. Fixes #11250 --- src/install/extract_tarball.rs | 25 +-- ...bun-install-windows-locked-temp-fixture.ts | 103 +++++++++++ test/regression/issue/11250.test.ts | 169 ++++++++++++++++++ 3 files changed, 287 insertions(+), 10 deletions(-) create mode 100644 test/cli/install/bun-install-windows-locked-temp-fixture.ts create mode 100644 test/regression/issue/11250.test.ts diff --git a/src/install/extract_tarball.rs b/src/install/extract_tarball.rs index b8d3a8b091b5..514525053a3f 100644 --- a/src/install/extract_tarball.rs +++ b/src/install/extract_tarball.rs @@ -522,11 +522,19 @@ impl ExtractTarball { // Now that we've extracted the archive, we rename. #[cfg(windows)] { - // Windows EBUSY/SHARING_VIOLATION on `NtSetInformationFile` is - // transient when a concurrent process (another `bun install` - // sharing the cache, AV, the Search Indexer) is closing its - // handle to the destination. Back off briefly between retries. - const MAX_RETRIES: u32 = 4; + // Windows EBUSY/SHARING_VIOLATION/ACCESS_DENIED on + // `NtSetInformationFile` are transient when a concurrent process + // holds a handle into the tree being renamed: another `bun + // install` sharing the cache, antivirus, the Search Indexer, or + // an MDM agent scanning a just-extracted file. An open handle + // on any file inside the directory that lacks + // FILE_SHARE_DELETE fails the directory rename with + // STATUS_ACCESS_DENIED, which scanners routinely trigger on + // freshly written executables. Back off and retry; the retry + // budget (10 attempts, ~1.4s of total sleep) follows SQLite's + // winIoerrRetry, which is tuned for exactly this class of + // interference. + const MAX_RETRIES: u32 = 10; let mut retries: u32 = 0; let mut path2_buf = WPathBuffer::uninit(); let path2 = strings::to_wpath_normalized(&mut path2_buf, folder_name); @@ -613,12 +621,9 @@ impl ExtractTarball { } } retries += 1; - // 10ms, 20ms, 40ms, 80ms — long enough - // for a concurrent close to land, - // short enough to not slow a legit - // failure noticeably. + // 25ms, 50ms, ... 250ms (∑ 1375ms). std::thread::sleep(std::time::Duration::from_millis( - 10u64 << (retries - 1), + 25u64 * u64::from(retries), )); continue; } diff --git a/test/cli/install/bun-install-windows-locked-temp-fixture.ts b/test/cli/install/bun-install-windows-locked-temp-fixture.ts new file mode 100644 index 000000000000..b40dd99fb697 --- /dev/null +++ b/test/cli/install/bun-install-windows-locked-temp-fixture.ts @@ -0,0 +1,103 @@ +// Simulates an antivirus / search-indexer process that opens a freshly +// extracted file for scanning without FILE_SHARE_DELETE. On NTFS, an open +// handle lacking FILE_SHARE_DELETE on any file inside a directory causes a +// rename of that directory to fail with STATUS_ACCESS_DENIED. +// +// argv: +// +// Spin-polls tmpDir for a new `.*-*` extraction directory, opens the first +// regular file inside it via CreateFileW with dwShareMode = +// FILE_SHARE_READ | FILE_SHARE_WRITE (no DELETE), prints "HELD", holds the +// handle for holdMs, closes it, prints "RELEASED", exits 0. +// Prints "MISSED" and exits 0 if no extraction dir appears within 30s. + +import { dlopen, FFIType, ptr } from "bun:ffi"; +import { readdirSync } from "node:fs"; +import { join } from "node:path"; + +if (process.platform !== "win32") { + console.log("MISSED"); + process.exit(0); +} + +const [, , tmpDir, holdMsStr] = process.argv; +const holdMs = Number(holdMsStr); + +const { symbols } = dlopen("kernel32.dll", { + CreateFileW: { + args: [FFIType.ptr, FFIType.u32, FFIType.u32, FFIType.ptr, FFIType.u32, FFIType.u32, FFIType.ptr], + returns: FFIType.u64, + }, + CloseHandle: { args: [FFIType.u64], returns: FFIType.i32 }, +}); + +const GENERIC_READ = 0x80000000; +const FILE_SHARE_READ = 0x00000001; +const FILE_SHARE_WRITE = 0x00000002; +// Deliberately omitting FILE_SHARE_DELETE (0x00000004). +const OPEN_EXISTING = 3; +const FILE_ATTRIBUTE_NORMAL = 0x80; +const INVALID_HANDLE_VALUE = 0xffffffffffffffffn; + +function toWide(s: string): Uint8Array { + const buf = Buffer.alloc((s.length + 1) * 2); + for (let i = 0; i < s.length; i++) buf.writeUInt16LE(s.charCodeAt(i), i * 2); + return buf; +} + +function tryOpenNoShareDelete(path: string): bigint { + const h = symbols.CreateFileW( + ptr(toWide(path)), + GENERIC_READ, + FILE_SHARE_READ | FILE_SHARE_WRITE, + null, + OPEN_EXISTING, + FILE_ATTRIBUTE_NORMAL, + 0n, + ) as bigint; + return h; +} + +console.log("READY"); + +const deadline = Date.now() + 15_000; +let handle: bigint = INVALID_HANDLE_VALUE; +let heldPath = ""; +outer: while (Date.now() < deadline) { + let entries: string[]; + try { + entries = readdirSync(tmpDir); + } catch { + continue; + } + for (const name of entries) { + // Temp extraction dirs look like `.{hex}-{counter}.{pkgbasename}`. + if (!name.startsWith(".") || name.indexOf("-") === -1) continue; + let inner: string[]; + try { + inner = readdirSync(join(tmpDir, name)); + } catch { + continue; + } + for (const f of inner) { + const target = join(tmpDir, name, f); + const h = tryOpenNoShareDelete(target); + if (h !== INVALID_HANDLE_VALUE && h !== 0n) { + handle = h; + heldPath = target; + break outer; + } + } + } +} + +if (handle === INVALID_HANDLE_VALUE) { + console.log("MISSED"); + process.exit(0); +} + +console.log("HELD " + heldPath); +await Bun.sleep(holdMs); +symbols.CloseHandle(handle); +console.log("RELEASED"); +process.exit(0); diff --git a/test/regression/issue/11250.test.ts b/test/regression/issue/11250.test.ts new file mode 100644 index 000000000000..ba53e7bcedf3 --- /dev/null +++ b/test/regression/issue/11250.test.ts @@ -0,0 +1,169 @@ +// https://github.com/oven-sh/bun/issues/11250 +// +// On Windows, after `bun install` extracts a tarball into a temporary +// directory, it renames that directory into the cache. Antivirus / Search +// Indexer / MDM agents commonly open freshly written files for scanning +// without FILE_SHARE_DELETE, and while such a handle is open NTFS fails the +// parent directory rename with STATUS_ACCESS_DENIED (EPERM). The install then +// fails with: +// +// error: moving "" to cache dir failed +// EPERM: Operation not permitted (NtSetInformationFile()) +// +// The rename is already retried on PERM/BUSY, but the total backoff was only +// ~150ms which is shorter than a typical scanner hold. This test simulates a +// scanner that holds the handle for ~500ms and asserts the install still +// succeeds. + +import { describe, test, expect } from "bun:test"; +import { bunEnv, bunExe, isWindows, stderrForInstall, tempDir } from "harness"; +import { join } from "node:path"; +import { mkdirSync, writeFileSync, readFileSync } from "node:fs"; +import { randomBytes, createHash } from "node:crypto"; +import { spawnSync } from "node:child_process"; + +describe.skipIf(!isWindows)("bun install with a scanner holding an extracted file open", () => { + test("retries EPERM until the handle is released", async () => { + using dir = tempDir("issue-11250", {}); + const root = String(dir); + + // Build a package with a 2MB incompressible payload. The registry below + // serves half of it and then stalls until the blocker has grabbed its + // handle, so the streaming extractor has written bin.exe to the temp dir + // and is waiting for more input when the blocker runs. + const pkgSrc = join(root, "pkg-src", "package"); + mkdirSync(pkgSrc, { recursive: true }); + writeFileSync(join(pkgSrc, "bin.exe"), randomBytes(2 * 1024 * 1024)); + writeFileSync(join(pkgSrc, "package.json"), JSON.stringify({ name: "av-test-pkg", version: "1.0.0" })); + const tgz = join(root, "pkg-src", "av-test-pkg-1.0.0.tgz"); + const tarRc = spawnSync("tar", ["-czf", tgz, "-C", join(root, "pkg-src"), "package"], { stdio: "inherit" }); + expect(tarRc.status).toBe(0); + const tgzBytes = readFileSync(tgz); + const sha1 = createHash("sha1").update(tgzBytes).digest("hex"); + + const held = Promise.withResolvers(); + + await using server = Bun.serve({ + port: 0, + async fetch(req) { + const url = new URL(req.url); + if (url.pathname === "/av-test-pkg") { + return Response.json({ + name: "av-test-pkg", + "dist-tags": { latest: "1.0.0" }, + versions: { + "1.0.0": { + name: "av-test-pkg", + version: "1.0.0", + dist: { + tarball: `http://localhost:${server.port}/av-test-pkg/-/av-test-pkg-1.0.0.tgz`, + shasum: sha1, + }, + }, + }, + }); + } + if (url.pathname === "/av-test-pkg/-/av-test-pkg-1.0.0.tgz") { + const half = tgzBytes.length >> 1; + return new Response( + new ReadableStream({ + type: "direct", + async pull(ctrl) { + ctrl.write(tgzBytes.subarray(0, half)); + await ctrl.flush(); + await held.promise; + ctrl.write(tgzBytes.subarray(half)); + await ctrl.flush(); + ctrl.close(); + }, + }), + { + headers: { + "content-type": "application/octet-stream", + "content-length": String(tgzBytes.length), + }, + }, + ); + } + return new Response("not found", { status: 404 }); + }, + }); + + const packageDir = join(root, "project"); + const tmp = join(root, "tmp"); + const cache = join(root, "cache"); + mkdirSync(packageDir, { recursive: true }); + mkdirSync(tmp, { recursive: true }); + mkdirSync(cache, { recursive: true }); + writeFileSync( + join(packageDir, "package.json"), + JSON.stringify({ name: "issue-11250", version: "1.0.0", dependencies: { "av-test-pkg": "1.0.0" } }), + ); + writeFileSync( + join(packageDir, "bunfig.toml"), + `[install]\ncache = "${cache.replaceAll("\\", "/")}"\nregistry = "http://localhost:${server.port}/"\n`, + ); + + // Hold the handle for 500ms: longer than the unfixed 150ms retry + // budget, shorter than the fixed ~1.3s budget. + await using blocker = Bun.spawn({ + cmd: [bunExe(), join(import.meta.dir, "../../cli/install/bun-install-windows-locked-temp-fixture.ts"), tmp, "500"], + env: bunEnv, + stdout: "pipe", + stderr: "inherit", + stdin: "ignore", + }); + const reader = blocker.stdout.getReader(); + const decoder = new TextDecoder(); + let blockerOut = ""; + const ready = Promise.withResolvers(); + const drained = (async () => { + while (true) { + const { value, done } = await reader.read(); + if (done) break; + blockerOut += decoder.decode(value, { stream: true }); + if (blockerOut.includes("READY")) ready.resolve(); + if (blockerOut.includes("HELD") || blockerOut.includes("MISSED")) held.resolve(); + } + ready.resolve(); + held.resolve(); + })(); + await ready.promise; + + await using install = Bun.spawn({ + cmd: [bunExe(), "install"], + cwd: packageDir, + env: { + ...bunEnv, + BUN_INSTALL_CACHE_DIR: cache, + BUN_TMPDIR: tmp, + TMPDIR: tmp, + TEMP: tmp, + TMP: tmp, + BUN_INSTALL_STREAMING_MIN_SIZE: "1", + }, + stdout: "pipe", + stderr: "pipe", + stdin: "ignore", + }); + const [, stderr, exitCode] = await Promise.all([ + install.stdout.text(), + install.stderr.text().then(stderrForInstall), + install.exited, + ]); + + // The install has finished; either the blocker caught the extraction + // (printed HELD and is sleeping or has exited) or it is still polling. + // Kill it so the background reader reaches EOF, then inspect its output. + // The test requires HELD so that a pass is meaningful. + blocker.kill(); + await blocker.exited; + await drained; + + expect({ blocker: blockerOut, stderr, exitCode }).toEqual({ + blocker: expect.stringContaining("HELD"), + stderr: expect.not.stringContaining("NtSetInformationFile"), + exitCode: 0, + }); + }); +}); From 9041c77bbc0633acb78ac9245d5ea21397738e8f Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Sat, 25 Jul 2026 07:09:54 +0000 Subject: [PATCH 2/8] [autofix.ci] apply automated fixes --- test/regression/issue/11250.test.ts | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/test/regression/issue/11250.test.ts b/test/regression/issue/11250.test.ts index ba53e7bcedf3..0b6aa25115cc 100644 --- a/test/regression/issue/11250.test.ts +++ b/test/regression/issue/11250.test.ts @@ -15,12 +15,12 @@ // scanner that holds the handle for ~500ms and asserts the install still // succeeds. -import { describe, test, expect } from "bun:test"; +import { describe, expect, test } from "bun:test"; import { bunEnv, bunExe, isWindows, stderrForInstall, tempDir } from "harness"; -import { join } from "node:path"; -import { mkdirSync, writeFileSync, readFileSync } from "node:fs"; -import { randomBytes, createHash } from "node:crypto"; import { spawnSync } from "node:child_process"; +import { createHash, randomBytes } from "node:crypto"; +import { mkdirSync, readFileSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; describe.skipIf(!isWindows)("bun install with a scanner holding an extracted file open", () => { test("retries EPERM until the handle is released", async () => { @@ -107,7 +107,12 @@ describe.skipIf(!isWindows)("bun install with a scanner holding an extracted fil // Hold the handle for 500ms: longer than the unfixed 150ms retry // budget, shorter than the fixed ~1.3s budget. await using blocker = Bun.spawn({ - cmd: [bunExe(), join(import.meta.dir, "../../cli/install/bun-install-windows-locked-temp-fixture.ts"), tmp, "500"], + cmd: [ + bunExe(), + join(import.meta.dir, "../../cli/install/bun-install-windows-locked-temp-fixture.ts"), + tmp, + "500", + ], env: bunEnv, stdout: "pipe", stderr: "inherit", From 00ef1d9370e831d6ca508832efbd054b1c553646 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 25 Jul 2026 07:27:35 +0000 Subject: [PATCH 3/8] address review: shorten comment, use Bun.$ for tar, fix fixture doc --- src/install/extract_tarball.rs | 16 ++++------------ .../bun-install-windows-locked-temp-fixture.ts | 2 +- test/regression/issue/11250.test.ts | 4 +--- 3 files changed, 6 insertions(+), 16 deletions(-) diff --git a/src/install/extract_tarball.rs b/src/install/extract_tarball.rs index 514525053a3f..4dec8745a007 100644 --- a/src/install/extract_tarball.rs +++ b/src/install/extract_tarball.rs @@ -522,18 +522,10 @@ impl ExtractTarball { // Now that we've extracted the archive, we rename. #[cfg(windows)] { - // Windows EBUSY/SHARING_VIOLATION/ACCESS_DENIED on - // `NtSetInformationFile` are transient when a concurrent process - // holds a handle into the tree being renamed: another `bun - // install` sharing the cache, antivirus, the Search Indexer, or - // an MDM agent scanning a just-extracted file. An open handle - // on any file inside the directory that lacks - // FILE_SHARE_DELETE fails the directory rename with - // STATUS_ACCESS_DENIED, which scanners routinely trigger on - // freshly written executables. Back off and retry; the retry - // budget (10 attempts, ~1.4s of total sleep) follows SQLite's - // winIoerrRetry, which is tuned for exactly this class of - // interference. + // Windows returns STATUS_ACCESS_DENIED/SHARING_VIOLATION when a + // scanner (AV, Search Indexer, MDM) holds a handle without + // FILE_SHARE_DELETE on a file inside the directory. Retry with + // SQLite's winIoerrRetry schedule (~1.4s total). const MAX_RETRIES: u32 = 10; let mut retries: u32 = 0; let mut path2_buf = WPathBuffer::uninit(); diff --git a/test/cli/install/bun-install-windows-locked-temp-fixture.ts b/test/cli/install/bun-install-windows-locked-temp-fixture.ts index b40dd99fb697..6a67b68b232a 100644 --- a/test/cli/install/bun-install-windows-locked-temp-fixture.ts +++ b/test/cli/install/bun-install-windows-locked-temp-fixture.ts @@ -9,7 +9,7 @@ // regular file inside it via CreateFileW with dwShareMode = // FILE_SHARE_READ | FILE_SHARE_WRITE (no DELETE), prints "HELD", holds the // handle for holdMs, closes it, prints "RELEASED", exits 0. -// Prints "MISSED" and exits 0 if no extraction dir appears within 30s. +// Prints "MISSED" and exits 0 if no extraction dir appears within 15s. import { dlopen, FFIType, ptr } from "bun:ffi"; import { readdirSync } from "node:fs"; diff --git a/test/regression/issue/11250.test.ts b/test/regression/issue/11250.test.ts index 0b6aa25115cc..20c39a4c8f38 100644 --- a/test/regression/issue/11250.test.ts +++ b/test/regression/issue/11250.test.ts @@ -17,7 +17,6 @@ import { describe, expect, test } from "bun:test"; import { bunEnv, bunExe, isWindows, stderrForInstall, tempDir } from "harness"; -import { spawnSync } from "node:child_process"; import { createHash, randomBytes } from "node:crypto"; import { mkdirSync, readFileSync, writeFileSync } from "node:fs"; import { join } from "node:path"; @@ -36,8 +35,7 @@ describe.skipIf(!isWindows)("bun install with a scanner holding an extracted fil writeFileSync(join(pkgSrc, "bin.exe"), randomBytes(2 * 1024 * 1024)); writeFileSync(join(pkgSrc, "package.json"), JSON.stringify({ name: "av-test-pkg", version: "1.0.0" })); const tgz = join(root, "pkg-src", "av-test-pkg-1.0.0.tgz"); - const tarRc = spawnSync("tar", ["-czf", tgz, "-C", join(root, "pkg-src"), "package"], { stdio: "inherit" }); - expect(tarRc.status).toBe(0); + await Bun.$`tar -czf ${tgz} -C ${join(root, "pkg-src")} package`.quiet(); const tgzBytes = readFileSync(tgz); const sha1 = createHash("sha1").update(tgzBytes).digest("hex"); From 127c5eb5b326d36837e1fe1c2e8498567d1f0cc4 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 25 Jul 2026 08:34:14 +0000 Subject: [PATCH 4/8] switch to deadline-based retry with BUN_INSTALL_WIN32_AV_RETRY_MS, move test to cli/install --- src/bun_core/env_var.rs | 1 + src/install/extract_tarball.rs | 21 ++++++++++--------- .../bun-install-windows-locked-temp.test.ts} | 9 ++------ 3 files changed, 14 insertions(+), 17 deletions(-) rename test/{regression/issue/11250.test.ts => cli/install/bun-install-windows-locked-temp.test.ts} (96%) diff --git a/src/bun_core/env_var.rs b/src/bun_core/env_var.rs index eb14e27b6a99..c5b1073bb8d3 100644 --- a/src/bun_core/env_var.rs +++ b/src/bun_core/env_var.rs @@ -105,6 +105,7 @@ new!(pub BUN_INSTALL_STREAMING_MIN_SIZE: unsigned, "BUN_INSTALL_STREAMING_MIN_SI // thread schedules a drain; collapses the per-chunk thread-pool futex wake // into roughly one per `threshold` bytes. new!(pub BUN_INSTALL_STREAMING_DRAIN_THRESHOLD: unsigned, "BUN_INSTALL_STREAMING_DRAIN_THRESHOLD", { default: 256 * 1024 }); +new!(pub BUN_INSTALL_WIN32_AV_RETRY_MS: unsigned, "BUN_INSTALL_WIN32_AV_RETRY_MS", { default: 5_000 }); new!(pub BUN_NEEDS_PROC_SELF_WORKAROUND: boolean, "BUN_NEEDS_PROC_SELF_WORKAROUND", { default: false }); new!(pub BUN_OPTIONS: string, "BUN_OPTIONS", {}); new!(pub BUN_POSTGRES_SOCKET_MONITOR: string, "BUN_POSTGRES_SOCKET_MONITOR", {}); diff --git a/src/install/extract_tarball.rs b/src/install/extract_tarball.rs index 4dec8745a007..7b3a5dbf6dcc 100644 --- a/src/install/extract_tarball.rs +++ b/src/install/extract_tarball.rs @@ -522,12 +522,14 @@ impl ExtractTarball { // Now that we've extracted the archive, we rename. #[cfg(windows)] { - // Windows returns STATUS_ACCESS_DENIED/SHARING_VIOLATION when a - // scanner (AV, Search Indexer, MDM) holds a handle without - // FILE_SHARE_DELETE on a file inside the directory. Retry with - // SQLite's winIoerrRetry schedule (~1.4s total). - const MAX_RETRIES: u32 = 10; - let mut retries: u32 = 0; + // AV scanners holding a handle without FILE_SHARE_DELETE on an extracted file cause a transient STATUS_ACCESS_DENIED; retry with the graceful-fs backoff (10ms increments capped at 100ms) until `BUN_INSTALL_WIN32_AV_RETRY_MS` elapses. + let budget = std::time::Duration::from_millis( + bun_core::env_var::BUN_INSTALL_WIN32_AV_RETRY_MS + .get() + .unwrap(), + ); + let start = std::time::Instant::now(); + let mut backoff_ms: u64 = 0; let mut path2_buf = WPathBuffer::uninit(); let path2 = strings::to_wpath_normalized(&mut path2_buf, folder_name); if create_subdir { @@ -573,7 +575,7 @@ impl ExtractTarball { true, ) { bun_sys::Result::Err(err) => { - if retries < MAX_RETRIES { + if start.elapsed() < budget { match err.get_errno() { sys::Errno::NOTEMPTY | sys::Errno::PERM @@ -612,10 +614,9 @@ impl ExtractTarball { let _ = tmpdir.delete_tree(tempdest.as_bytes()); } } - retries += 1; - // 25ms, 50ms, ... 250ms (∑ 1375ms). + backoff_ms = (backoff_ms + 10).min(100); std::thread::sleep(std::time::Duration::from_millis( - 25u64 * u64::from(retries), + backoff_ms, )); continue; } diff --git a/test/regression/issue/11250.test.ts b/test/cli/install/bun-install-windows-locked-temp.test.ts similarity index 96% rename from test/regression/issue/11250.test.ts rename to test/cli/install/bun-install-windows-locked-temp.test.ts index 20c39a4c8f38..8d5997e409d6 100644 --- a/test/regression/issue/11250.test.ts +++ b/test/cli/install/bun-install-windows-locked-temp.test.ts @@ -103,14 +103,9 @@ describe.skipIf(!isWindows)("bun install with a scanner holding an extracted fil ); // Hold the handle for 500ms: longer than the unfixed 150ms retry - // budget, shorter than the fixed ~1.3s budget. + // budget, shorter than the fixed default 5s budget. await using blocker = Bun.spawn({ - cmd: [ - bunExe(), - join(import.meta.dir, "../../cli/install/bun-install-windows-locked-temp-fixture.ts"), - tmp, - "500", - ], + cmd: [bunExe(), join(import.meta.dir, "bun-install-windows-locked-temp-fixture.ts"), tmp, "500"], env: bunEnv, stdout: "pipe", stderr: "inherit", From d1eb73b0d58058164f2d4ccbc7971be4be259dbf Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 25 Jul 2026 12:24:59 +0000 Subject: [PATCH 5/8] ci: retrigger From 9ee07e05ef27d677346501e132e5b0a026329719 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 13 Aug 2026 01:27:11 +0000 Subject: [PATCH 6/8] install: share one Windows rename retry budget across all cache publishes Extracted tarballs, patched packages and global virtual store entries are all published into the cache with a directory rename, and on Windows all three fail with STATUS_ACCESS_DENIED while a scanner holds a file inside the directory open. Move the retry into cache_rename::RenameRetry and use it at every site; the budget is BUN_INSTALL_WINDOWS_RENAME_RETRY_MS (default 5s) and the error reported once it runs out names the variable. The global store publish used to treat EPERM as a collision with a concurrent install, deleting the held staging dir and reporting success; it now only does so when the destination actually exists. --- src/bun_core/env_var.rs | 8 +- src/install/cache_rename.rs | 101 +++++++ src/install/extract_tarball.rs | 103 ++++--- src/install/isolated_install/Installer.rs | 142 +++++---- src/install/lib.rs | 1 + src/install/patch_install.rs | 32 ++- ...bun-install-windows-locked-temp-fixture.ts | 103 ------- .../bun-install-windows-locked-temp.test.ts | 167 ----------- ...un-install-windows-rename-retry-fixture.ts | 112 ++++++++ .../bun-install-windows-rename-retry.test.ts | 272 ++++++++++++++++++ 10 files changed, 648 insertions(+), 393 deletions(-) create mode 100644 src/install/cache_rename.rs delete mode 100644 test/cli/install/bun-install-windows-locked-temp-fixture.ts delete mode 100644 test/cli/install/bun-install-windows-locked-temp.test.ts create mode 100644 test/cli/install/bun-install-windows-rename-retry-fixture.ts create mode 100644 test/cli/install/bun-install-windows-rename-retry.test.ts diff --git a/src/bun_core/env_var.rs b/src/bun_core/env_var.rs index c5b1073bb8d3..3eb6ebaeb3e8 100644 --- a/src/bun_core/env_var.rs +++ b/src/bun_core/env_var.rs @@ -105,7 +105,13 @@ new!(pub BUN_INSTALL_STREAMING_MIN_SIZE: unsigned, "BUN_INSTALL_STREAMING_MIN_SI // thread schedules a drain; collapses the per-chunk thread-pool futex wake // into roughly one per `threshold` bytes. new!(pub BUN_INSTALL_STREAMING_DRAIN_THRESHOLD: unsigned, "BUN_INSTALL_STREAMING_DRAIN_THRESHOLD", { default: 256 * 1024 }); -new!(pub BUN_INSTALL_WIN32_AV_RETRY_MS: unsigned, "BUN_INSTALL_WIN32_AV_RETRY_MS", { default: 5_000 }); +// How long (ms) `bun install` keeps retrying a rename into the cache on +// Windows while a scanner holds a file in the directory open (see +// `bun_install::cache_rename`). 5s covers the real-time scan of a +// multi-megabyte binary with margin (SQLite's equivalent retry waits 1.4s, +// graceful-fs 60s); it is also what a permanent failure such as an unwritable +// cache dir now costs per package before it is reported. 0 disables retrying. +new!(pub BUN_INSTALL_WINDOWS_RENAME_RETRY_MS: unsigned, "BUN_INSTALL_WINDOWS_RENAME_RETRY_MS", { default: 5_000 }); new!(pub BUN_NEEDS_PROC_SELF_WORKAROUND: boolean, "BUN_NEEDS_PROC_SELF_WORKAROUND", { default: false }); new!(pub BUN_OPTIONS: string, "BUN_OPTIONS", {}); new!(pub BUN_POSTGRES_SOCKET_MONITOR: string, "BUN_POSTGRES_SOCKET_MONITOR", {}); diff --git a/src/install/cache_rename.rs b/src/install/cache_rename.rs new file mode 100644 index 000000000000..92342332a558 --- /dev/null +++ b/src/install/cache_rename.rs @@ -0,0 +1,101 @@ +//! Retry budget shared by the renames that publish a freshly built directory +//! into the install cache: an extracted tarball, a patched package, or a +//! global virtual store entry. +//! +//! On Windows, renaming a directory fails with `STATUS_ACCESS_DENIED` or +//! `STATUS_SHARING_VIOLATION` while any other process holds a handle without +//! `FILE_SHARE_DELETE` on a file inside it. Antivirus, the Search Indexer and +//! endpoint agents open freshly written files exactly that way, typically for +//! tens of milliseconds up to a few seconds, so the rename is retried until +//! `BUN_INSTALL_WINDOWS_RENAME_RETRY_MS` has elapsed. POSIX renames are not +//! affected by open handles and `EPERM`/`EACCES` are real permission failures +//! there, so nothing is ever retried off Windows. + +use core::fmt; +use core::time::Duration; +use std::time::Instant; + +use bun_sys as sys; + +pub(crate) const ENV_VAR_NAME: &str = "BUN_INSTALL_WINDOWS_RENAME_RETRY_MS"; + +pub(crate) struct RenameRetry { + started: Instant, + budget: Duration, + /// Sleep before the next attempt; grows 10ms per attempt and caps at 100ms, + /// which is the schedule npm's `graceful-fs` uses for the same failure. + next_backoff: Duration, + exhausted: bool, +} + +impl RenameRetry { + pub(crate) fn start() -> Self { + Self { + started: Instant::now(), + budget: Duration::from_millis( + bun_core::env_var::BUN_INSTALL_WINDOWS_RENAME_RETRY_MS + .get() + .unwrap_or(5_000), + ), + next_backoff: Duration::ZERO, + exhausted: false, + } + } + + /// Whether `err` is one of the errors Windows reports while another process + /// holds a handle inside the directory being renamed or at its destination. + pub(crate) fn is_transient(err: &sys::Error) -> bool { + cfg!(windows) + && matches!( + err.get_errno(), + sys::Errno::EPERM | sys::Errno::EACCES | sys::Errno::EBUSY + ) + } + + /// Called after a failed attempt. Sleeps and returns `true` while the budget + /// allows another attempt; returns `false` once it is spent. + pub(crate) fn wait(&mut self) -> bool { + if self.started.elapsed() >= self.budget { + self.exhausted = true; + return false; + } + self.next_backoff = + (self.next_backoff + Duration::from_millis(10)).min(Duration::from_millis(100)); + std::thread::sleep(self.next_backoff); + true + } + + pub(crate) fn exhausted(&self) -> bool { + self.exhausted + } + + /// Suffix for the error reported to the user once `wait()` has returned + /// `false`; displays as nothing otherwise. + pub(crate) fn exhausted_hint(&self) -> ExhaustedHint { + ExhaustedHint { + waited: if self.exhausted { + Some(self.started.elapsed()) + } else { + None + }, + } + } +} + +pub(crate) struct ExhaustedHint { + waited: Option, +} + +impl fmt::Display for ExhaustedHint { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self.waited { + Some(waited) => write!( + f, + " (gave up after retrying for {}ms; another process is holding a file open in the directory. Set {} to wait longer)", + waited.as_millis(), + ENV_VAR_NAME, + ), + None => Ok(()), + } + } +} diff --git a/src/install/extract_tarball.rs b/src/install/extract_tarball.rs index 7b3a5dbf6dcc..ea645f9072f1 100644 --- a/src/install/extract_tarball.rs +++ b/src/install/extract_tarball.rs @@ -21,6 +21,9 @@ use bun_libarchive::{ArchiveAppender, ExtractOptions}; use bun_resolver::fs::FileSystem; #[cfg(windows)] use bun_sys::FdDirExt; + +#[cfg(windows)] +use crate::cache_rename::RenameRetry; type Error = crate::Error; pub struct ExtractTarball { @@ -522,14 +525,14 @@ impl ExtractTarball { // Now that we've extracted the archive, we rename. #[cfg(windows)] { - // AV scanners holding a handle without FILE_SHARE_DELETE on an extracted file cause a transient STATUS_ACCESS_DENIED; retry with the graceful-fs backoff (10ms increments capped at 100ms) until `BUN_INSTALL_WIN32_AV_RETRY_MS` elapses. - let budget = std::time::Duration::from_millis( - bun_core::env_var::BUN_INSTALL_WIN32_AV_RETRY_MS - .get() - .unwrap(), - ); - let start = std::time::Instant::now(); - let mut backoff_ms: u64 = 0; + // The rename fails transiently when another process holds a + // handle into either directory: a concurrent `bun install` + // sharing the cache still has the destination open (EXIST / + // NOTEMPTY, or PERM since NTFS reports replacing a directory + // that way too), or a scanner has one of our freshly extracted + // files open (PERM / BUSY, see `cache_rename`). Both are + // retried against the same `RenameRetry` budget. + let mut retry = RenameRetry::start(); let mut path2_buf = WPathBuffer::uninit(); let path2 = strings::to_wpath_normalized(&mut path2_buf, folder_name); if create_subdir { @@ -575,61 +578,51 @@ impl ExtractTarball { true, ) { bun_sys::Result::Err(err) => { - if start.elapsed() < budget { - match 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()); - } - } - backoff_ms = (backoff_ms + 10).min(100); - std::thread::sleep(std::time::Duration::from_millis( - backoff_ms, - )); - continue; + // before we attempt to delete the destination, let's close the source dir. + let _ = sys::close(dir_to_move); + + let retryable = RenameRetry::is_transient(&err) + || matches!( + err.get_errno(), + sys::Errno::NOTEMPTY | sys::Errno::EXIST + ); + if retryable && retry.wait() { + // 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()); } - _ => {} } + continue; } - let _ = sys::close(dir_to_move); log.add_error_fmt( None, bun_ast::Loc::EMPTY, format_args!( - "moving \"{}\" to cache dir failed\n{}\n From: {}\n To: {}", + "moving \"{}\" to cache dir failed{}\n{}\n From: {}\n To: {}", bun_fmt::s(name), + retry.exhausted_hint(), err, bun_fmt::s(tmpname.as_bytes()), bun_fmt::s(folder_name), diff --git a/src/install/isolated_install/Installer.rs b/src/install/isolated_install/Installer.rs index 1984c05fb3a7..a55ad83ec3e8 100644 --- a/src/install/isolated_install/Installer.rs +++ b/src/install/isolated_install/Installer.rs @@ -13,6 +13,7 @@ use bun_semver::String as SemverString; use bun_sys::{FdDirExt as _, FdExt as _}; use crate::bin_real; +use crate::cache_rename::RenameRetry; use crate::lockfile::package; use crate::lockfile_real::PackageIDSlice; use crate::package_install::{Method as InstallMethod, Summary as InstallSummary}; @@ -2446,61 +2447,80 @@ impl<'a> Installer<'a> { let mut final_ = AutoAbsPath::init(); self.append_global_store_entry_path(&mut final_, entry_id, Which::Final); - match sys::renameat(Fd::cwd(), staging.slice_z(), Fd::cwd(), final_.slice_z()) { - sys::Result::Ok(()) => sys::Result::Ok(()), - sys::Result::Err(err) => { - if !is_rename_collision(&err) { - let _ = Fd::cwd().delete_tree(staging.slice()); - return sys::Result::Err(err); - } - // Under --force, the existing entry may be the corrupt one - // we were asked to replace. Swap it aside (atomic from a - // reader's POV: `final` is always either the old or the new - // tree, never missing), publish staging, then GC the old - // tree. Without --force, the existing entry came from a - // concurrent install and is content-identical — keep it and - // discard ours. - if self.manager().options.enable.force_install() { - let mut old = AutoAbsPath::init(); - let _ = old.append(self.global_store_path.as_ref().unwrap().as_bytes()); // OOM/capacity: fire-and-forget - // OOM/capacity: fire-and-forget - let _ = old.append_fmt(format_args!( - "{}.old-{:x}", - store::entry::fmt_global_store_path(entry_id, self.store, self.lockfile()), - bun_core::fast_random(), - )); - if let Some(swap_err) = - sys::renameat(Fd::cwd(), final_.slice_z(), Fd::cwd(), old.slice_z()).err() - { - let _ = Fd::cwd().delete_tree(staging.slice()); - return sys::Result::Err(swap_err); - } - match sys::renameat(Fd::cwd(), staging.slice_z(), Fd::cwd(), final_.slice_z()) { - sys::Result::Ok(()) => { - let _ = Fd::cwd().delete_tree(old.slice()); - return sys::Result::Ok(()); - } - sys::Result::Err(publish_err) => { - // Another --force install raced us in the window - // between swap-out and publish. Theirs is fresh - // too; clean up both temp trees. - let _ = Fd::cwd().delete_tree(staging.slice()); - let _ = Fd::cwd().delete_tree(old.slice()); - return if is_rename_collision(&publish_err) { - sys::Result::Ok(()) - } else { - sys::Result::Err(publish_err) - }; - } + let mut retry = RenameRetry::start(); + loop { + let err = match sys::renameat(Fd::cwd(), staging.slice_z(), Fd::cwd(), final_.slice_z()) + { + sys::Result::Ok(()) => return sys::Result::Ok(()), + sys::Result::Err(err) => err, + }; + if is_rename_collision(&err, final_.slice_z()) { + break; + } + if RenameRetry::is_transient(&err) && retry.wait() { + continue; + } + let _ = Fd::cwd().delete_tree(staging.slice()); + report_exhausted_publish(&retry, &final_); + return sys::Result::Err(err); + } + + // Under --force, the existing entry may be the corrupt one + // we were asked to replace. Swap it aside (atomic from a + // reader's POV: `final` is always either the old or the new + // tree, never missing), publish staging, then GC the old + // tree. Without --force, the existing entry came from a + // concurrent install and is content-identical — keep it and + // discard ours. + if self.manager().options.enable.force_install() { + let mut old = AutoAbsPath::init(); + let _ = old.append(self.global_store_path.as_ref().unwrap().as_bytes()); // OOM/capacity: fire-and-forget + // OOM/capacity: fire-and-forget + let _ = old.append_fmt(format_args!( + "{}.old-{:x}", + store::entry::fmt_global_store_path(entry_id, self.store, self.lockfile()), + bun_core::fast_random(), + )); + if let Some(swap_err) = + sys::renameat(Fd::cwd(), final_.slice_z(), Fd::cwd(), old.slice_z()).err() + { + let _ = Fd::cwd().delete_tree(staging.slice()); + return sys::Result::Err(swap_err); + } + loop { + let publish_err = match sys::renameat( + Fd::cwd(), + staging.slice_z(), + Fd::cwd(), + final_.slice_z(), + ) { + sys::Result::Ok(()) => { + let _ = Fd::cwd().delete_tree(old.slice()); + return sys::Result::Ok(()); } + sys::Result::Err(err) => err, + }; + let raced = is_rename_collision(&publish_err, final_.slice_z()); + if !raced && RenameRetry::is_transient(&publish_err) && retry.wait() { + continue; } + // Another --force install raced us in the window + // between swap-out and publish. Theirs is fresh + // too; clean up both temp trees. let _ = Fd::cwd().delete_tree(staging.slice()); - // A concurrent install renamed first; both writers produced - // the same content-addressed bytes, so theirs is as good as - // ours. - sys::Result::Ok(()) + let _ = Fd::cwd().delete_tree(old.slice()); + if raced { + return sys::Result::Ok(()); + } + report_exhausted_publish(&retry, &final_); + return sys::Result::Err(publish_err); } } + let _ = Fd::cwd().delete_tree(staging.slice()); + // A concurrent install renamed first; both writers produced + // the same content-addressed bytes, so theirs is as good as + // ours. + sys::Result::Ok(()) } /// Project-local path `node_modules/.bun/` (the symlink that @@ -2795,13 +2815,25 @@ pub enum Which { Staging, } -fn is_rename_collision(err: &sys::Error) -> bool { +fn is_rename_collision(err: &sys::Error, final_: &ZStr) -> bool { match err.get_errno() { sys::Errno::EEXIST | sys::Errno::ENOTEMPTY => true, - // Windows maps a rename onto an in-use directory to - // ERROR_ACCESS_DENIED; on POSIX PERM/ACCES are real - // permission failures and must propagate. - sys::Errno::EPERM | sys::Errno::EACCES => cfg!(windows), + // Windows maps a rename onto an existing directory to + // ERROR_ACCESS_DENIED, but reports a scanner holding one of our + // staged files open the same way (see `cache_rename`); only the + // destination existing makes it a collision. On POSIX PERM/ACCES are + // real permission failures and must propagate. + sys::Errno::EPERM | sys::Errno::EACCES => cfg!(windows) && sys::exists_z(final_), _ => false, } } + +fn report_exhausted_publish(retry: &RenameRetry, final_: &AutoAbsPath) { + if retry.exhausted() { + bun_core::pretty_errorln!( + "error: publishing {} to the global store failed{}", + bstr::BStr::new(final_.slice()), + retry.exhausted_hint(), + ); + } +} diff --git a/src/install/lib.rs b/src/install/lib.rs index 11258ad52a84..a8fa9a0aa220 100644 --- a/src/install/lib.rs +++ b/src/install/lib.rs @@ -74,6 +74,7 @@ pub mod resolution; // Legacy alias kept while callers migrate from the stub/real split. pub use resolution as resolution_real; pub mod auto_installer; +pub(crate) mod cache_rename; #[path = "ConfigVersion.rs"] pub mod config_version; pub mod dependency; diff --git a/src/install/patch_install.rs b/src/install/patch_install.rs index 2b5addb0149b..afa3553014ed 100644 --- a/src/install/patch_install.rs +++ b/src/install/patch_install.rs @@ -14,6 +14,7 @@ use bun_threading::IntrusiveWorkTask as _; use bun_threading::thread_pool::{Batch, Node as ThreadPoolNode, Task as ThreadPoolTask}; use bun_wyhash::Wyhash11; +use crate::cache_rename::RenameRetry; use crate::package_install::PackageInstall; use crate::package_manager; use crate::{ @@ -578,26 +579,33 @@ impl PatchTask { ); let cache_dir_subpath_z: &ZStr = patch.cache_dir_subpath.as_zstr(); - if let Err(e) = sys::renameat_concurrently( - system_tmpdir, - path_in_tmpdir, - patch.cache_dir, - cache_dir_subpath_z, - sys::RenameOptions { - move_fallback: true, - ..Default::default() - }, - ) { + let mut retry = RenameRetry::start(); + loop { + let Err(e) = sys::renameat_concurrently( + system_tmpdir, + path_in_tmpdir, + patch.cache_dir, + cache_dir_subpath_z, + sys::RenameOptions { + move_fallback: true, + ..Default::default() + }, + ) else { + return Ok(()); + }; + if RenameRetry::is_transient(&e) && retry.wait() { + continue; + } log.add_error_fmt_opts( format_args!( - "renaming changes to cache dir: {}", + "renaming changes to cache dir{}: {}", + retry.exhausted_hint(), e.with_path(cache_dir_subpath_z.as_bytes()) ), Default::default(), ); return Ok(()); } - Ok(()) } pub(crate) fn calc_hash(&mut self) -> Option { diff --git a/test/cli/install/bun-install-windows-locked-temp-fixture.ts b/test/cli/install/bun-install-windows-locked-temp-fixture.ts deleted file mode 100644 index 6a67b68b232a..000000000000 --- a/test/cli/install/bun-install-windows-locked-temp-fixture.ts +++ /dev/null @@ -1,103 +0,0 @@ -// Simulates an antivirus / search-indexer process that opens a freshly -// extracted file for scanning without FILE_SHARE_DELETE. On NTFS, an open -// handle lacking FILE_SHARE_DELETE on any file inside a directory causes a -// rename of that directory to fail with STATUS_ACCESS_DENIED. -// -// argv: -// -// Spin-polls tmpDir for a new `.*-*` extraction directory, opens the first -// regular file inside it via CreateFileW with dwShareMode = -// FILE_SHARE_READ | FILE_SHARE_WRITE (no DELETE), prints "HELD", holds the -// handle for holdMs, closes it, prints "RELEASED", exits 0. -// Prints "MISSED" and exits 0 if no extraction dir appears within 15s. - -import { dlopen, FFIType, ptr } from "bun:ffi"; -import { readdirSync } from "node:fs"; -import { join } from "node:path"; - -if (process.platform !== "win32") { - console.log("MISSED"); - process.exit(0); -} - -const [, , tmpDir, holdMsStr] = process.argv; -const holdMs = Number(holdMsStr); - -const { symbols } = dlopen("kernel32.dll", { - CreateFileW: { - args: [FFIType.ptr, FFIType.u32, FFIType.u32, FFIType.ptr, FFIType.u32, FFIType.u32, FFIType.ptr], - returns: FFIType.u64, - }, - CloseHandle: { args: [FFIType.u64], returns: FFIType.i32 }, -}); - -const GENERIC_READ = 0x80000000; -const FILE_SHARE_READ = 0x00000001; -const FILE_SHARE_WRITE = 0x00000002; -// Deliberately omitting FILE_SHARE_DELETE (0x00000004). -const OPEN_EXISTING = 3; -const FILE_ATTRIBUTE_NORMAL = 0x80; -const INVALID_HANDLE_VALUE = 0xffffffffffffffffn; - -function toWide(s: string): Uint8Array { - const buf = Buffer.alloc((s.length + 1) * 2); - for (let i = 0; i < s.length; i++) buf.writeUInt16LE(s.charCodeAt(i), i * 2); - return buf; -} - -function tryOpenNoShareDelete(path: string): bigint { - const h = symbols.CreateFileW( - ptr(toWide(path)), - GENERIC_READ, - FILE_SHARE_READ | FILE_SHARE_WRITE, - null, - OPEN_EXISTING, - FILE_ATTRIBUTE_NORMAL, - 0n, - ) as bigint; - return h; -} - -console.log("READY"); - -const deadline = Date.now() + 15_000; -let handle: bigint = INVALID_HANDLE_VALUE; -let heldPath = ""; -outer: while (Date.now() < deadline) { - let entries: string[]; - try { - entries = readdirSync(tmpDir); - } catch { - continue; - } - for (const name of entries) { - // Temp extraction dirs look like `.{hex}-{counter}.{pkgbasename}`. - if (!name.startsWith(".") || name.indexOf("-") === -1) continue; - let inner: string[]; - try { - inner = readdirSync(join(tmpDir, name)); - } catch { - continue; - } - for (const f of inner) { - const target = join(tmpDir, name, f); - const h = tryOpenNoShareDelete(target); - if (h !== INVALID_HANDLE_VALUE && h !== 0n) { - handle = h; - heldPath = target; - break outer; - } - } - } -} - -if (handle === INVALID_HANDLE_VALUE) { - console.log("MISSED"); - process.exit(0); -} - -console.log("HELD " + heldPath); -await Bun.sleep(holdMs); -symbols.CloseHandle(handle); -console.log("RELEASED"); -process.exit(0); diff --git a/test/cli/install/bun-install-windows-locked-temp.test.ts b/test/cli/install/bun-install-windows-locked-temp.test.ts deleted file mode 100644 index 8d5997e409d6..000000000000 --- a/test/cli/install/bun-install-windows-locked-temp.test.ts +++ /dev/null @@ -1,167 +0,0 @@ -// https://github.com/oven-sh/bun/issues/11250 -// -// On Windows, after `bun install` extracts a tarball into a temporary -// directory, it renames that directory into the cache. Antivirus / Search -// Indexer / MDM agents commonly open freshly written files for scanning -// without FILE_SHARE_DELETE, and while such a handle is open NTFS fails the -// parent directory rename with STATUS_ACCESS_DENIED (EPERM). The install then -// fails with: -// -// error: moving "" to cache dir failed -// EPERM: Operation not permitted (NtSetInformationFile()) -// -// The rename is already retried on PERM/BUSY, but the total backoff was only -// ~150ms which is shorter than a typical scanner hold. This test simulates a -// scanner that holds the handle for ~500ms and asserts the install still -// succeeds. - -import { describe, expect, test } from "bun:test"; -import { bunEnv, bunExe, isWindows, stderrForInstall, tempDir } from "harness"; -import { createHash, randomBytes } from "node:crypto"; -import { mkdirSync, readFileSync, writeFileSync } from "node:fs"; -import { join } from "node:path"; - -describe.skipIf(!isWindows)("bun install with a scanner holding an extracted file open", () => { - test("retries EPERM until the handle is released", async () => { - using dir = tempDir("issue-11250", {}); - const root = String(dir); - - // Build a package with a 2MB incompressible payload. The registry below - // serves half of it and then stalls until the blocker has grabbed its - // handle, so the streaming extractor has written bin.exe to the temp dir - // and is waiting for more input when the blocker runs. - const pkgSrc = join(root, "pkg-src", "package"); - mkdirSync(pkgSrc, { recursive: true }); - writeFileSync(join(pkgSrc, "bin.exe"), randomBytes(2 * 1024 * 1024)); - writeFileSync(join(pkgSrc, "package.json"), JSON.stringify({ name: "av-test-pkg", version: "1.0.0" })); - const tgz = join(root, "pkg-src", "av-test-pkg-1.0.0.tgz"); - await Bun.$`tar -czf ${tgz} -C ${join(root, "pkg-src")} package`.quiet(); - const tgzBytes = readFileSync(tgz); - const sha1 = createHash("sha1").update(tgzBytes).digest("hex"); - - const held = Promise.withResolvers(); - - await using server = Bun.serve({ - port: 0, - async fetch(req) { - const url = new URL(req.url); - if (url.pathname === "/av-test-pkg") { - return Response.json({ - name: "av-test-pkg", - "dist-tags": { latest: "1.0.0" }, - versions: { - "1.0.0": { - name: "av-test-pkg", - version: "1.0.0", - dist: { - tarball: `http://localhost:${server.port}/av-test-pkg/-/av-test-pkg-1.0.0.tgz`, - shasum: sha1, - }, - }, - }, - }); - } - if (url.pathname === "/av-test-pkg/-/av-test-pkg-1.0.0.tgz") { - const half = tgzBytes.length >> 1; - return new Response( - new ReadableStream({ - type: "direct", - async pull(ctrl) { - ctrl.write(tgzBytes.subarray(0, half)); - await ctrl.flush(); - await held.promise; - ctrl.write(tgzBytes.subarray(half)); - await ctrl.flush(); - ctrl.close(); - }, - }), - { - headers: { - "content-type": "application/octet-stream", - "content-length": String(tgzBytes.length), - }, - }, - ); - } - return new Response("not found", { status: 404 }); - }, - }); - - const packageDir = join(root, "project"); - const tmp = join(root, "tmp"); - const cache = join(root, "cache"); - mkdirSync(packageDir, { recursive: true }); - mkdirSync(tmp, { recursive: true }); - mkdirSync(cache, { recursive: true }); - writeFileSync( - join(packageDir, "package.json"), - JSON.stringify({ name: "issue-11250", version: "1.0.0", dependencies: { "av-test-pkg": "1.0.0" } }), - ); - writeFileSync( - join(packageDir, "bunfig.toml"), - `[install]\ncache = "${cache.replaceAll("\\", "/")}"\nregistry = "http://localhost:${server.port}/"\n`, - ); - - // Hold the handle for 500ms: longer than the unfixed 150ms retry - // budget, shorter than the fixed default 5s budget. - await using blocker = Bun.spawn({ - cmd: [bunExe(), join(import.meta.dir, "bun-install-windows-locked-temp-fixture.ts"), tmp, "500"], - env: bunEnv, - stdout: "pipe", - stderr: "inherit", - stdin: "ignore", - }); - const reader = blocker.stdout.getReader(); - const decoder = new TextDecoder(); - let blockerOut = ""; - const ready = Promise.withResolvers(); - const drained = (async () => { - while (true) { - const { value, done } = await reader.read(); - if (done) break; - blockerOut += decoder.decode(value, { stream: true }); - if (blockerOut.includes("READY")) ready.resolve(); - if (blockerOut.includes("HELD") || blockerOut.includes("MISSED")) held.resolve(); - } - ready.resolve(); - held.resolve(); - })(); - await ready.promise; - - await using install = Bun.spawn({ - cmd: [bunExe(), "install"], - cwd: packageDir, - env: { - ...bunEnv, - BUN_INSTALL_CACHE_DIR: cache, - BUN_TMPDIR: tmp, - TMPDIR: tmp, - TEMP: tmp, - TMP: tmp, - BUN_INSTALL_STREAMING_MIN_SIZE: "1", - }, - stdout: "pipe", - stderr: "pipe", - stdin: "ignore", - }); - const [, stderr, exitCode] = await Promise.all([ - install.stdout.text(), - install.stderr.text().then(stderrForInstall), - install.exited, - ]); - - // The install has finished; either the blocker caught the extraction - // (printed HELD and is sleeping or has exited) or it is still polling. - // Kill it so the background reader reaches EOF, then inspect its output. - // The test requires HELD so that a pass is meaningful. - blocker.kill(); - await blocker.exited; - await drained; - - expect({ blocker: blockerOut, stderr, exitCode }).toEqual({ - blocker: expect.stringContaining("HELD"), - stderr: expect.not.stringContaining("NtSetInformationFile"), - exitCode: 0, - }); - }); -}); diff --git a/test/cli/install/bun-install-windows-rename-retry-fixture.ts b/test/cli/install/bun-install-windows-rename-retry-fixture.ts new file mode 100644 index 000000000000..580683cb59df --- /dev/null +++ b/test/cli/install/bun-install-windows-rename-retry-fixture.ts @@ -0,0 +1,112 @@ +// Simulates an antivirus / search-indexer process scanning files that +// `bun install` has just written. On NTFS, an open handle that lacks +// FILE_SHARE_DELETE on any file inside a directory makes a rename of that +// directory fail with STATUS_ACCESS_DENIED. +// +// argv: +// +// Spin-polls watchDir until a subdirectory whose name contains subdirFilter +// ("" matches any) contains a regular file, opens that file via CreateFileW +// with dwShareMode = FILE_SHARE_READ | FILE_SHARE_WRITE (no DELETE), prints +// "HELD ", keeps the handle open for holdMs, closes it, prints +// "RELEASED" and exits 0. Prints "MISSED" and exits 0 if nothing shows up +// within 15s. + +import { dlopen, FFIType, ptr } from "bun:ffi"; +import { readdirSync } from "node:fs"; +import { join } from "node:path"; + +if (process.platform !== "win32") { + console.log("MISSED"); + process.exit(0); +} + +const [, , watchDir, subdirFilter, holdMsStr] = process.argv; +const holdMs = Number(holdMsStr); + +const { symbols } = dlopen("kernel32.dll", { + CreateFileW: { + args: [FFIType.ptr, FFIType.u32, FFIType.u32, FFIType.ptr, FFIType.u32, FFIType.u32, FFIType.ptr], + returns: FFIType.u64, + }, + CloseHandle: { args: [FFIType.u64], returns: FFIType.i32 }, +}); + +const GENERIC_READ = 0x80000000; +const FILE_SHARE_READ = 0x00000001; +const FILE_SHARE_WRITE = 0x00000002; +// Deliberately omitting FILE_SHARE_DELETE (0x00000004). +const OPEN_EXISTING = 3; +const FILE_ATTRIBUTE_NORMAL = 0x80; +const INVALID_HANDLE_VALUE = 0xffffffffffffffffn; + +function toWide(s: string): Uint8Array { + const buf = Buffer.alloc((s.length + 1) * 2); + for (let i = 0; i < s.length; i++) buf.writeUInt16LE(s.charCodeAt(i), i * 2); + return buf; +} + +function tryOpenNoShareDelete(path: string): bigint { + return symbols.CreateFileW( + ptr(toWide(path)), + GENERIC_READ, + FILE_SHARE_READ | FILE_SHARE_WRITE, + null, + OPEN_EXISTING, + FILE_ATTRIBUTE_NORMAL, + 0n, + ) as bigint; +} + +// Returns a handle to the first regular file found at most `depth` levels +// below `dir`, or INVALID_HANDLE_VALUE. Directories fail to open with +// FILE_ATTRIBUTE_NORMAL, which is what lets this tell them apart. +function grabFileBelow(dir: string, depth: number): [bigint, string] { + let entries; + try { + entries = readdirSync(dir, { withFileTypes: true }); + } catch { + return [INVALID_HANDLE_VALUE, ""]; + } + for (const entry of entries) { + const path = join(dir, entry.name); + if (entry.isFile()) { + const h = tryOpenNoShareDelete(path); + if (h !== INVALID_HANDLE_VALUE && h !== 0n) return [h, path]; + } else if (entry.isDirectory() && depth > 0) { + const found = grabFileBelow(path, depth - 1); + if (found[0] !== INVALID_HANDLE_VALUE) return found; + } + } + return [INVALID_HANDLE_VALUE, ""]; +} + +console.log("READY"); + +const deadline = Date.now() + 15_000; +let handle: bigint = INVALID_HANDLE_VALUE; +let heldPath = ""; +outer: while (Date.now() < deadline) { + let names: string[]; + try { + names = readdirSync(watchDir); + } catch { + continue; + } + for (const name of names) { + if (!name.includes(subdirFilter)) continue; + [handle, heldPath] = grabFileBelow(join(watchDir, name), 4); + if (handle !== INVALID_HANDLE_VALUE) break outer; + } +} + +if (handle === INVALID_HANDLE_VALUE) { + console.log("MISSED"); + process.exit(0); +} + +console.log("HELD " + heldPath); +await Bun.sleep(holdMs); +symbols.CloseHandle(handle); +console.log("RELEASED"); +process.exit(0); diff --git a/test/cli/install/bun-install-windows-rename-retry.test.ts b/test/cli/install/bun-install-windows-rename-retry.test.ts new file mode 100644 index 000000000000..fc53707c2729 --- /dev/null +++ b/test/cli/install/bun-install-windows-rename-retry.test.ts @@ -0,0 +1,272 @@ +// https://github.com/oven-sh/bun/issues/11250 +// +// `bun install` publishes directories into the cache by renaming them into +// place: the temp dir a tarball was extracted into, the temp dir a patch was +// applied in, and a global virtual store entry's staging dir. On Windows that +// rename fails with STATUS_ACCESS_DENIED (EPERM) for as long as any other +// process holds a handle without FILE_SHARE_DELETE on a file inside the +// directory, which is what antivirus / Search Indexer / MDM agents do to +// freshly written files. The fixture spawned below is such a process. +// +// Each publish path is exercised twice: with the default retry budget the +// install must outlast a 2s hold, and with BUN_INSTALL_WINDOWS_RENAME_RETRY_MS=0 +// it must fail immediately and name the variable (which also proves the held +// handle is what blocks the rename). + +import { afterAll, beforeAll, describe, expect, test } from "bun:test"; +import { bunEnv, bunExe, isWindows, tempDir } from "harness"; +import { createHash, randomBytes } from "node:crypto"; +import { existsSync, mkdirSync, readFileSync, readlinkSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; + +const PKG = "av-test-pkg"; +const ENV = "BUN_INSTALL_WINDOWS_RENAME_RETRY_MS"; +const HOLD_MS = 2000; + +const patch = `diff --git a/index.js b/index.js +--- a/index.js ++++ b/index.js +@@ -1 +1 @@ +-module.exports = "unpatched"; ++module.exports = "patched"; +`; + +let pkgDir: ReturnType | undefined; +let tgzBytes: Buffer; +let tgzSha1: string; + +beforeAll(async () => { + if (!isWindows) return; + const files: Record = { + "package/package.json": JSON.stringify({ name: PKG, version: "1.0.0" }), + "package/index.js": `module.exports = "unpatched";\n`, + // A blob that takes a moment to extract, plus enough files that copying + // or hardlinking the package into a staging dir is a window the fixture + // reliably lands in. + "package/bin.exe": randomBytes(2 * 1024 * 1024), + }; + for (let i = 0; i < 300; i++) files[`package/files/${i}.txt`] = `${i}\n`; + pkgDir = tempDir("rename-retry-pkg", files); + const tgz = join(String(pkgDir), `${PKG}-1.0.0.tgz`); + await Bun.$`tar -czf ${tgz} -C ${String(pkgDir)} package`.quiet(); + tgzBytes = readFileSync(tgz); + tgzSha1 = createHash("sha1").update(tgzBytes).digest("hex"); +}); + +afterAll(() => { + pkgDir?.[Symbol.dispose](); +}); + +function serveRegistry(stallTarballUntil?: Promise) { + const server = Bun.serve({ + port: 0, + async fetch(req) { + const { pathname } = new URL(req.url); + if (pathname === `/${PKG}`) { + return Response.json({ + name: PKG, + "dist-tags": { latest: "1.0.0" }, + versions: { + "1.0.0": { + name: PKG, + version: "1.0.0", + dist: { tarball: `http://localhost:${server.port}/${PKG}/-/${PKG}-1.0.0.tgz`, shasum: tgzSha1 }, + }, + }, + }); + } + if (pathname === `/${PKG}/-/${PKG}-1.0.0.tgz`) { + const headers = { "content-type": "application/octet-stream", "content-length": String(tgzBytes.length) }; + if (!stallTarballUntil) return new Response(tgzBytes, { headers }); + // Send the first half, then hold the rest back until the fixture has + // grabbed a handle, so the extraction dir is guaranteed to be held + // when bun tries to rename it into the cache. + const half = tgzBytes.length >> 1; + return new Response( + new ReadableStream({ + type: "direct", + async pull(ctrl) { + ctrl.write(tgzBytes.subarray(0, half)); + await ctrl.flush(); + await stallTarballUntil; + ctrl.write(tgzBytes.subarray(half)); + await ctrl.flush(); + ctrl.close(); + }, + }), + { headers }, + ); + } + return new Response("not found", { status: 404 }); + }, + }); + return server; +} + +function spawnBlocker(watchDir: string, subdirFilter: string, holdMs: number) { + const proc = Bun.spawn({ + cmd: [ + bunExe(), + join(import.meta.dir, "bun-install-windows-rename-retry-fixture.ts"), + watchDir, + subdirFilter, + String(holdMs), + ], + env: bunEnv, + stdout: "pipe", + stderr: "inherit", + stdin: "ignore", + }); + const ready = Promise.withResolvers(); + const held = Promise.withResolvers(); + let output = ""; + const drained = (async () => { + const decoder = new TextDecoder(); + for await (const chunk of proc.stdout) { + output += decoder.decode(chunk, { stream: true }); + if (output.includes("READY")) ready.resolve(); + if (output.includes("HELD") || output.includes("MISSED")) held.resolve(); + } + ready.resolve(); + held.resolve(); + })(); + return { + ready: ready.promise, + held: held.promise, + async finish() { + proc.kill(); + await proc.exited; + await drained; + return output; + }, + }; +} + +async function runInstall(cwd: string, env: Record) { + await using proc = Bun.spawn({ + cmd: [bunExe(), "install"], + cwd, + env: { ...bunEnv, ...env }, + stdout: "pipe", + stderr: "pipe", + stdin: "ignore", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + return { stdout, stderr, exitCode }; +} + +function scaffold(name: string, bunfig: string, packageJson: Record) { + const dir = tempDir(name, { + "cache/.keep": "", + "tmp/.keep": "", + "project/package.json": JSON.stringify({ + name: "app", + version: "1.0.0", + dependencies: { [PKG]: "1.0.0" }, + ...packageJson, + }), + "project/bunfig.toml": bunfig, + }); + const root = String(dir); + const cache = join(root, "cache"); + const tmp = join(root, "tmp"); + return { + [Symbol.dispose]: () => dir[Symbol.dispose](), + project: join(root, "project"), + cache, + tmp, + env: { BUN_INSTALL_CACHE_DIR: cache, BUN_TMPDIR: tmp, TEMP: tmp, TMP: tmp }, + }; +} + +const registryBunfig = (port: number) => `[install]\nregistry = "http://localhost:${port}/"\n`; + +const modes = [ + { mode: "default budget outlasts a 2s hold", budget: undefined, holdMs: HOLD_MS }, + { mode: `${ENV}=0 fails at once and names the variable`, budget: "0", holdMs: 15_000 }, +]; + +describe.skipIf(!isWindows).concurrent("bun install renames into the cache while a scanner holds a file open", () => { + test.each(modes)("extracted tarball: $mode", async ({ budget, holdMs }) => { + const blockerCaught = Promise.withResolvers(); + await using server = serveRegistry(blockerCaught.promise); + using s = scaffold("rename-retry-tarball", registryBunfig(server.port), {}); + + const blocker = spawnBlocker(s.tmp, "", holdMs); + await blocker.ready; + blocker.held.then(blockerCaught.resolve); + + const result = await runInstall(s.project, { + ...s.env, + // Stream the (small) tarball so files hit the temp dir before the registry stalls. + BUN_INSTALL_STREAMING_MIN_SIZE: "1", + [ENV]: budget, + }); + const blockerOut = await blocker.finish(); + expect(blockerOut).toContain("HELD"); + + if (budget === undefined) { + expect(result).toMatchObject({ exitCode: 0 }); + expect(existsSync(join(s.project, "node_modules", PKG, "bin.exe"))).toBe(true); + } else { + expect(result.stderr).toContain(`moving "${PKG}" to cache dir failed`); + expect(result.stderr).toContain(ENV); + expect(result.stderr).toContain("NtSetInformationFile"); + expect(result.exitCode).toBe(1); + } + }); + + test.each(modes)("patched package: $mode", async ({ budget, holdMs }) => { + await using server = serveRegistry(); + using s = scaffold("rename-retry-patch", registryBunfig(server.port), { + patchedDependencies: { [`${PKG}@1.0.0`]: `patches/${PKG}.patch` }, + }); + mkdirSync(join(s.project, "patches")); + writeFileSync(join(s.project, "patches", `${PKG}.patch`), patch); + + // The patch is applied in a `.-.tmp` dir under the temp dir. The + // tarball is extracted into a `.-.av-test-pkg` sibling first, + // which the filter keeps the fixture away from. + const blocker = spawnBlocker(s.tmp, ".tmp", holdMs); + await blocker.ready; + + const result = await runInstall(s.project, { ...s.env, [ENV]: budget }); + const blockerOut = await blocker.finish(); + expect(blockerOut).toContain("HELD"); + + if (budget === undefined) { + expect(result).toMatchObject({ exitCode: 0 }); + expect(readFileSync(join(s.project, "node_modules", PKG, "index.js"), "utf8")).toContain('"patched"'); + } else { + expect(result.stderr).toContain("renaming changes to cache dir"); + expect(result.stderr).toContain(ENV); + expect(result.exitCode).not.toBe(0); + } + }); + + test.each(modes)("global virtual store entry: $mode", async ({ budget, holdMs }) => { + await using server = serveRegistry(); + using s = scaffold("rename-retry-global-store", registryBunfig(server.port) + `linker = "isolated"\n`, {}); + + // Entries are assembled in `/links/.tmp-` and + // renamed to `/links/`. + const blocker = spawnBlocker(join(s.cache, "links"), ".tmp-", holdMs); + await blocker.ready; + + const result = await runInstall(s.project, { ...s.env, BUN_INSTALL_GLOBAL_STORE: "1", [ENV]: budget }); + const blockerOut = await blocker.finish(); + expect(blockerOut).toContain("HELD"); + + if (budget === undefined) { + expect(result).toMatchObject({ exitCode: 0 }); + const entry = readlinkSync(join(s.project, "node_modules", ".bun", `${PKG}@1.0.0`)); + expect(existsSync(join(entry, "node_modules", PKG, "bin.exe"))).toBe(true); + } else { + // Without the retry this path mistook the held staging dir for an + // entry a concurrent install had published, deleted it and reported + // success, leaving node_modules/.bun pointing at nothing. + expect(result.stderr).toContain(ENV); + expect(result.exitCode).not.toBe(0); + } + }); +}); From e8a6264bc8b4c1b792baec7743f398558ffad2a0 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 13 Aug 2026 01:44:03 +0000 Subject: [PATCH 7/8] tighten comments, generalize the exhausted-retry hint --- src/bun_core/env_var.rs | 10 +++--- src/install/cache_rename.rs | 37 ++++++++--------------- src/install/extract_tarball.rs | 11 +++---- src/install/isolated_install/Installer.rs | 8 ++--- 4 files changed, 23 insertions(+), 43 deletions(-) diff --git a/src/bun_core/env_var.rs b/src/bun_core/env_var.rs index 3eb6ebaeb3e8..716f8e0807c0 100644 --- a/src/bun_core/env_var.rs +++ b/src/bun_core/env_var.rs @@ -105,12 +105,10 @@ new!(pub BUN_INSTALL_STREAMING_MIN_SIZE: unsigned, "BUN_INSTALL_STREAMING_MIN_SI // thread schedules a drain; collapses the per-chunk thread-pool futex wake // into roughly one per `threshold` bytes. new!(pub BUN_INSTALL_STREAMING_DRAIN_THRESHOLD: unsigned, "BUN_INSTALL_STREAMING_DRAIN_THRESHOLD", { default: 256 * 1024 }); -// How long (ms) `bun install` keeps retrying a rename into the cache on -// Windows while a scanner holds a file in the directory open (see -// `bun_install::cache_rename`). 5s covers the real-time scan of a -// multi-megabyte binary with margin (SQLite's equivalent retry waits 1.4s, -// graceful-fs 60s); it is also what a permanent failure such as an unwritable -// cache dir now costs per package before it is reported. 0 disables retrying. +// How long `bun install` retries a cache-publish rename that Windows fails +// because a scanner has a file in the directory open (`bun_install::cache_rename`). +// 5s outlasts a real-time scan of a multi-MB binary (SQLite retries 1.4s, +// graceful-fs 60s) and is also the per-package cost of a permanent failure. new!(pub BUN_INSTALL_WINDOWS_RENAME_RETRY_MS: unsigned, "BUN_INSTALL_WINDOWS_RENAME_RETRY_MS", { default: 5_000 }); new!(pub BUN_NEEDS_PROC_SELF_WORKAROUND: boolean, "BUN_NEEDS_PROC_SELF_WORKAROUND", { default: false }); new!(pub BUN_OPTIONS: string, "BUN_OPTIONS", {}); diff --git a/src/install/cache_rename.rs b/src/install/cache_rename.rs index 92342332a558..2acf66f1aaf8 100644 --- a/src/install/cache_rename.rs +++ b/src/install/cache_rename.rs @@ -1,15 +1,11 @@ -//! Retry budget shared by the renames that publish a freshly built directory -//! into the install cache: an extracted tarball, a patched package, or a -//! global virtual store entry. +//! Retry budget for the renames that publish a directory into the install +//! cache (extracted tarball, patched package, global virtual store entry). //! -//! On Windows, renaming a directory fails with `STATUS_ACCESS_DENIED` or -//! `STATUS_SHARING_VIOLATION` while any other process holds a handle without -//! `FILE_SHARE_DELETE` on a file inside it. Antivirus, the Search Indexer and -//! endpoint agents open freshly written files exactly that way, typically for -//! tens of milliseconds up to a few seconds, so the rename is retried until -//! `BUN_INSTALL_WINDOWS_RENAME_RETRY_MS` has elapsed. POSIX renames are not -//! affected by open handles and `EPERM`/`EACCES` are real permission failures -//! there, so nothing is ever retried off Windows. +//! On Windows a directory rename fails with `STATUS_ACCESS_DENIED` or +//! `STATUS_SHARING_VIOLATION` while any process holds a handle without +//! `FILE_SHARE_DELETE` on a file inside it, which is how antivirus and the +//! Search Indexer open freshly written files. Nothing is retried on POSIX, +//! where open handles do not block renames and `EPERM` is a real failure. use core::fmt; use core::time::Duration; @@ -22,8 +18,7 @@ pub(crate) const ENV_VAR_NAME: &str = "BUN_INSTALL_WINDOWS_RENAME_RETRY_MS"; pub(crate) struct RenameRetry { started: Instant, budget: Duration, - /// Sleep before the next attempt; grows 10ms per attempt and caps at 100ms, - /// which is the schedule npm's `graceful-fs` uses for the same failure. + /// graceful-fs schedule: +10ms per attempt, capped at 100ms. next_backoff: Duration, exhausted: bool, } @@ -42,8 +37,6 @@ impl RenameRetry { } } - /// Whether `err` is one of the errors Windows reports while another process - /// holds a handle inside the directory being renamed or at its destination. pub(crate) fn is_transient(err: &sys::Error) -> bool { cfg!(windows) && matches!( @@ -52,8 +45,7 @@ impl RenameRetry { ) } - /// Called after a failed attempt. Sleeps and returns `true` while the budget - /// allows another attempt; returns `false` once it is spent. + /// Sleeps and returns `true` if another attempt fits in the budget. pub(crate) fn wait(&mut self) -> bool { if self.started.elapsed() >= self.budget { self.exhausted = true; @@ -69,15 +61,10 @@ impl RenameRetry { self.exhausted } - /// Suffix for the error reported to the user once `wait()` has returned - /// `false`; displays as nothing otherwise. + /// Error-message suffix; displays as nothing unless the budget ran out. pub(crate) fn exhausted_hint(&self) -> ExhaustedHint { ExhaustedHint { - waited: if self.exhausted { - Some(self.started.elapsed()) - } else { - None - }, + waited: self.exhausted.then(|| self.started.elapsed()), } } } @@ -91,7 +78,7 @@ impl fmt::Display for ExhaustedHint { match self.waited { Some(waited) => write!( f, - " (gave up after retrying for {}ms; another process is holding a file open in the directory. Set {} to wait longer)", + " (gave up after retrying for {}ms; usually another process such as antivirus has a file in the directory open. Set {} to wait longer)", waited.as_millis(), ENV_VAR_NAME, ), diff --git a/src/install/extract_tarball.rs b/src/install/extract_tarball.rs index ea645f9072f1..4b0690c6d515 100644 --- a/src/install/extract_tarball.rs +++ b/src/install/extract_tarball.rs @@ -525,13 +525,10 @@ impl ExtractTarball { // Now that we've extracted the archive, we rename. #[cfg(windows)] { - // The rename fails transiently when another process holds a - // handle into either directory: a concurrent `bun install` - // sharing the cache still has the destination open (EXIST / - // NOTEMPTY, or PERM since NTFS reports replacing a directory - // that way too), or a scanner has one of our freshly extracted - // files open (PERM / BUSY, see `cache_rename`). Both are - // retried against the same `RenameRetry` budget. + // Transient on Windows while another process holds a handle + // in either directory: a concurrent `bun install` sharing the + // cache (EXIST/NOTEMPTY, or PERM for a directory destination) + // or a scanner reading what we just extracted (PERM/BUSY). let mut retry = RenameRetry::start(); let mut path2_buf = WPathBuffer::uninit(); let path2 = strings::to_wpath_normalized(&mut path2_buf, folder_name); diff --git a/src/install/isolated_install/Installer.rs b/src/install/isolated_install/Installer.rs index a55ad83ec3e8..e8463165d0e1 100644 --- a/src/install/isolated_install/Installer.rs +++ b/src/install/isolated_install/Installer.rs @@ -2818,11 +2818,9 @@ pub enum Which { fn is_rename_collision(err: &sys::Error, final_: &ZStr) -> bool { match err.get_errno() { sys::Errno::EEXIST | sys::Errno::ENOTEMPTY => true, - // Windows maps a rename onto an existing directory to - // ERROR_ACCESS_DENIED, but reports a scanner holding one of our - // staged files open the same way (see `cache_rename`); only the - // destination existing makes it a collision. On POSIX PERM/ACCES are - // real permission failures and must propagate. + // Windows reports both "destination directory exists" and "a scanner + // has one of our staged files open" as ERROR_ACCESS_DENIED; only the + // former is a collision. On POSIX these are real permission failures. sys::Errno::EPERM | sys::Errno::EACCES => cfg!(windows) && sys::exists_z(final_), _ => false, } From 2172ab4f25fa512ffcceae9507e48b94c9ff56d2 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 13 Aug 2026 01:51:16 +0000 Subject: [PATCH 8/8] retry the --force swap-out rename too; take the env var name from its accessor --- src/install/cache_rename.rs | 11 +++-------- src/install/isolated_install/Installer.rs | 6 +++++- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/install/cache_rename.rs b/src/install/cache_rename.rs index 2acf66f1aaf8..6986ae6aa449 100644 --- a/src/install/cache_rename.rs +++ b/src/install/cache_rename.rs @@ -11,10 +11,9 @@ use core::fmt; use core::time::Duration; use std::time::Instant; +use bun_core::env_var::BUN_INSTALL_WINDOWS_RENAME_RETRY_MS; use bun_sys as sys; -pub(crate) const ENV_VAR_NAME: &str = "BUN_INSTALL_WINDOWS_RENAME_RETRY_MS"; - pub(crate) struct RenameRetry { started: Instant, budget: Duration, @@ -27,11 +26,7 @@ impl RenameRetry { pub(crate) fn start() -> Self { Self { started: Instant::now(), - budget: Duration::from_millis( - bun_core::env_var::BUN_INSTALL_WINDOWS_RENAME_RETRY_MS - .get() - .unwrap_or(5_000), - ), + budget: Duration::from_millis(BUN_INSTALL_WINDOWS_RENAME_RETRY_MS.get().unwrap()), next_backoff: Duration::ZERO, exhausted: false, } @@ -80,7 +75,7 @@ impl fmt::Display for ExhaustedHint { f, " (gave up after retrying for {}ms; usually another process such as antivirus has a file in the directory open. Set {} to wait longer)", waited.as_millis(), - ENV_VAR_NAME, + bstr::BStr::new(BUN_INSTALL_WINDOWS_RENAME_RETRY_MS.key().as_bytes()), ), None => Ok(()), } diff --git a/src/install/isolated_install/Installer.rs b/src/install/isolated_install/Installer.rs index e8463165d0e1..eb05f25c5cfb 100644 --- a/src/install/isolated_install/Installer.rs +++ b/src/install/isolated_install/Installer.rs @@ -2481,10 +2481,14 @@ impl<'a> Installer<'a> { store::entry::fmt_global_store_path(entry_id, self.store, self.lockfile()), bun_core::fast_random(), )); - if let Some(swap_err) = + while let Some(swap_err) = sys::renameat(Fd::cwd(), final_.slice_z(), Fd::cwd(), old.slice_z()).err() { + if RenameRetry::is_transient(&swap_err) && retry.wait() { + continue; + } let _ = Fd::cwd().delete_tree(staging.slice()); + report_exhausted_publish(&retry, &final_); return sys::Result::Err(swap_err); } loop {