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 7 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
179 changes: 149 additions & 30 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,49 @@ impl HardLinkWindowsInstallTask {
let dest_len = dest.len() - 1;
debug_assert_eq!(dest[dest_len], 0);

// Concurrent bun processes installing the same package from a shared
// cache (e.g. parallel `bun x <tool>` runs share one bunx install dir)
// race on every file here: a peer briefly holding the file open
// surfaces as an EBUSY-class sharing violation, and a peer starting
// its own install of the package renames the destination dir away
// (uninstall-before-install), which surfaces as ENOENT between our
// mkdir and link. Both are transient, so retry with backoff,
// mirroring the cache-move retries in extract_tarball. Each process
// renames the destination at most once per package, so the storm is
// bounded and the last installer's links land in the final dir.
Comment thread
robobun marked this conversation as resolved.
Outdated
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). `None` on success, including when `dest` already is
/// the same file object as `src` — a concurrent install of the same
/// package landed the identical link first, which must not be treated as
/// a conflict to delete and recreate.
Comment thread
robobun marked this conversation as resolved.
Outdated
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 +677,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 +699,13 @@ impl HardLinkWindowsInstallTask {
return None;
}

// The DeleteFileW above can fail while a peer holds the file open, in
// which case `dest` may still be the link we want; CopyFileW onto it
// would copy the file over itself and fail with a sharing violation.
Comment thread
robobun marked this conversation as resolved.
Outdated
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 @@ -1582,12 +1626,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) let the loop re-open
// `destination_dir` after a concurrent install renames it away.
Comment thread
robobun marked this conversation as resolved.
Outdated
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 +1654,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 +1682,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 +1700,83 @@ 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 {
// Concurrent bun processes installing the same
// package from a shared cache (e.g. parallel
// `bun x <tool>` runs share one bunx install dir)
// race on every file here: a peer that already
// linked this file surfaces as EEXIST, and a peer
// starting its own install of the package renames
// the destination dir away
// (uninstall-before-install), which surfaces as
// ENOENT. A dest that already is the same inode
// counts as success; the rest retries right after
// the fix-up (unlink the stale file, recreate the
// dir). No sleeps: this loop runs on the install
// thread, and the fix-up needs no waiting — each
// peer renames the destination at most once per
// package, so the retries are bounded in practice
// too. The Windows hardlink task does the same
// dance on the worker pool.
Comment thread
robobun marked this conversation as resolved.
Outdated
const MAX_RETRIES: u32 = 8;
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.
sys::E::ENOENT => {
// `destination_dir` may point at a
// directory a peer renamed away and
// deleted; nothing can be created
// inside an unlinked directory, so
// re-open the destination path fresh
// before retrying.
Comment thread
robobun marked this conversation as resolved.
Outdated
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;
}

real_file_count += 1;
Expand Down Expand Up @@ -1727,7 +1842,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 +1853,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
Loading