Skip to content
5 changes: 5 additions & 0 deletions src/bun_core/env_var.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,11 @@ 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 `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.
Comment thread
robobun marked this conversation as resolved.
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", {});
Expand Down
88 changes: 88 additions & 0 deletions src/install/cache_rename.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
//! Retry budget for the renames that publish a directory into the install
//! cache (extracted tarball, patched package, global virtual store entry).
//!
//! 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.
Comment thread
robobun marked this conversation as resolved.

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";
Comment thread
robobun marked this conversation as resolved.
Outdated
Comment thread
robobun marked this conversation as resolved.
Outdated

pub(crate) struct RenameRetry {
started: Instant,
budget: Duration,
/// graceful-fs schedule: +10ms per attempt, capped at 100ms.
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,
}
}

pub(crate) fn is_transient(err: &sys::Error) -> bool {
cfg!(windows)
&& matches!(
err.get_errno(),
sys::Errno::EPERM | sys::Errno::EACCES | sys::Errno::EBUSY
)
}

/// 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;
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
}

/// Error-message suffix; displays as nothing unless the budget ran out.
pub(crate) fn exhausted_hint(&self) -> ExhaustedHint {
ExhaustedHint {
waited: self.exhausted.then(|| self.started.elapsed()),
}
}
}

pub(crate) struct ExhaustedHint {
waited: Option<Duration>,
}

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; usually another process such as antivirus has a file in the directory open. Set {} to wait longer)",
waited.as_millis(),
ENV_VAR_NAME,
),
None => Ok(()),
}
}
}
102 changes: 45 additions & 57 deletions src/install/extract_tarball.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -522,12 +525,11 @@ 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;
let mut retries: u32 = 0;
// 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).
Comment thread
robobun marked this conversation as resolved.
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 {
Expand Down Expand Up @@ -573,65 +575,51 @@ impl ExtractTarball {
true,
) {
bun_sys::Result::Err(err) => {
if retries < MAX_RETRIES {
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());
}
}
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;
// 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
Comment thread
robobun marked this conversation as resolved.
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),
Expand Down
140 changes: 85 additions & 55 deletions src/install/isolated_install/Installer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
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};
Expand Down Expand Up @@ -2446,61 +2447,80 @@
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.
Comment thread
robobun marked this conversation as resolved.
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);
}
Comment thread
robobun marked this conversation as resolved.
Outdated
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.
Comment thread
robobun marked this conversation as resolved.
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.
Comment thread
robobun marked this conversation as resolved.
sys::Result::Ok(())
}

/// Project-local path `node_modules/.bun/<storepath>` (the symlink that
Expand Down Expand Up @@ -2795,13 +2815,23 @@
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 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.
Comment thread
robobun marked this conversation as resolved.
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!(
"<r><red>error<r>: publishing {} to the global store failed{}",
bstr::BStr::new(final_.slice()),
retry.exhausted_hint(),
);
}

Check warning on line 2836 in src/install/isolated_install/Installer.rs

View check run for this annotation

Claude / Claude Code Review

report_exhausted_publish emits a separate 'error:' line via pretty_errorln! instead of threading the hint into the returned error

`report_exhausted_publish` writes its own `error:` line via `pretty_errorln!` and then the returned `sys::Error` is reported again as `error: failed to link package: pkg@ver` at Installer.rs:326-329, so one exhausted retry produces two `error:` lines — the sibling paths in extract_tarball.rs and patch_install.rs interpolate `retry.exhausted_hint()` inline into their single logged error instead. Not blocking (`sys::Result<()>` has no room to carry the hint and the two lines together are still com
Comment thread
robobun marked this conversation as resolved.
}
Loading
Loading