From 592e98c1e9d3af0b9f852a4c8a6f1ade9ca28484 Mon Sep 17 00:00:00 2001 From: Valentin Haudiquet Date: Wed, 16 Sep 2026 02:05:40 +0200 Subject: [PATCH] deb: pass the build context explicitly instead of swapping the global build_binary_package installed its ephemeral chroot context into the process-global manager and read it back with context::current(), ignoring its ctx parameter: two concurrent builds would re-point each other's global and each drop would clean up whichever chroot was current at the time. The guard now keeps the Arc of the context it created (parented directly on the base context, not on a config-name lookup), exposes it via context(), and Drop cleans up exactly that context and restores the exact handle that was current at creation, so overlapping builds no longer cross-destroy each other. --- src/context/manager.rs | 8 ++++- src/deb/ephemeral.rs | 72 ++++++++++++++++++++++++++++++++---------- src/deb/mod.rs | 17 ++++++---- 3 files changed, 72 insertions(+), 25 deletions(-) diff --git a/src/context/manager.rs b/src/context/manager.rs index 485fb44..a8df511 100644 --- a/src/context/manager.rs +++ b/src/context/manager.rs @@ -162,7 +162,13 @@ impl ContextManager { } /// Set current context, without modifying configuration - pub fn set_current_ephemeral(&self, context: Context) { + /// + /// Accepts either an owned [`Context`] or an already-shared + /// `Arc`: callers that keep their own handle to the context + /// they install (e.g. [`crate::deb::ephemeral::EphemeralContextGuard`]) + /// pass the Arc so they can restore exactly this context afterwards + /// instead of relying on whatever happens to be current at that time. + pub fn set_current_ephemeral(&self, context: impl Into>) { *self.context.write().unwrap() = context.into(); } diff --git a/src/deb/ephemeral.rs b/src/deb/ephemeral.rs index 9a6ddf1..e701426 100644 --- a/src/deb/ephemeral.rs +++ b/src/deb/ephemeral.rs @@ -283,7 +283,15 @@ fn unescape_mount_field(field: &str) -> String { /// An ephemeral unshare context guard that creates and manages a temporary chroot environment /// for building packages with unshare permissions. pub struct EphemeralContextGuard { - previous_context: String, + /// The ephemeral build context this guard created (an unshare context + /// bound to the chroot, parented on the base context). Held explicitly so + /// cleanup and the build itself never depend on the process-global + /// "current" context, which concurrent builds swap for their own. + ephemeral_ctx: Arc, + /// The context that was current (globally) when this guard was created, + /// restored on drop. Saving the handle instead of a config name is what + /// keeps concurrent builds from restoring over each other. + previous_context: Arc, chroot_path: PathBuf, build_succeeded: bool, base_ctx: Arc, @@ -306,7 +314,11 @@ impl EphemeralContextGuard { base_ctx: Arc, ui: Option>, ) -> Result> { - let current_context_name = context::manager().current_name(); + // Save the globally-installed context so Drop can restore exactly + // this handle: concurrent builds install their own ephemeral + // overrides, so the only safe restoration value is the one observed + // before this guard swapped anything in. + let previous_context = context::current(); // Create a temporary directory for the chroot let chroot_path_str = base_ctx.create_temp_dir()?; @@ -353,14 +365,27 @@ impl EphemeralContextGuard { return Err(e); } - // Switch to an ephemeral context to build the package in the chroot - context::manager().set_current_ephemeral(Context::new(ContextConfig::Unshare { - path: chroot_path.to_string_lossy().to_string(), - parent: Some(current_context_name.clone()), - })); + // Switch to an ephemeral context to build the package in the chroot. + // The parent is the base context itself (the one that bootstrapped + // the chroot), wired through `with_parent` instead of a config-name + // lookup, so an explicit non-current base (e.g. ssh) is used for + // everything that runs inside the chroot. The Arc stays in the + // guard: the build and the cleanup use it directly. + let ephemeral_ctx = Arc::new(Context::with_parent( + ContextConfig::Unshare { + path: chroot_path.to_string_lossy().to_string(), + // The real parent is bound below via `with_parent`; the + // config field is only used for contexts read from the + // persisted configuration. + parent: None, + }, + base_ctx.clone(), + )); + context::manager().set_current_ephemeral(ephemeral_ctx.clone()); Ok(Self { - previous_context: current_context_name, + previous_context, + ephemeral_ctx, chroot_path, build_succeeded: false, base_ctx, @@ -368,6 +393,16 @@ impl EphemeralContextGuard { }) } + /// The ephemeral build context created by this guard + /// + /// Callers must take the context from here rather than from + /// [`crate::context::current()`]: the process-global is a shared swap + /// slot that another concurrent build may have re-pointed at its own + /// chroot, while this handle is guaranteed to be this guard's context. + pub fn context(&self) -> Arc { + Arc::clone(&self.ephemeral_ctx) + } + async fn download_and_extract_chroot( series: &str, arch: Option<&str>, @@ -687,19 +722,22 @@ impl Drop for EphemeralContextGuard { log::debug!("Cleaning up ephemeral context ({:?})...", self.chroot_path); - // Clean up any overlay mounts before resetting the context. - // This must happen while the ephemeral context is still current so its - // driver is accessible. The actual unmount commands run via the parent + // Clean up any overlay mounts before resetting the context. This + // explicitly targets the context this guard created — never + // `context::current()`, which a concurrent build may have re-pointed + // at its own chroot. The actual unmount commands run via the parent // (base) context, so they work regardless. - let ephemeral_ctx = context::current(); - if let Err(e) = ephemeral_ctx.cleanup() { + if let Err(e) = self.ephemeral_ctx.cleanup() { log::warn!("Failed to clean up overlay mounts: {}", e); } - // Reset to normal context - if let Err(e) = context::manager().set_current(&self.previous_context) { - log::error!("Failed to restore context {}: {}", self.previous_context, e); - } + // Restore the context that was current when this guard was created, + // not whatever is globally current at drop time (another concurrent + // build's override may be installed there). This only swaps the + // in-memory handle: the persisted configuration still names the + // context selected by the user, as `set_current_ephemeral` never + // touches it. + context::manager().set_current_ephemeral(self.previous_context.clone()); // Remove chroot directory only if build succeeded if self.build_succeeded { diff --git a/src/deb/mod.rs b/src/deb/mod.rs index 38b0311..2ff25ac 100644 --- a/src/deb/mod.rs +++ b/src/deb/mod.rs @@ -134,14 +134,17 @@ async fn build_binary_package_impl( None }; - let result = async { - // Get the build context - either the ephemeral context or the base context - let build_ctx = if mode == BuildMode::Local { - context::current() - } else { - base_ctx.clone() - }; + // Determine the build context explicitly: for Local builds it is the + // ephemeral context the guard just created (taken from the guard itself, + // never from the process-global, which concurrent builds may have + // re-pointed at their own chroot); otherwise the base context is used + // directly. + let build_ctx = match guard.as_ref() { + Some(g) => g.context(), + None => base_ctx.clone(), + }; + let result = async { // Prepare build directory let build_root = build_ctx.create_temp_dir()?;