[SL3.3b] Recover interrupted CLI launches under the sandbox lock

nvx-rs/src/host/cli/store/launch.rs

@@ -1,13 +1,17 @@
-//! Locked launch transitions, without spawning, recovery, or process termination.
+//! Locked launch transitions and bounded recovery of an interrupted launch.
+
+use std::thread;
+use std::time::{Duration, Instant};
 
 use super::StateFile;
 use super::files;
 use super::locked::LockedState;
 use super::record::{LaunchJournal, RuntimeRecord};
-use crate::host::process::ProcessRef;
+use crate::host::cli::deadline::RETRY_INTERVAL;
+use crate::host::process::{BootId, KILL_TIMEOUT, Opened, Process, ProcessRef};
 use crate::host::runtime::RuntimeIdentity;
 use crate::host::vm::{Prepared, SavedVm};
-use crate::{Error, Result};
+use crate::{Error, ErrorKind, Result};
 
 /// A launch whose lock stays borrowed until commit or until its journal is left behind.
 #[derive(Debug)]
@@ -17,6 +21,77 @@ pub(crate) struct Launch<'a> {
 }
 
 impl LockedState {
+    /// Resolves only an uncommitted journal, before the next command can mutate state.
+    pub(crate) fn recover(&mut self, deadline: Instant) -> Result<()> {
+        if self.runtime()?.is_some() {
+            return Ok(());
+        }
+        let Some(journal) = self.journal()? else {
+            return Ok(());
+        };
+        if journal.vm_dir != self.dir() || journal.log != self.path(StateFile::Log) {
+            return Err(Error::io(
+                "the launch journal belongs to another sandbox directory",
+            ));
+        }
+        match journal.process() {
+            Some(reference) => self.end_recorded(&journal, reference, deadline)?,
+            None if !self.released_claim(&journal, deadline)? => {
+                let process = self.endpoint_owner(&journal, deadline)?;
+                end_process(&process, deadline)?;
+            }
+            None => {}
+        }
+        if self.capability()?.is_some() {
+            files::remove(self, StateFile::Capability)?;
+        }
+        // Keep the journal until cleanup has finished, including after a partial removal.
+        files::remove(self, StateFile::Journal)
+    }
+
+    /// Ends only the recorded process, or accepts an independently proved release of its claim.
+    fn end_recorded(
+        &self,
+        journal: &LaunchJournal,
+        reference: &ProcessRef,
+        deadline: Instant,
+    ) -> Result<()> {
+        let boot = BootId::current().map_err(|error| {
+            Error::io("cannot identify the current host boot").with_source(error)
+        })?;
+        if boot != reference.boot {
+            return Ok(());
+        }
+        let unheld = match Process::open(reference) {
+            Ok(Opened::Running(process)) => return end_process(&process, deadline),
+            Ok(Opened::Exited(_)) => Error::new(
+                ErrorKind::StillRunning,
+                "the recorded VM exited, but a process may still hold its launch log",
+            ),
+            Err(error) => Error::io("cannot verify the recorded VM process").with_source(error),
+        };
+        if self.released_claim(journal, deadline)? {
+            Ok(())
+        } else {
+            Err(unheld)
+        }
+    }
+
+    /// Rechecks transient fork-inherited descriptors for at most one second.
+    fn released_claim(&self, journal: &LaunchJournal, deadline: Instant) -> Result<bool> {
+        let until = deadline.min(Instant::now() + Duration::from_secs(1));
+        loop {
+            if !self.claim_held(journal)? {
+                return Ok(true);
+            }
+            let left = until.saturating_duration_since(Instant::now());
+            if left.is_zero() {
+                return Ok(false);
+            }
+            thread::sleep(left.min(RETRY_INTERVAL));
+        }
+    }
+
     /// Durably publishes the capability and complete prepared journal before the caller spawns.
     pub(crate) fn begin_launch(
         &mut self,
@@ -38,6 +113,48 @@ impl LockedState {
     }
 }
 
+/// Ends a held, verified process and requires confirmation of its exit before the deadline.
+fn end_process(process: &Process, deadline: Instant) -> Result<()> {
+    let failed = |message: &str| {
+        Error::new(
+            ErrorKind::StillRunning,
+            format!(
+                "cannot confirm exit of VM process {}: {message}",
+                process.reference().pid
+            ),
+        )
+    };
+    if process
+        .wait(Instant::now())
+        .map_err(|error| failed("the exit check failed").with_source(error))?
+        .is_some()
+    {
+        return Ok(());
+    }
+    let until = deadline.min(Instant::now() + KILL_TIMEOUT);
+    if until <= Instant::now() {
+        return Err(failed("the recovery deadline passed"));
+    }
+    let kill_error = process.kill().err();
+    let exited = process.wait(until).map_err(|error| {
+        let mut failure = failed("the exit wait failed");
+        if let Some(kill_error) = &kill_error {
+            failure = failed(&format!(
+                "termination failed: {kill_error}; the exit wait failed"
+            ));
+        }
+        failure.with_source(error)
+    })?;
+    if exited.is_some() {
+        return Ok(());
+    }
+    let failure = failed("the verified process has not exited");
+    Err(match kill_error {
+        Some(error) => failure.with_source(error),
+        None => failure,
+    })
+}
+
 impl Launch<'_> {
     /// Records only the caller's spawned process, durably, before changing the in-memory state.
     pub(crate) fn record_process(&mut self, process: &ProcessRef) -> Result<()> {

nvx-rs/src/host/cli/store/liveness.rs

@@ -1,6 +1,7 @@
 //! Bounded launch proofs, without recovery, record removal, or process termination.
 
 use std::io;
+use std::time::Instant;
 
 use super::StateFile;
 use super::locked::LockedState;
@@ -25,12 +26,19 @@ impl LockedState {
 
     /// Holds the server of the recorded endpoint, rechecks its ownership, and on Linux proves
     /// that this very process also holds the launch's log. An error is not proof of exit.
-    pub(crate) fn endpoint_owner(&self, journal: &LaunchJournal) -> Result<Process> {
+    pub(crate) fn endpoint_owner(
+        &self,
+        journal: &LaunchJournal,
+        deadline: Instant,
+    ) -> Result<Process> {
         if !self.claim_held(journal)? {
             return Err(Error::io("the launch log claim is released"));
         }
-        let process =
-            hold_endpoint_server(|| platform::endpoint_server_pid(&journal.endpoint), find)?;
+        let process = hold_endpoint_server(
+            deadline,
+            |deadline| platform::endpoint_server_pid_until(&journal.endpoint, deadline),
+            find,
+        )?;
         let reference = process.reference();
         let matches =
             platform::serves_launch(reference.pid, reference.start_time, journal.log_identity)
@@ -66,16 +74,23 @@ fn find(pid: u32) -> io::Result<Option<Process>> {
 /// Adapted from the old store's owner check: the second endpoint lookup happens only after
 /// the process is held. The callbacks keep replacement races testable without test-only hooks.
 pub(super) fn hold_endpoint_server(
-    mut server_pid: impl FnMut() -> io::Result<Option<u32>>,
+    deadline: Instant,
+    mut server_pid: impl FnMut(Instant) -> io::Result<Option<u32>>,
     hold: impl FnOnce(u32) -> io::Result<Option<Process>>,
 ) -> Result<Process> {
-    let pid = server_pid()
+    let mut probe = || {
+        if Instant::now() >= deadline {
+            return Err(io::ErrorKind::TimedOut.into());
+        }
+        server_pid(deadline)
+    };
+    let pid = probe()
         .map_err(|error| Error::io("cannot examine the control endpoint").with_source(error))?
         .ok_or_else(|| Error::io("no process serves the control endpoint"))?;
     let process = hold(pid)
         .map_err(|error| Error::io("cannot hold the control endpoint owner").with_source(error))?
         .ok_or_else(|| Error::io("the control endpoint owner exited before it could be held"))?;
-    let again = server_pid()
+    let again = probe()
         .map_err(|error| Error::io("cannot recheck the control endpoint").with_source(error))?;
     if again != Some(pid) {
         return Err(Error::io(

nvx-rs/src/host/cli/store/locked.rs

@@ -54,6 +54,9 @@ impl StateDir {
     /// access, who could hold its lock. A lock file that another transition removed or replaced
     /// after this opened it locks nothing, so the lock is taken again: at once the first time,
     /// and after a pause, until the deadline, when that happens again right after.
+    ///
+    /// Once held, the lock recovers an interrupted launch before returning. Recovery can end a
+    /// verified VM, but retains indeterminate records and reports the cause of its refusal.
     pub(crate) fn lock(&self, deadline: Deadline) -> Result<LockedState> {
         let path = self.path(StateFile::Lock);
         let mut retried_at_once = false;
@@ -71,10 +74,14 @@ impl StateDir {
                 }
             }
             if names_locked_file(&path, &file)? {
-                return Ok(LockedState {
+                let mut locked = LockedState {
                     state: self.clone(),
                     lock: file,
-                });
+                };
+                locked
+                    .recover(deadline.instant())
+                    .map_err(crate::host::cli::sandbox::error_with_cause)?;
+                return Ok(locked);
             }
             if retried_at_once && !deadline.pause(RETRY_INTERVAL)? {
                 return Err(busy());

nvx-rs/src/host/cli/store/mod.rs

@@ -33,6 +33,9 @@
 //! Its journal also identifies the inherited log claim: held means a launch process may run,
 //! and released proves exit. An endpoint owner is held and rechecked before it is returned;
 //! Linux additionally verifies that this process holds that launch's log.
+//! Taking the next lock recovers an interrupted, uncommitted launch before allowing mutation:
+//! it ends only a verified process, removes its journal and capability only after proved exit,
+//! and refuses indeterminate recovery without changing that evidence.
 #![cfg_attr(
     not(test),
     expect(

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

@@ -10,6 +10,7 @@ use crate::host::store::{JOURNAL_NAME, RUNTIME_NAME, SandboxStore};
 
 mod launch;
 mod liveness;
+mod recovery;
 
 /// How long a test waits for a lock that it released to be free.
 ///

nvx-rs/src/host/cli/store/tests/launch.rs

@@ -175,16 +175,19 @@ fn an_interruption_keeps_the_journal_and_capability_without_publishing_runtime()
                 launch.record_process(&saved.process).unwrap();
             }
         }
+        let before = fs::read(state.path(StateFile::Journal)).unwrap();
+        assert!(locked.begin_launch(&prepared, &runtime).is_err());
+        assert_eq!(fs::read(state.path(StateFile::Journal)).unwrap(), before);
         drop(locked);
-        let mut reopened = state.lock(Deadline::after(10.0)).unwrap();
-        let journal = reopened.journal().unwrap().unwrap();
+        let journal: record::LaunchJournal =
+            serde_json::from_slice(&fs::read(state.path(StateFile::Journal)).unwrap()).unwrap();
         assert_eq!(journal.process(), spawned.then_some(&saved.process));
         assert_eq!(journal.capability, saved.capability);
-        assert_eq!(reopened.capability().unwrap(), Some(saved.capability));
-        assert!(reopened.runtime().unwrap().is_none());
-        let before = fs::read(state.path(StateFile::Journal)).unwrap();
-        assert!(reopened.begin_launch(&prepared, &runtime).is_err());
-        assert_eq!(fs::read(state.path(StateFile::Journal)).unwrap(), before);
+        assert_eq!(
+            fs::read(state.path(StateFile::Capability)).unwrap(),
+            saved.capability
+        );
+        assert!(!state.path(StateFile::Runtime).exists());
     }
 }