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.
This commit is contained in:
@@ -162,7 +162,13 @@ impl ContextManager {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/// Set current context, without modifying configuration
|
/// Set current context, without modifying configuration
|
||||||
pub fn set_current_ephemeral(&self, context: Context) {
|
///
|
||||||
|
/// Accepts either an owned [`Context`] or an already-shared
|
||||||
|
/// `Arc<Context>`: 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<Arc<Context>>) {
|
||||||
*self.context.write().unwrap() = context.into();
|
*self.context.write().unwrap() = context.into();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
+55
-17
@@ -283,7 +283,15 @@ fn unescape_mount_field(field: &str) -> String {
|
|||||||
/// An ephemeral unshare context guard that creates and manages a temporary chroot environment
|
/// An ephemeral unshare context guard that creates and manages a temporary chroot environment
|
||||||
/// for building packages with unshare permissions.
|
/// for building packages with unshare permissions.
|
||||||
pub struct EphemeralContextGuard {
|
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<Context>,
|
||||||
|
/// 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<Context>,
|
||||||
chroot_path: PathBuf,
|
chroot_path: PathBuf,
|
||||||
build_succeeded: bool,
|
build_succeeded: bool,
|
||||||
base_ctx: Arc<Context>,
|
base_ctx: Arc<Context>,
|
||||||
@@ -306,7 +314,11 @@ impl EphemeralContextGuard {
|
|||||||
base_ctx: Arc<Context>,
|
base_ctx: Arc<Context>,
|
||||||
ui: Option<Arc<DebUi>>,
|
ui: Option<Arc<DebUi>>,
|
||||||
) -> Result<Self, Box<dyn Error>> {
|
) -> Result<Self, Box<dyn Error>> {
|
||||||
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
|
// Create a temporary directory for the chroot
|
||||||
let chroot_path_str = base_ctx.create_temp_dir()?;
|
let chroot_path_str = base_ctx.create_temp_dir()?;
|
||||||
@@ -353,14 +365,27 @@ impl EphemeralContextGuard {
|
|||||||
return Err(e);
|
return Err(e);
|
||||||
}
|
}
|
||||||
|
|
||||||
// Switch to an ephemeral context to build the package in the chroot
|
// Switch to an ephemeral context to build the package in the chroot.
|
||||||
context::manager().set_current_ephemeral(Context::new(ContextConfig::Unshare {
|
// The parent is the base context itself (the one that bootstrapped
|
||||||
path: chroot_path.to_string_lossy().to_string(),
|
// the chroot), wired through `with_parent` instead of a config-name
|
||||||
parent: Some(current_context_name.clone()),
|
// 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 {
|
Ok(Self {
|
||||||
previous_context: current_context_name,
|
previous_context,
|
||||||
|
ephemeral_ctx,
|
||||||
chroot_path,
|
chroot_path,
|
||||||
build_succeeded: false,
|
build_succeeded: false,
|
||||||
base_ctx,
|
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<Context> {
|
||||||
|
Arc::clone(&self.ephemeral_ctx)
|
||||||
|
}
|
||||||
|
|
||||||
async fn download_and_extract_chroot(
|
async fn download_and_extract_chroot(
|
||||||
series: &str,
|
series: &str,
|
||||||
arch: Option<&str>,
|
arch: Option<&str>,
|
||||||
@@ -687,19 +722,22 @@ impl Drop for EphemeralContextGuard {
|
|||||||
|
|
||||||
log::debug!("Cleaning up ephemeral context ({:?})...", self.chroot_path);
|
log::debug!("Cleaning up ephemeral context ({:?})...", self.chroot_path);
|
||||||
|
|
||||||
// Clean up any overlay mounts before resetting the context.
|
// Clean up any overlay mounts before resetting the context. This
|
||||||
// This must happen while the ephemeral context is still current so its
|
// explicitly targets the context this guard created — never
|
||||||
// driver is accessible. The actual unmount commands run via the parent
|
// `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.
|
// (base) context, so they work regardless.
|
||||||
let ephemeral_ctx = context::current();
|
if let Err(e) = self.ephemeral_ctx.cleanup() {
|
||||||
if let Err(e) = ephemeral_ctx.cleanup() {
|
|
||||||
log::warn!("Failed to clean up overlay mounts: {}", e);
|
log::warn!("Failed to clean up overlay mounts: {}", e);
|
||||||
}
|
}
|
||||||
|
|
||||||
// Reset to normal context
|
// Restore the context that was current when this guard was created,
|
||||||
if let Err(e) = context::manager().set_current(&self.previous_context) {
|
// not whatever is globally current at drop time (another concurrent
|
||||||
log::error!("Failed to restore context {}: {}", self.previous_context, e);
|
// 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
|
// Remove chroot directory only if build succeeded
|
||||||
if self.build_succeeded {
|
if self.build_succeeded {
|
||||||
|
|||||||
+10
-7
@@ -134,14 +134,17 @@ async fn build_binary_package_impl(
|
|||||||
None
|
None
|
||||||
};
|
};
|
||||||
|
|
||||||
let result = async {
|
// Determine the build context explicitly: for Local builds it is the
|
||||||
// Get the build context - either the ephemeral context or the base context
|
// ephemeral context the guard just created (taken from the guard itself,
|
||||||
let build_ctx = if mode == BuildMode::Local {
|
// never from the process-global, which concurrent builds may have
|
||||||
context::current()
|
// re-pointed at their own chroot); otherwise the base context is used
|
||||||
} else {
|
// directly.
|
||||||
base_ctx.clone()
|
let build_ctx = match guard.as_ref() {
|
||||||
};
|
Some(g) => g.context(),
|
||||||
|
None => base_ctx.clone(),
|
||||||
|
};
|
||||||
|
|
||||||
|
let result = async {
|
||||||
// Prepare build directory
|
// Prepare build directory
|
||||||
let build_root = build_ctx.create_temp_dir()?;
|
let build_root = build_ctx.create_temp_dir()?;
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user