Skip to content

fix: concurrent pixi install #4566

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix: concurrent pixi install #4566

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c77dae5
fix(install): add file locking to prevent concurrent environment inst…
r-ushil-gsa Sep 10, 2025
237c44f
Merge branch 'prefix-dev:main' into fix/concurrent-pixi-install
r-ushil-gsa Sep 11, 2025
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
44 changes: 44 additions & 0 deletions crates/pixi_command_dispatcher/src/install_pixi/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use itertools::{Either, Itertools};
use miette::Diagnostic;
use pixi_build_discovery::EnabledProtocols;
use pixi_record::{PixiRecord, SourceRecord};
use pixi_utils::AsyncPrefixGuard;
use rattler::install::{
Installer, InstallerError, Transaction,
link_script::{LinkScriptError, PrePostLinkResult},
Expand Down Expand Up @@ -118,6 +119,34 @@ impl InstallPixiEnvironmentSpec {
install_reporter: Option<Box<dyn rattler::install::Reporter>>,
) -> Result<InstallPixiEnvironmentResult, CommandDispatcherError<InstallPixiEnvironmentError>>
{
// Acquire a lock on the prefix to prevent concurrent installations
let guard = AsyncPrefixGuard::new(self.prefix.path())
.await
.map_err(|e| {
CommandDispatcherError::Failed(InstallPixiEnvironmentError::AcquireLock(
self.prefix.clone(),
e,
))
})?;

let mut write_guard = guard.write().await.map_err(|e| {
CommandDispatcherError::Failed(InstallPixiEnvironmentError::AcquireLock(
self.prefix.clone(),
e,
))
})?;

// Mark that we're beginning installation. We always call begin() because:
// 1. We have the exclusive write lock, so no one else is installing
// 2. If a previous process crashed, the state might be stale
// 3. The begin() method is idempotent if already in "Installing" state
write_guard.begin().await.map_err(|e| {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think it makes sense to check for is_ready state, and if it was already installed early out?

We are doing the same in create_exec_prefix, so I'm thinking maybe we can apply this pattern here?

CommandDispatcherError::Failed(InstallPixiEnvironmentError::UpdateLock(
self.prefix.clone(),
e,
))
})?;

// Split into source and binary records
let (source_records, mut binary_records): (Vec<_>, Vec<_>) =
std::mem::take(&mut self.records)
Expand Down Expand Up @@ -192,6 +221,14 @@ impl InstallPixiEnvironmentSpec {
.map_err(InstallPixiEnvironmentError::Installer)
.map_err(CommandDispatcherError::Failed)?;

// Mark the environment as ready
write_guard.finish().await.map_err(|e| {
CommandDispatcherError::Failed(InstallPixiEnvironmentError::UpdateLock(
self.prefix.clone(),
e,
))
})?;

Ok(InstallPixiEnvironmentResult {
transaction: result.transaction,
post_link_script_result: result.post_link_script_result,
Expand Down Expand Up @@ -261,4 +298,11 @@ pub enum InstallPixiEnvironmentError {
#[source]
SourceBuildError,
),

#[error("failed to acquire lock for prefix '{}'", .0.path().display())]
#[diagnostic(help("another process may be installing to the same environment"))]
AcquireLock(Prefix, #[source] std::io::Error),

#[error("failed to update lock for prefix '{}'", .0.path().display())]
UpdateLock(Prefix, #[source] std::io::Error),
}
Loading