From 1ad36e0dde15f82c738804019708c1be37dda3b8 Mon Sep 17 00:00:00 2001 From: Pedro Henrique Penna Date: Tue, 10 Nov 2026 14:14:15 -0800 Subject: [PATCH] tests: never reuse an adapter sandbox's name within a test process The Windows/WHP runtime adapter job of run 42896649941 failed before busy_root_fails_the_teardown started: Sandbox::new could not create its sandbox, ra-2012-3, "Access is denied. (os error 5)". Since 07d16d0150, which dropped the scenario's name, Sandbox::new names each sandbox ra-PID-N after the first N that no directory takes, so each test of a binary takes 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 so failed 38 of 10,044 creations. 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 30 bytes short of the 55 that its sockets allow. 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 | 12 ++++++---- tests/runtime-adapter/tests/sandbox_names.rs | 23 ++++++++++++++++++++ 3 files changed, 36 insertions(+), 4 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 d700bda5f..5faee57f2 100644 --- a/tests/runtime-adapter/README.md +++ b/tests/runtime-adapter/README.md @@ -2564,6 +2564,11 @@ greeting and answer of the VM tests' listener client on Linux and Windows. On Linux it also checks that a self-connection leaves the guest's publication port bindable and that a connection reset before inspection is retried. +`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 fef951c26..be44c046f 100644 --- a/tests/runtime-adapter/tests/common/mod.rs +++ b/tests/runtime-adapter/tests/common/mod.rs @@ -21,10 +21,11 @@ use std::error::Error; use std::ffi::{OsStr, OsString}; use std::fs::{self, File}; use std::io::{self, Read, Write}; +use std::iter; use std::net::{Shutdown, SocketAddr, TcpStream}; use std::path::{Path, PathBuf}; use std::process::{Child, Command, Stdio}; -use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; use std::thread; use std::time::{Duration, Instant}; @@ -45,6 +46,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; @@ -150,11 +153,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!("ra-{pid}-{attempt}"))) + let root = iter::repeat_with(|| NEXT_SANDBOX.fetch_add(1, Ordering::Relaxed)) + .map(|number| env::temp_dir().join(format!("ra-{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); +}