From d5abc5f9d3c2662bf67dd31985ddeb166458afb5 Mon Sep 17 00:00:00 2001 From: Pedro Henrique Penna Date: Tue, 10 Nov 2026 14:19:50 -0800 Subject: [PATCH] tests: never reuse an adapter sandbox's name within a test process Sandbox::new names each sandbox runtime-adapter-NAME-PID-N after the first N that no directory takes, so tests that share a scenario name, such as the six of vm_clone that start clones named cl-r, take the names that the others free. Windows keeps the name of a directory that remove_dir_all deletes until the deletion completes, and until then refuses to create a directory under it with ERROR_ACCESS_DENIED, not ERROR_ALREADY_EXISTS, which Sandbox::new took for a failure: a test that took a name while another was removing the sandbox of that name panicked. On Windows, eight threads that create and remove sandboxes of one name failed 55 of 12,470 creations so. On feature/nvx-refactor, whose sandbox names leave out the scenario's name, the race failed the Windows/WHP runtime adapter job of run 42896649941. Number the sandboxes of a process from a counter instead, so that a process never takes the name of a sandbox that it created, and skips only the names that another process left. On Linux, the counter adds at most two digits, and with a seven-digit PID the longest template store below /tmp stays 6 bytes short of the 55 that its sockets allow, one byte less than before. The new sandbox_names test, which boots nothing, failed before the change, with the second sandbox at the name of the first. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- tests/runtime-adapter/README.md | 5 +++++ tests/runtime-adapter/tests/common/mod.rs | 11 +++++++--- tests/runtime-adapter/tests/sandbox_names.rs | 23 ++++++++++++++++++++ 3 files changed, 36 insertions(+), 3 deletions(-) create mode 100644 tests/runtime-adapter/tests/sandbox_names.rs diff --git a/tests/runtime-adapter/README.md b/tests/runtime-adapter/README.md index 4cd71bcf9..17a25768e 100644 --- a/tests/runtime-adapter/README.md +++ b/tests/runtime-adapter/README.md @@ -2656,6 +2656,11 @@ configuration, on Linux and Windows: in the initramfs too, and missing install metadata, which fails as NVX reports it. +`tests/sandbox_names.rs` checks, in the host-only configuration, that a test +process never gives a VM test's sandbox the name of one that it dropped, since +Windows refuses to create a directory under the name of one that another test +is still removing. + `tests/staged_images.rs` checks, in the host-only configuration, on Linux and Windows: diff --git a/tests/runtime-adapter/tests/common/mod.rs b/tests/runtime-adapter/tests/common/mod.rs index f27089851..d35782fdd 100644 --- a/tests/runtime-adapter/tests/common/mod.rs +++ b/tests/runtime-adapter/tests/common/mod.rs @@ -21,8 +21,10 @@ use std::error::Error; use std::ffi::{OsStr, OsString}; use std::fs::{self, File}; use std::io; +use std::iter; use std::path::{Path, PathBuf}; use std::process::{Child, Command, Stdio}; +use std::sync::atomic::{AtomicUsize, Ordering}; use std::thread; use std::time::{Duration, Instant}; @@ -43,6 +45,8 @@ pub(crate) const SCRATCH_TEMPLATE: &str = "ubuntu-smoke-scratch.ext4"; const COMMAND_TIMEOUT: Duration = Duration::from_secs(180); /// How much of the end of the VMM's log a failure shows. const LOG_TAIL: usize = 8192; +/// The number of the next sandbox of the test process. +static NEXT_SANDBOX: AtomicUsize = AtomicUsize::new(0); /// The `KEY=VALUE` lines that a command of the host program printed. pub(crate) type Printed = BTreeMap; @@ -148,11 +152,12 @@ impl Drop for Started { impl Sandbox { /// Creates the directory of the test `name`, in the system's temporary directory, so that the /// path of a Linux control socket in it stays short. A directory that an earlier test process - /// kept stays as it is. + /// kept stays as it is. Each sandbox of the process takes a name of its own, since Windows + /// refuses to create a directory under the name of one that another test is still removing. pub(crate) fn new(name: &str) -> Self { let pid = std::process::id(); - let root = (0..) - .map(|attempt| env::temp_dir().join(format!("runtime-adapter-{name}-{pid}-{attempt}"))) + let root = iter::repeat_with(|| NEXT_SANDBOX.fetch_add(1, Ordering::Relaxed)) + .map(|number| env::temp_dir().join(format!("runtime-adapter-{name}-{pid}-{number}"))) .find(|root| match fs::create_dir(root) { Ok(()) => true, Err(error) if error.kind() == io::ErrorKind::AlreadyExists => false, diff --git a/tests/runtime-adapter/tests/sandbox_names.rs b/tests/runtime-adapter/tests/sandbox_names.rs new file mode 100644 index 000000000..d9af9a1c9 --- /dev/null +++ b/tests/runtime-adapter/tests/sandbox_names.rs @@ -0,0 +1,23 @@ +//! Regressions for the names that the VM tests give their sandboxes. +#![cfg(feature = "host")] + +mod common; + +use common::Sandbox; + +/// Windows refuses to create a directory under the name of one that another test of the process is +/// still removing, so a sandbox never takes the name of one that the process dropped, even for a +/// test of the same name. +#[test] +fn a_process_never_reuses_the_name_of_a_dropped_sandbox() { + let dropped = Sandbox::new("names"); + let name = dropped.root.clone(); + drop(dropped); + assert!( + !name.exists(), + "the dropped sandbox kept {}", + name.display() + ); + let next = Sandbox::new("names"); + assert_ne!(next.root, name); +}