Skip to content

ai slop #36955

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
robobun wants to merge 18 commits into from
Closed

ai slop #36955

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
8d50ddf
install: tolerate concurrent installs racing on the same destination …
robobun Aug 5, 2026
b582f96
test: concurrent installs into the same destination tolerate racing h…
robobun Aug 5, 2026
7ee45be
install: keep the existing cache entry when a concurrent extract move…
robobun Aug 5, 2026
422ac38
install: retry transient ENOENT when a peer renames the destination d…
robobun Aug 5, 2026
f27a19b
test: cover cold-cache concurrent extraction in the racing installs test
robobun Aug 5, 2026
897335a
install: survive destination renames from concurrent installs on POSIX
robobun Aug 5, 2026
377f91e
[autofix.ci] apply automated fixes
autofix-ci[bot] Aug 5, 2026
b464718
Tighten comments
robobun Aug 5, 2026
6eb3a5a
install: probe package.json for tarball cache entries; assert file co…
robobun Aug 5, 2026
a3ff14f
install: retry racing destination-dir opens and stage the POSIX backoff
robobun Aug 5, 2026
c99bfd9
install: accept an already-identical link in the isolated linker
robobun Aug 5, 2026
9da7429
Allow unused mut on the Windows arm
robobun Aug 5, 2026
51da66d
install: recover from EINVAL for an unlinked destination dirfd on macOS
robobun Aug 5, 2026
06c4eae
install: symlink backend EEXIST retry must link to the cache target, …
robobun Aug 5, 2026
0907aac
install: give the macOS clonefile copy loop the same destination-rena…
robobun Aug 5, 2026
f6eadd8
[autofix.ci] apply automated fixes
autofix-ci[bot] Aug 5, 2026
d4b9960
install: propagate non-transient destination re-open errors
robobun Aug 5, 2026
9e99ab2
[autofix.ci] apply automated fixes
autofix-ci[bot] Aug 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
261 changes: 217 additions & 44 deletions src/install/PackageInstall.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -614,7 +614,6 @@ impl HardLinkWindowsInstallTask {
}

fn run(&mut self) -> Option<crate::Error> {
use bun_sys::windows;
// Read scalar fields before borrowing `bytes` so no `&mut self` reborrow
// overlaps the slice borrows below.
let src_len = self.src_len;
Expand All @@ -626,6 +625,41 @@ impl HardLinkWindowsInstallTask {
let dest_len = dest.len() - 1;
debug_assert_eq!(dest[dest_len], 0);

// Peers installing the same package cause transient EBUSY (file
// briefly held open) and ENOENT (destination dir renamed aside by
// uninstall-before-install); retry with the same backoff
// extract_tarball uses for cache moves.
Comment thread
robobun marked this conversation as resolved.
const MAX_RETRIES: u32 = 4;
let mut retries: u32 = 0;
loop {
let err = match Self::link_or_copy(src, dest, dest_len, basename) {
None => return None,
Some(err) => err,
};
let transient = matches!(
err,
crate::Error::Sys(bun_errno::SystemErrno::EBUSY)
| crate::Error::Sys(bun_errno::SystemErrno::ENOENT)
);
if !transient || retries == MAX_RETRIES {
return Some(err);
}
retries += 1;
std::thread::sleep(std::time::Duration::from_millis(10u64 << (retries - 1)));
}
}

/// One attempt at materializing `dest` as a hard link of `src`, falling
/// back to a copy. A `dest` that already is the same file object as `src`
/// (a peer linked it first) is success, not a conflict.
Comment thread
robobun marked this conversation as resolved.
fn link_or_copy(
src: &[u16],
dest: &mut [u16],
dest_len: usize,
basename: usize,
) -> Option<crate::Error> {
use bun_sys::windows;

// `windows::CreateHardLinkW` is the safe wrapper (logs + Option<&mut SA>).
if windows::CreateHardLinkW(dest.as_ptr(), src.as_ptr(), None) != 0 {
return None;
Expand All @@ -635,13 +669,8 @@ impl HardLinkWindowsInstallTask {
windows::Win32Error::ALREADY_EXISTS
| windows::Win32Error::FILE_EXISTS
| windows::Win32Error::CANNOT_MAKE => {
// Race condition: this shouldn't happen
if cfg!(debug_assertions) {
bun_output::scoped_log!(
install,
"CreateHardLinkW returned EEXIST, this shouldn't happen: {}",
bun_core::fmt::fmt_path_u16(&dest[..dest_len], Default::default())
);
if windows::same_file_w(dest.as_ptr(), src.as_ptr()) {
return None;
}
// SAFETY: FFI — dest is a valid NUL-terminated u16 buffer.
unsafe { windows::DeleteFileW(dest.as_ptr()) };
Expand All @@ -662,6 +691,12 @@ impl HardLinkWindowsInstallTask {
return None;
}

// If the DeleteFileW above failed, dest may still be the link we
// want; CopyFileW onto it would copy the file over itself and fail.
Comment thread
robobun marked this conversation as resolved.
if windows::same_file_w(dest.as_ptr(), src.as_ptr()) {
return None;
}

if PackageManager::verbose_install() {
bun_core::run_once! {{
bun_core::warn!(
Expand Down Expand Up @@ -1046,13 +1081,18 @@ impl<'a> PackageInstall<'a> {
};
walker_.resolve_unknown_entry_types = true;

fn copy(destination_dir_: &Dir, walker: &mut Walker) -> crate::Result<u32> {
fn copy(
destbase: &Dir,
destpath: &ZStr,
subdir: &mut Dir,
walker: &mut Walker,
) -> crate::Result<u32> {
let mut real_file_count: u32 = 0;
let mut stackpath = [0u8; path::MAX_PATH_BYTES];
while let Some(entry) = walker.next()? {
match entry.kind {
EntryKind::Directory => {
let _ = sys::mkdirat(destination_dir_, entry.path, 0o755);
let _ = sys::mkdirat(&*subdir, entry.path, 0o755);
}
EntryKind::File => {
let path_len = entry.path.len();
Expand All @@ -1066,25 +1106,62 @@ impl<'a> PackageInstall<'a> {
// buffer (which were UB-adjacent as aliased `&mut`s).
let path_ = ZStr::from_buf(&stackpath, path_len);
let basename = ZStr::from_buf(&stackpath[path_len - base_len..], base_len);
match sys::clonefileat(entry.dir, basename, destination_dir_.fd(), path_) {
Ok(()) => {}
// Same retry dance as the hardlink loop below: a peer
// installing the same package renames the destination
// dir aside (uninstall-before-install), which
// surfaces as ENOENT (or EINVAL for an unlinked
// dirfd); re-open by path and retry.
Comment thread
robobun marked this conversation as resolved.
const MAX_RETRIES: u32 = 6;
let mut retries: u32 = 0;
loop {
let err =
match sys::clonefileat(entry.dir, basename, subdir.fd(), path_) {
Ok(()) => break,
Err(e) => e,
};
// `get_errno` bounds-checks (SUCCESS for out-of-range errno) — avoids
// `from_raw`'s release-mode transmute on an unexpected value.
Err(e) => match e.get_errno() {
match err.get_errno() {
sys::Errno::EXDEV => return Err(crate::Error::NotSupported), // not same file system
sys::Errno::EOPNOTSUPP => {
return Err(crate::Error::NotSupported);
}
sys::Errno::ENOENT => {
return Err(crate::Error::Sys(bun_errno::SystemErrno::ENOENT));
// A peer's clone of the same source; the
// package's own node_modules can also collide.
Comment thread
robobun marked this conversation as resolved.
sys::Errno::EEXIST => break,
sys::Errno::ENOENT | sys::Errno::EINVAL => {
if retries == MAX_RETRIES {
return Err(crate::Error::Sys(
bun_errno::SystemErrno::ENOENT,
));
}
if let Ok(reopened) = destbase.make_open_path(
destpath.as_bytes(),
OpenDirOptions::default(),
) {
*subdir = reopened;
}
let entry_dirname = bun_paths::resolve_path::dirname::<
bun_paths::platform::Auto,
>(entry.path.as_bytes());
if !entry_dirname.is_empty() {
let _ = bun_sys::MakePath::make_path::<OSPathChar>(
&*subdir,
entry_dirname,
);
}
retries += 1;
if retries >= 3 {
std::thread::sleep(std::time::Duration::from_millis(
10u64 << (retries - 3),
));
}
}
// sometimes the downloaded npm package has already node_modules with it, so just ignore exist error here
sys::Errno::EEXIST => {}
sys::Errno::EACCES => {
return Err(crate::Error::Sys(bun_errno::SystemErrno::EACCES));
}
_ => return Err(crate::Error::Unexpected),
},
}
}

real_file_count += 1;
Expand All @@ -1096,14 +1173,19 @@ impl<'a> PackageInstall<'a> {
Ok(real_file_count)
}

let subdir = match destination_dir.make_open_path(
let mut subdir = match destination_dir.make_open_path(
self.destination_dir_subpath.as_bytes(),
OpenDirOptions::default(),
) {
Ok(d) => d,
Err(err) => return Ok(InstallResult::fail(err.into(), Step::OpeningDestDir, None)),
};
self.file_count = match copy(&subdir, &mut walker_) {
self.file_count = match copy(
destination_dir,
self.destination_dir_subpath,
&mut subdir,
&mut walker_,
) {
Ok(n) => n,
Err(err) => return Ok(InstallResult::fail(err, Step::CopyingFiles, None)),
};
Expand Down Expand Up @@ -1568,7 +1650,27 @@ impl<'a> PackageInstall<'a> {

fn install_with_hardlink(&mut self, dest_dir: &Dir) -> crate::Result<InstallResult> {
let mut state = InstallDirState::default();
let res = self.init_install_dir(&mut state, dest_dir, Method::Hardlink);
#[cfg_attr(windows, allow(unused_mut))]
let mut res = self.init_install_dir(&mut state, dest_dir, Method::Hardlink);
// A peer installing the same package can rename the destination dir
// aside between our mkdir and open (uninstall-before-install);
// re-init rather than failing the package.
Comment thread
robobun marked this conversation as resolved.
#[cfg(not(windows))]
for _ in 0..4 {
match &res {
InstallResult::Failure(f)
if f.step == Step::OpeningDestDir
&& matches!(
f.err,
crate::Error::Sys(bun_errno::SystemErrno::ENOENT)
| crate::Error::FileNotFound
) => {}
_ => break,
}
std::thread::sleep(std::time::Duration::from_millis(5));
state = InstallDirState::default();
res = self.init_install_dir(&mut state, dest_dir, Method::Hardlink);
}
if res.is_fail() {
return Ok(res);
}
Expand All @@ -1582,12 +1684,24 @@ impl<'a> PackageInstall<'a> {
#[cfg(not(windows))]
type WinOffset = ();

#[cfg(windows)]
type PosixDirRef<'b> = ();
#[cfg(not(windows))]
type PosixDirRef<'b> = &'b Dir;
#[cfg(windows)]
type PosixZStrRef<'b> = ();
#[cfg(not(windows))]
type PosixZStrRef<'b> = &'b ZStr;

// Two overlapping slices into the same buffer (`head` is the whole
// buffer, `to_copy_into` is its tail) would be two live aliasing
// `&mut [u16]`, which is UB — pass head buffer + tail offset and
// reslice inside.
// reslice inside. `destbase`/`destpath` (posix) re-open
// `destination_dir` if a peer renames it away.
Comment thread
robobun marked this conversation as resolved.
fn copy(
destination_dir: &Dir,
destbase: PosixDirRef<'_>,
destpath: PosixZStrRef<'_>,
destination_dir: &mut Dir,
walker: &mut Walker,
to_copy_into1_offset: WinOffset,
head1: WinSlice<'_>,
Expand All @@ -1598,7 +1712,7 @@ impl<'a> PackageInstall<'a> {
#[cfg(not(windows))]
let _ = (to_copy_into1_offset, head1, to_copy_into2_offset, head2);
#[cfg(windows)]
let _ = destination_dir;
let _ = (destination_dir, destbase, destpath);
#[cfg(windows)]
let queue = HardLinkWindowsInstallTask::init_queue();
// on Windows, tasks already pushed to `queue` are running on
Expand Down Expand Up @@ -1626,7 +1740,7 @@ impl<'a> PackageInstall<'a> {
match entry.kind {
EntryKind::Directory => {
let _ = bun_sys::MakePath::make_path::<OSPathChar>(
destination_dir,
&*destination_dir,
entry.path.as_bytes(),
);
}
Expand All @@ -1644,24 +1758,79 @@ impl<'a> PackageInstall<'a> {
}
}

if let Err(err) = sys::linkat(
entry.dir,
entry.basename,
destination_dir.fd(),
entry.path,
) {
if err.get_errno() == sys::E::EEXIST {
let _ = sys::unlinkat(destination_dir, entry.path);
sys::linkat(
entry.dir,
entry.basename,
destination_dir.fd(),
entry.path,
)
.map_err(map_linkat_err)?;
} else {
// Peers installing the same package cause
// EEXIST (one already linked this file; same
// inode is success) and ENOENT (destination dir
// renamed aside by uninstall-before-install).
// Fix up and retry: first immediately (the
// single-process EEXIST case must not slow
// down), then with backoff to outlast a peer
// interfering more than once.
Comment thread
robobun marked this conversation as resolved.
const MAX_RETRIES: u32 = 6;
let mut retries: u32 = 0;
loop {
let err = match sys::linkat(
entry.dir,
entry.basename,
destination_dir.fd(),
entry.path,
) {
Ok(()) => break,
Comment thread
robobun marked this conversation as resolved.
Err(err) => err,
};
match err.get_errno() {
sys::E::EEXIST => {
if let (Ok(src), Ok(dest)) = (
sys::fstatat(entry.dir, entry.basename),
sys::fstatat(&*destination_dir, entry.path),
) {
if src.st_dev == dest.st_dev
&& src.st_ino == dest.st_ino
{
break;
}
}
let _ = sys::unlinkat(&*destination_dir, entry.path);
}
Comment thread
robobun marked this conversation as resolved.
// EINVAL: macOS reports it instead of
// ENOENT for a dirfd whose directory was
// unlinked.
Comment thread
robobun marked this conversation as resolved.
sys::E::ENOENT | sys::E::EINVAL => {
// The held fd may be an unlinked dir
// (peer renamed it aside and deleted
// it); re-open by path.
Comment thread
robobun marked this conversation as resolved.
if let Ok(reopened) = destbase.make_open_path(
destpath.as_bytes(),
OpenDirOptions {
iterate: true,
..Default::default()
},
) {
*destination_dir = reopened;
}
let entry_dirname = bun_paths::resolve_path::dirname::<
bun_paths::platform::Auto,
>(
entry.path.as_bytes()
);
if !entry_dirname.is_empty() {
let _ = bun_sys::MakePath::make_path::<OSPathChar>(
&*destination_dir,
entry_dirname,
);
}
}
_ => return Err(map_linkat_err(err)),
}
if retries == MAX_RETRIES {
return Err(map_linkat_err(err));
}
retries += 1;
if retries >= 3 {
std::thread::sleep(std::time::Duration::from_millis(
10u64 << (retries - 3),
));
}
}

real_file_count += 1;
Expand Down Expand Up @@ -1727,7 +1896,9 @@ impl<'a> PackageInstall<'a> {

#[cfg(windows)]
let result = copy(
&state.subdir,
(),
(),
&mut state.subdir,
state.walker.as_mut().unwrap(),
state.to_copy_buf_off,
&mut state.buf[..],
Expand All @@ -1736,7 +1907,9 @@ impl<'a> PackageInstall<'a> {
);
#[cfg(not(windows))]
let result = copy(
&state.subdir,
dest_dir,
self.destination_dir_subpath,
&mut state.subdir,
state.walker.as_mut().unwrap(),
(),
(),
Expand Down Expand Up @@ -1849,7 +2022,7 @@ impl<'a> PackageInstall<'a> {
}

let _ = sys::unlinkat(destination_dir, entry.path);
sys::symlinkat(entry.basename, destination_dir.fd(), entry.path)?;
sys::symlinkat(target, destination_dir.fd(), entry.path)?;
}

real_file_count += 1;
Expand Down
Loading
Loading