fix(vmm): scoped ShimReaper for zombie shim PIDs (Issue #523)

src/boxlite/src/litebox/init/tasks/vmm_spawn.rs

@@ -90,9 +90,15 @@ impl PipelineTask<InitCtx> for VmmSpawnTask {
         .inspect_err(|e| log_task_error(&box_id, task_name, e))?;
 
         // Spawn VM
-        let handler = spawn_vm(&box_id, &instance_spec, &options, &layout)
-            .await
-            .inspect_err(|e| log_task_error(&box_id, task_name, e))?;
+        let handler = spawn_vm(
+            &box_id,
+            &instance_spec,
+            &options,
+            &layout,
+            &runtime.shim_reaper,
+        )
+        .await
+        .inspect_err(|e| log_task_error(&box_id, task_name, e))?;
 
         let mut ctx = ctx.lock().await;
         ctx.guard.set_handler(handler);
@@ -375,13 +381,15 @@ async fn spawn_vm(
     config: &InstanceSpec,
     options: &BoxOptions,
     layout: &BoxFilesystemLayout,
+    reaper: &std::sync::Arc<crate::util::ShimReaper>,
 ) -> BoxliteResult<Box<dyn VmmHandler>> {
     let mut controller = ShimController::new(
         find_binary("boxlite-shim")?,
         VmmKind::Libkrun,
         box_id.clone(),
         options.clone(),
         layout.clone(),
+        reaper.clone(),
     )?;
 
     controller.start(config).await

src/boxlite/src/metrics/runtime_metrics.rs

@@ -19,6 +19,21 @@ pub struct RuntimeMetricsStorage {
     pub(crate) total_commands: Arc<AtomicU64>,
     /// Total command execution errors across all boxes
     pub(crate) total_exec_errors: Arc<AtomicU64>,
+    /// Total shim PIDs the scoped reaper has collected (Issue #523).
+    /// Monotonic; tracks shim children whose `Child` handle was dropped
+    /// without `wait()` — the load-bearing zombie-prevention signal.
+    pub(crate) shim_reaped: Arc<AtomicU64>,
+    /// Total shim PIDs that vanished out from under the reaper before
+    /// it could collect them (ECHILD on waitpid). Monotonic. High vs
+    /// `shim_reaped` ratio means most cleanups are handled by the
+    /// owner's own `Child::wait()` — that's a healthy signal.
+    pub(crate) shim_reaper_vanished: Arc<AtomicU64>,
+    /// Current size of the reaper's PID registry. Gauge, not counter —
+    /// this rises when a shim is registered, falls when reaper sweeps
+    /// it. Sustained growth means either many running boxes or a
+    /// reaper-side bug; flag in dashboards if it climbs without
+    /// `shim_reaped` keeping pace.
+    pub(crate) shim_reaper_registered: Arc<AtomicU64>,
 }
 
 impl RuntimeMetricsStorage {
@@ -92,6 +107,24 @@ impl RuntimeMetrics {
     pub fn total_exec_errors(&self) -> u64 {
         self.storage.total_exec_errors.load(Ordering::Relaxed)
     }
+
+    /// Total shim PIDs the scoped reaper collected (Issue #523).
+    /// Never decreases.
+    pub fn shim_reaped_total(&self) -> u64 {
+        self.storage.shim_reaped.load(Ordering::Relaxed)
+    }
+
+    /// Total shim PIDs that vanished (ECHILD) before the reaper saw
+    /// them. Never decreases. See `shim_reaped` field doc on
+    /// `RuntimeMetricsStorage` for what the ratio means operationally.
+    pub fn shim_reaper_vanished_total(&self) -> u64 {
+        self.storage.shim_reaper_vanished.load(Ordering::Relaxed)
+    }
+
+    /// Current size of the reaper's PID registry. Gauge.
+    pub fn shim_reaper_registered(&self) -> u64 {
+        self.storage.shim_reaper_registered.load(Ordering::Relaxed)
+    }
 }
 
 #[cfg(test)]

src/boxlite/src/rest/runtime.rs

@@ -171,6 +171,15 @@ fn runtime_metrics_from_response(resp: &RuntimeMetricsResponse) -> RuntimeMetric
     storage
         .total_exec_errors
         .store(resp.total_exec_errors, Ordering::Relaxed);
+    storage
+        .shim_reaped
+        .store(resp.shim_reaped_total, Ordering::Relaxed);
+    storage
+        .shim_reaper_vanished
+        .store(resp.shim_reaper_vanished_total, Ordering::Relaxed);
+    storage
+        .shim_reaper_registered
+        .store(resp.shim_reaper_registered, Ordering::Relaxed);
 
     RuntimeMetrics::new(storage)
 }

src/boxlite/src/rest/types.rs

@@ -406,6 +406,14 @@ pub(crate) struct RuntimeMetricsResponse {
     pub total_commands_executed: u64,
     #[serde(default)]
     pub total_exec_errors: u64,
+    /// Scoped shim reaper counters (Issue #523). Optional for
+    /// forward/backward compat with servers that pre-date the reaper.
+    #[serde(default)]
+    pub shim_reaped_total: u64,
+    #[serde(default)]
+    pub shim_reaper_vanished_total: u64,
+    #[serde(default)]
+    pub shim_reaper_registered: u64,
 }
 
 #[derive(Debug, Deserialize)]
@@ -690,6 +698,50 @@ mod tests {
         assert_eq!(resp.total_commands_executed, 100);
     }
 
+    #[test]
+    fn test_runtime_metrics_back_compat_default_zero_for_missing_shim_fields() {
+        // Older servers that pre-date the shim reaper (Issue #523) emit
+        // JSON without the three `shim_*` fields. `serde(default)` on the
+        // client mirror must surface them as 0 instead of failing the
+        // deserialize — losing metrics is preferable to losing the whole
+        // /v1/metrics response.
+        let json = r#"{
+            "boxes_created_total": 0,
+            "boxes_failed_total": 0,
+            "boxes_stopped_total": 0,
+            "num_running_boxes": 0,
+            "total_commands_executed": 0,
+            "total_exec_errors": 0
+        }"#;
+        let resp: RuntimeMetricsResponse = serde_json::from_str(json).unwrap();
+        assert_eq!(resp.shim_reaped_total, 0);
+        assert_eq!(resp.shim_reaper_vanished_total, 0);
+        assert_eq!(resp.shim_reaper_registered, 0);
+    }
+
+    #[test]
+    fn test_runtime_metrics_shim_fields_round_trip() {
+        // Server-shaped JSON with the new fields populated round-trips
+        // through the deserializer with values intact. Locks the wire
+        // contract: the server's `shim_reaped_total` etc. must reach the
+        // client unchanged.
+        let json = r#"{
+            "boxes_created_total": 5,
+            "boxes_failed_total": 1,
+            "boxes_stopped_total": 2,
+            "num_running_boxes": 2,
+            "total_commands_executed": 0,
+            "total_exec_errors": 0,
+            "shim_reaped_total": 17,
+            "shim_reaper_vanished_total": 4,
+            "shim_reaper_registered": 3
+        }"#;
+        let resp: RuntimeMetricsResponse = serde_json::from_str(json).unwrap();
+        assert_eq!(resp.shim_reaped_total, 17);
+        assert_eq!(resp.shim_reaper_vanished_total, 4);
+        assert_eq!(resp.shim_reaper_registered, 3);
+    }
+
     #[test]
     fn test_box_status_transient_mapping() {
         let mut resp = BoxResponse {

src/boxlite/src/runtime/rt_impl.rs

@@ -109,6 +109,11 @@ pub struct RuntimeImpl {
     /// Use `.is_cancelled()` for sync checks, `.cancelled()` for async select!.
     /// Child tokens are passed to each box via `.child_token()`.
     pub(crate) shutdown_token: CancellationToken,
+
+    /// Scoped reaper for shim PIDs (Issue #523). Cleans up zombies left
+    /// by abandoned mid-init shims. Per-runtime so test binaries can have
+    /// many runtimes serially without leaking worker tasks.
+    pub(crate) shim_reaper: Arc<crate::util::ShimReaper>,
 }
 
 /// Synchronized state protected by RwLock.
@@ -223,6 +228,17 @@ impl RuntimeImpl {
             ImageDiskManager::new(layout.image_layout().disk_images_dir(), layout.temp_dir());
         let guest_rootfs_mgr = GuestRootfsManager::new(base_disk_mgr.clone(), layout.temp_dir());
 
+        // Construct runtime_metrics before the reaper so we can hand it
+        // a sink that mirrors reap activity into the metrics storage
+        // (visible via `RuntimeMetrics` and the REST /v1/metrics endpoint).
+        let runtime_metrics = RuntimeMetricsStorage::new();
+        let shim_reaper =
+            crate::util::ShimReaper::spawn_with_sink(Some(crate::util::ShimReaperMetricsSink {
+                reaped_total: Arc::clone(&runtime_metrics.shim_reaped),
+                vanished_total: Arc::clone(&runtime_metrics.shim_reaper_vanished),
+                registered_now: Arc::clone(&runtime_metrics.shim_reaper_registered),
+            }));
+
         let inner = Arc::new(Self {
             sync_state: RwLock::new(SynchronizedState {
                 active_boxes_by_id: HashMap::new(),
@@ -234,12 +250,13 @@ impl RuntimeImpl {
             image_disk_mgr,
             guest_rootfs_mgr,
             guest_rootfs: Arc::new(OnceCell::new()),
-            runtime_metrics: RuntimeMetricsStorage::new(),
+            runtime_metrics,
             base_disk_mgr,
             snapshot_mgr,
             lock_manager,
             _runtime_lock: runtime_lock,
             shutdown_token: CancellationToken::new(),
+            shim_reaper,
         });
 
         tracing::debug!("initialized runtime");
@@ -615,6 +632,7 @@ impl RuntimeImpl {
 
         if active_boxes.is_empty() {
             tracing::info!("No active boxes to shutdown");
+            self.shim_reaper.shutdown();
             return Ok(());
         }
 
@@ -657,6 +675,13 @@ impl RuntimeImpl {
             }
         }
 
+        // Stop the shim reaper worker. Sync call but very fast — the
+        // Condvar wakeup short-circuits the REAPER_TICK sleep. Doing this
+        // after the per-box stop loop means any final shim exits triggered
+        // by the stop loop get a chance to be reaped in the reaper's
+        // shutdown-drain pass.
+        self.shim_reaper.shutdown();
+
         if errors.is_empty() {
             tracing::info!("Runtime shutdown complete");
             Ok(())
@@ -736,6 +761,11 @@ impl RuntimeImpl {
                 .pid_file_path();
             let _ = std::fs::remove_file(&pid_file);
         }
+
+        // Stop the reaper worker thread. Mirrors the async shutdown() path —
+        // important for the atexit case so a long-lived process doesn't
+        // leak the reaper thread on graceful exit.
+        self.shim_reaper.shutdown();
     }
 
     // ========================================================================

src/boxlite/src/util/mod.rs

@@ -1,6 +1,7 @@
 mod binary_finder;
 mod pid_file;
 pub mod process;
+pub mod reaper;
 
 pub use binary_finder::{RuntimeBinaryFinder, find_binary};
 pub use pid_file::{PidFileReader, PidFileWriter, PidRecord, ProcessIdentity};
@@ -17,6 +18,7 @@ use tracing_subscriber::{EnvFilter, fmt};
 pub use process::{
     ProcessExit, ProcessMonitor, is_process_alive, kill_process, process_start_time,
 };
+pub use reaper::{MetricsSink as ShimReaperMetricsSink, ReaperStats, ShimReaper};
 
 #[cfg(any(target_os = "linux", target_os = "macos"))]
 unsafe extern "C" {