cli: take the downloads and the control session off the old supervisor (SL3.10a)

nvx-rs/src/host/cli/download/archive.rs

@@ -19,7 +19,6 @@ use super::system_program;
 use crate::ErrorCode;
 use crate::host::artifacts::CHECKSUMS_NAME;
 use crate::host::cli::{Exit, Result, check_interrupt, io_error};
-use crate::host::supervisor::spawn_command;
 
 /// The directory beside the package into which `tar` extracts it.
 const EXTRACTION_DIR: &str = "package";
@@ -83,7 +82,8 @@ fn tar(directory: &Path, arguments: &[&str]) -> Result<String> {
         .stdin(Stdio::null())
         .stdout(Stdio::piped())
         .stderr(Stdio::piped());
-    let output = spawn_command(&mut command)
+    let output = command
+        .spawn()
         .and_then(Child::wait_with_output)
         .map_err(|error| {
             Exit::error(

nvx-rs/src/host/cli/download/archive/tests.rs

@@ -1,7 +1,13 @@
 //! The tests of packages: the names of their members, their extraction with the host's `tar`,
 //! and their checksums.
+//!
+//! Each test that runs `tar` holds the tests' spawn lock while it does: the CLI downloads before it
+//! launches a VM, but these tests run beside tests that launch stand-in VMs, and on Windows a `tar`
+//! that starts during a VM's start inherits that VM's handles, such as its log, which it holds
+//! until it exits.
 
 use super::*;
+use crate::host::tests::hold_spawns;
 
 /// The root directory of the packages of the tests.
 const ROOT: &str = "nvx-0.1.0-linux-kvm";
@@ -43,10 +49,11 @@ fn archive(source: &Path, archive: &Path) {
     } else {
         command.arg("-czf");
     }
-    let status = spawn_command(command.arg(archive).arg(ROOT).current_dir(source))
-        .unwrap()
-        .wait()
-        .unwrap();
+    let status =
+        crate::host::tests::spawn_locked(command.arg(archive).arg(ROOT).current_dir(source))
+            .unwrap()
+            .wait()
+            .unwrap();
     assert!(status.success());
 }
 
@@ -65,6 +72,7 @@ fn staged(directory: &Path, name: &str, change: impl FnOnce(&Path)) -> PathBuf {
 
 #[test]
 fn gzipped_packages_extract_once_their_names_and_checksums_pass() {
+    let _spawns = hold_spawns();
     let directory = tempfile::tempdir().unwrap();
     let archive = staged(directory.path(), "nvx-0.1.0-linux-kvm.tar.gz", |_| {});
     let package = extract(&archive).unwrap();
@@ -80,6 +88,7 @@ fn gzipped_packages_extract_once_their_names_and_checksums_pass() {
 #[cfg(windows)]
 #[test]
 fn zip_packages_extract_with_the_tar_of_windows() {
+    let _spawns = hold_spawns();
     let directory = tempfile::tempdir().unwrap();
     let archive = staged(directory.path(), "nvx-0.1.0-windows-whp.zip", |_| {});
     let package = extract(&archive).unwrap();
@@ -91,6 +100,7 @@ fn zip_packages_extract_with_the_tar_of_windows() {
 
 #[test]
 fn packages_whose_checksums_fail_are_refused() {
+    let _spawns = hold_spawns();
     for (change, expected) in [
         (
             (|root: &Path| fs::write(root.join("guest").join("vmlinux"), "other\n").unwrap())
@@ -120,6 +130,7 @@ fn packages_whose_checksums_fail_are_refused() {
 #[cfg(unix)]
 #[test]
 fn packages_with_symlinks_are_refused() {
+    let _spawns = hold_spawns();
     let directory = tempfile::tempdir().unwrap();
     let archive = staged(directory.path(), "nvx-0.1.0-linux-kvm.tar.gz", |root| {
         std::os::unix::fs::symlink("/etc/passwd", root.join("guest").join("link")).unwrap();
@@ -133,6 +144,7 @@ fn packages_with_symlinks_are_refused() {
 
 #[test]
 fn archives_that_tar_cannot_read_are_refused() {
+    let _spawns = hold_spawns();
     let directory = tempfile::tempdir().unwrap();
     let archive = directory.path().join("nvx-0.1.0-linux-kvm.tar.gz");
     fs::write(&archive, "not an archive").unwrap();

nvx-rs/src/host/cli/download/git.rs

@@ -10,7 +10,6 @@ use std::process::{Command, Output, Stdio};
 use super::History;
 use crate::ErrorCode;
 use crate::host::cli::{Exit, Result};
-use crate::host::supervisor::spawn_command;
 
 /// The `git` of a checkout.
 pub(super) struct Git {
@@ -43,7 +42,7 @@ impl Git {
         {
             command.env("GIT_CEILING_DIRECTORIES", ceiling);
         }
-        spawn_command(&mut command)?.wait_with_output()
+        command.spawn()?.wait_with_output()
     }
 }
 

nvx-rs/src/host/cli/download/github.rs

@@ -18,7 +18,6 @@ use serde_json::Value;
 use super::{Asset, Releases, remove_stale, system_program};
 use crate::ErrorCode;
 use crate::host::cli::{Exit, Result, check_interrupt, io_error};
-use crate::host::supervisor::spawn_command;
 
 /// The base URL of the GitHub REST API.
 const API: &str = "https://api.github.com";
@@ -323,7 +322,7 @@ impl Curl {
             } else {
                 Stdio::piped()
             });
-        let mut child = spawn_command(&mut command).map_err(|error| {
+        let mut child = command.spawn().map_err(|error| {
             Exit::error(
                 ErrorCode::Unavailable,
                 format!(

nvx-rs/src/host/cli/download/github/tests.rs

@@ -1,5 +1,10 @@
 //! The tests of the client of the GitHub REST API: its messages, its configuration of `curl`,
 //! and its requests, which a real `curl` sends to servers on the loopback interface.
+//!
+//! Each test that runs `curl` holds the tests' spawn lock while it does: the CLI downloads before
+//! it launches a VM, but these tests run beside tests that launch stand-in VMs, and on Windows a
+//! `curl` that starts during a VM's start inherits that VM's handles, such as its log, which it
+//! holds until it exits.
 
 use std::io::{BufRead, BufReader};
 use std::net::{TcpListener, TcpStream};
@@ -9,6 +14,7 @@ use std::thread;
 use serde_json::json;
 
 use super::*;
+use crate::host::tests::hold_spawns;
 
 impl Curl {
     /// Returns a `curl` that also speaks plain HTTP, to the servers of tests.
@@ -134,6 +140,7 @@ fn answer(
 
 #[test]
 fn queries_and_downloads_carry_the_token_to_the_origin_of_the_api_alone() {
+    let _spawns = hold_spawns();
     let (api_listener, api) = Server::bind();
     let (asset_listener, assets) = Server::bind();
     let asset_url = format!("{api}/repos/octo/nvx/releases/assets/7");
@@ -253,6 +260,7 @@ fn queries_and_downloads_carry_the_token_to_the_origin_of_the_api_alone() {
 
 #[test]
 fn requests_without_a_token_carry_none() {
+    let _spawns = hold_spawns();
     let (listener, api) = Server::bind();
     let server = Server::serve(listener, Vec::new());
     let scratch = tempfile::tempdir().unwrap();
@@ -275,6 +283,7 @@ fn requests_without_a_token_carry_none() {
 
 #[test]
 fn redirects_resolve_relative_locations_against_the_urls_of_their_requests() {
+    let _spawns = hold_spawns();
     let (listener, api) = Server::bind();
     let server = Server::serve(
         listener,
@@ -329,6 +338,7 @@ fn redirects_resolve_relative_locations_against_the_urls_of_their_requests() {
 
 #[test]
 fn the_last_response_to_a_retried_request_counts() {
+    let _spawns = hold_spawns();
     let (listener, api) = Server::bind();
     let path = "/repos/octo/nvx/releases/tags/v0.1.0-dev.a1b2c3d4e5f6";
     // The status, the header, and the message of the first response, which `curl` retries, would
@@ -399,6 +409,32 @@ fn requests_to_github_speak_https_alone() {
     );
 }
 
+#[test]
+fn a_missing_curl_reports_the_program_and_query_context() {
+    let scratch = tempfile::tempdir().unwrap();
+    let program = scratch.path().join("not-installed-curl");
+    let github = GitHub::at(
+        API,
+        "octo/nvx",
+        None,
+        scratch.path(),
+        Curl {
+            program: program.clone(),
+            ..Curl::new()
+        },
+    )
+    .unwrap();
+    let error = Exit::from(github.release("v1").unwrap_err());
+    assert_eq!(error.code(), Some(ErrorCode::Unavailable));
+    assert!(
+        error.message().unwrap().starts_with(&format!(
+            "GitHub release query needs curl, which did not run: {}: ",
+            program.display()
+        )),
+        "{error:?}"
+    );
+}
+
 #[test]
 fn configurations_quote_their_values_and_carry_the_token_when_asked() {
     let curl = Curl {

nvx-rs/src/host/cli/run.rs

@@ -1190,12 +1190,12 @@ fn foreground(
     if managed && output.console.is_some() {
         let endpoint = launched.outputs.endpoint.clone();
         let capability = *launched.capability.as_bytes();
-        let child = launched.child_mut().map_err(Exit::from_os_error)?;
+        let process = &launched.process;
         match attach_with_output(
             |cancellation| {
                 Session::connect_launched_cancellable(
                     &endpoint,
-                    child,
+                    process,
                     &capability,
                     60.0,
                     cancellation,

nvx-rs/src/host/cli/session.rs

@@ -37,10 +37,10 @@ use std::time::Duration;
 use super::deadline::{Deadline, RETRY_INTERVAL, sleep};
 use super::{Exit, Result, check_interrupt};
 use crate::host::platform::{self, Transport};
+use crate::host::process::Process;
 use crate::host::runtime::Runtime;
 use crate::host::sandbox::options::validate_exec_environment;
 use crate::host::session::{ControlSession, SessionError};
-use crate::host::supervisor::Launched;
 use crate::host::transport;
 use crate::host::vm::{self, SavedVm};
 use crate::protocol::{
@@ -335,7 +335,7 @@ impl Session {
         })
     }
 
-    /// Connects to `endpoint`, which `launched`, the OpenVMM process that a cold run launched,
+    /// Connects to `endpoint`, which `process`, the OpenVMM process that a cold run launched,
     /// serves, as [`Self::connect_unless_exited`] does, but stops waiting for the endpoint once
     /// OpenVMM has exited, as `nvx::host::session::connect` does: an OpenVMM that exits before it
     /// serves its endpoint, as one that refuses its configuration does, then ends the wait at
@@ -344,22 +344,18 @@ impl Session {
     /// `cancellation`.
     pub(super) fn connect_launched_cancellable(
         endpoint: &str,
-        launched: &mut Launched,
+        process: &Process,
         capability: &[u8; CAPABILITY_LEN],
         timeout: f64,
         cancellation: AttachmentCancellation,
     ) -> Result<Self> {
-        let pid = launched.id();
+        let pid = process.reference().pid;
         Self::connect_unless_exited(
             endpoint,
             pid,
             capability,
             timeout,
-            || {
-                launched
-                    .wait_timeout(Duration::ZERO)
-                    .is_ok_and(|status| status.is_some())
-            },
+            || process.has_exited().is_ok_and(|exited| exited),
             Some(cancellation),
         )
     }
@@ -686,7 +682,7 @@ impl Session {
             match kind {
                 APP_STDOUT | APP_STDERR => {
                     // The guest agent forwards at most this much output, so more breaks the
-                    // protocol, as `nvx::host::instance` also finds.
+                    // protocol.
                     if stdout.len() + stderr.len() + response.len() > MAX_OUTPUT_BYTES {
                         return Err(violation(&format!(
                             "managed guest exceeded the {MAX_OUTPUT_BYTES}-byte output limit"

nvx-rs/src/host/cli/session/tests.rs

@@ -827,6 +827,89 @@ fn check_waits_for_openvmm_that_exits(endpoint: &str) {
     assert_ne!(checks, 0);
 }
 
+/// A process that runs for a minute unless it is killed, which dropping it kills and reaps.
+#[cfg(any(target_os = "linux", windows))]
+struct Sleeper(std::process::Child);
+
+#[cfg(any(target_os = "linux", windows))]
+impl Sleeper {
+    /// Spawns the sleeper under a `SpawnLock`, so that on Windows it holds no handle of a VMM that
+    /// another test spawns meanwhile.
+    fn spawn() -> Self {
+        let mut command = if cfg!(windows) {
+            let mut command = std::process::Command::new("ping");
+            command.args(["-n", "60", "127.0.0.1"]);
+            command
+        } else {
+            let mut command = std::process::Command::new("sleep");
+            command.arg("60");
+            command
+        };
+        command
+            .stdin(std::process::Stdio::null())
+            .stdout(std::process::Stdio::null());
+        Self(crate::host::tests::spawn_locked(&mut command).unwrap())
+    }
+
+    /// Returns the verified process of the sleeper.
+    fn process(&self) -> Process {
+        use crate::host::process::{BootId, Opened, ProcessRef};
+
+        let reference = ProcessRef {
+            pid: self.0.id(),
+            start_time: platform::process_start_time(self.0.id()).unwrap().unwrap(),
+            boot: BootId::current().unwrap(),
+        };
+        let Opened::Running(process) = Process::open(&reference).unwrap() else {
+            panic!("the sleeper runs");
+        };
+        process
+    }
+}
+
+#[cfg(any(target_os = "linux", windows))]
+impl Drop for Sleeper {
+    fn drop(&mut self) {
+        let _ = self.0.kill();
+        let _ = self.0.wait();
+    }
+}
+
+/// Checks the wait for `endpoint`, which never appears, of a cold run whose OpenVMM is the
+/// verified process that [`Session::connect_launched_cancellable`] receives: the wait goes on
+/// while the process runs, and ends at once, as a closed endpoint, once it exits, rather than at
+/// the timeout.
+#[cfg(any(target_os = "linux", windows))]
+fn check_launched_waits_end_once_their_process_exits(endpoint: &str) {
+    let mut sleeper = Sleeper::spawn();
+    let process = sleeper.process();
+    let started = Instant::now();
+    let failure = thread::scope(|scope| {
+        let waiting = scope.spawn(|| {
+            Session::connect_launched_cancellable(
+                endpoint,
+                &process,
+                &CAPABILITY,
+                60.0,
+                AttachmentCancellation::default(),
+            )
+            .err()
+        });
+        thread::sleep(Duration::from_millis(500));
+        let ended_early = waiting.is_finished();
+        sleeper.0.kill().unwrap();
+        sleeper.0.wait().unwrap();
+        assert!(!ended_early, "the wait ended while OpenVMM ran");
+        waiting.join().unwrap().unwrap()
+    });
+    assert!(
+        started.elapsed() < Duration::from_secs(30),
+        "{:?}",
+        started.elapsed()
+    );
+    assert!(matches!(failure, Exit::EndpointClosed), "{failure:?}");
+}
+
 #[cfg(any(target_os = "linux", windows))]
 fn check_forward_failure_during_connect(endpoint: &str) {
     let (attempted, attempt) = mpsc::sync_channel(1);
@@ -1211,6 +1294,14 @@ mod endpoints {
         check_waits_for_openvmm_that_exits(path.to_str().unwrap());
     }
 
+    #[test]
+    #[cfg(target_os = "linux")]
+    fn launched_waits_end_once_their_process_exits() {
+        let directory = tempfile::tempdir().unwrap();
+        let path = directory.path().join("control.sock");
+        check_launched_waits_end_once_their_process_exits(path.to_str().unwrap());
+    }
+
     #[test]
     #[cfg(target_os = "linux")]
     fn forward_failure_cancels_the_connection_before_its_timeout() {
@@ -1446,6 +1537,13 @@ mod endpoints {
         check_waits_for_openvmm_that_exits(r"\\.\pipe\nvx-cli-session-test-exited");
     }
 
+    #[test]
+    fn launched_waits_end_once_their_process_exits() {
+        check_launched_waits_end_once_their_process_exits(
+            r"\\.\pipe\nvx-cli-session-test-launched-exited",
+        );
+    }
+
     #[test]
     fn forward_failure_cancels_the_connection_before_its_timeout() {
         check_forward_failure_during_connect(r"\\.\pipe\nvx-cli-session-test-cancelled");