diff --git a/e2e/rust/src/harness/sandbox.rs b/e2e/rust/src/harness/sandbox.rs index 0aeb25038c..d9b274a20f 100644 --- a/e2e/rust/src/harness/sandbox.rs +++ b/e2e/rust/src/harness/sandbox.rs @@ -7,10 +7,11 @@ //! is dropped, replacing the `trap cleanup EXIT` pattern from the bash tests. use std::process::Stdio; -use std::sync::{Arc, Mutex}; +use std::sync::{Arc, Mutex, OnceLock}; use std::time::Duration; use tokio::io::{AsyncBufReadExt, BufReader}; +use tokio::sync::{OwnedRwLockReadGuard, OwnedRwLockWriteGuard, RwLock}; use tokio::time::timeout; use super::binary::openshell_cmd; @@ -31,6 +32,57 @@ fn extract_sandbox_name(output: &str) -> Option { /// startup. const SANDBOX_READY_TIMEOUT: Duration = Duration::from_secs(600); +/// Upper bound for best-effort sandbox deletion during test teardown. +const SANDBOX_CLEANUP_TIMEOUT: Duration = Duration::from_secs(60); + +/// Coordinates sandbox creation and cleanup in the Podman E2E lane. +/// +/// A standalone Podman reproducer confirmed that removing a container with an +/// image volume can race with another container attaching the same image. The +/// E2E suite can trigger that Podman issue when automatic cleanup from one test +/// overlaps sandbox creation in another test. +/// +/// Creates take a shared lock so concurrent-create coverage is preserved. +/// Cleanup takes an exclusive lock so Podman image-volume detach never overlaps +/// attach in this test harness. This is intentionally not a production-driver +/// lock: serializing the driver would reduce supported lifecycle concurrency. +static PODMAN_E2E_LIFECYCLE_GATE: OnceLock>> = OnceLock::new(); + +fn is_podman_e2e() -> bool { + std::env::var("OPENSHELL_E2E_DRIVER").as_deref() == Ok("podman") +} + +async fn create_guard() -> Option> { + if !is_podman_e2e() { + return None; + } + Some( + Arc::clone(PODMAN_E2E_LIFECYCLE_GATE.get_or_init(|| Arc::new(RwLock::new(())))) + .read_owned() + .await, + ) +} + +async fn cleanup_guard() -> Option> { + if !is_podman_e2e() { + return None; + } + Some( + Arc::clone(PODMAN_E2E_LIFECYCLE_GATE.get_or_init(|| Arc::new(RwLock::new(())))) + .write_owned() + .await, + ) +} + +async fn delete_sandbox(name: &str) { + let _lifecycle_guard = cleanup_guard().await; + let mut cmd = openshell_cmd(); + cmd.arg("sandbox").arg("delete").arg(name); + cmd.stdout(Stdio::null()).stderr(Stdio::null()); + + let _ = timeout(SANDBOX_CLEANUP_TIMEOUT, cmd.status()).await; +} + /// RAII guard that deletes a sandbox on drop. /// /// For sandboxes created with `--keep` (long-running background command), the @@ -67,6 +119,7 @@ impl SandboxGuard { /// Returns an error if the CLI exits with a non-zero status or the sandbox /// name cannot be parsed from the output. pub async fn create(args: &[&str]) -> Result { + let _lifecycle_guard = create_guard().await; let mut cmd = openshell_cmd(); cmd.arg("sandbox").arg("create"); for arg in args { @@ -139,6 +192,7 @@ impl SandboxGuard { command: &[&str], ready_marker: &str, ) -> Result { + let _lifecycle_guard = create_guard().await; let mut cmd = openshell_cmd(); cmd.arg("sandbox").arg("create").arg("--keep"); for arg in create_args { @@ -264,6 +318,7 @@ impl SandboxGuard { uploads: &[(&str, &str)], command: &[&str], ) -> Result { + let _lifecycle_guard = create_guard().await; let mut cmd = openshell_cmd(); cmd.arg("sandbox").arg("create"); for (local, dest) in uploads { @@ -511,12 +566,7 @@ impl SandboxGuard { let _ = child.wait().await; } - // Delete the sandbox. - let mut cmd = openshell_cmd(); - cmd.arg("sandbox").arg("delete").arg(&self.name); - cmd.stdout(Stdio::null()).stderr(Stdio::null()); - - let _ = cmd.status().await; + delete_sandbox(&self.name).await; } } @@ -526,14 +576,15 @@ impl Drop for SandboxGuard { return; } - // We need to run async cleanup in a sync Drop. Use block_in_place to - // avoid blocking the tokio runtime. This is acceptable for test code. let name = self.name.clone(); let mut child = self.child.take(); - // Attempt cleanup with a new runtime if we're not inside one, or - // block_in_place if we are. - std::thread::spawn(move || { + // Cleanup uses a separate runtime because Drop cannot await. Join the + // thread so teardown completes before the guard disappears; detaching + // it allowed cleanup to leak into a later test or be terminated when + // the test process exited. + let cleanup_name = name.clone(); + let cleanup = std::thread::spawn(move || { let rt = tokio::runtime::Runtime::new().expect("create cleanup runtime"); rt.block_on(async { if let Some(ref mut child) = child { @@ -541,11 +592,11 @@ impl Drop for SandboxGuard { let _ = child.wait().await; } - let mut cmd = openshell_cmd(); - cmd.arg("sandbox").arg("delete").arg(&name); - cmd.stdout(Stdio::null()).stderr(Stdio::null()); - let _ = cmd.status().await; + delete_sandbox(&name).await; }); }); + if cleanup.join().is_err() { + eprintln!("sandbox cleanup thread panicked for {cleanup_name}"); + } } }