fix(zone-exec): catch panic errors and show all errors immediately (#359)

crates/ctl/src/console.rs

@@ -1,4 +1,4 @@
-use anyhow::{anyhow, Result};
+use anyhow::Result;
 use async_stream::stream;
 use crossterm::{
     terminal::{disable_raw_mode, enable_raw_mode, is_raw_mode_enabled},
@@ -118,7 +118,12 @@ impl StdioConsoleStream {
                 return if reply.error.is_empty() {
                     Ok(reply.exit_code)
                 } else {
-                    Err(anyhow!("exec failed: {}", reply.error))
+                    StdioConsoleStream::restore_terminal_mode();
+                    stderr
+                        .write_all(format!("Error: exec failed: {}\n", reply.error).as_bytes())
+                        .await?;
+                    stderr.flush().await?;
+                    Ok(-1)
                 };
             }
         }

crates/daemon/src/reconcile/zone/mod.rs

@@ -27,7 +27,7 @@ use tokio::{
     select,
     sync::{
         mpsc::{channel, Receiver, Sender},
-        Mutex, RwLock,
+        RwLock,
     },
     task::JoinHandle,
     time::sleep,
@@ -45,16 +45,9 @@ enum ZoneReconcilerResult {
 }
 
 struct ZoneReconcilerEntry {
-    task: JoinHandle<()>,
     sender: Sender<()>,
 }
 
-impl Drop for ZoneReconcilerEntry {
-    fn drop(&mut self) {
-        self.task.abort();
-    }
-}
-
 #[derive(Clone)]
 pub struct ZoneReconciler {
     devices: DaemonDeviceManager,
@@ -66,7 +59,7 @@ pub struct ZoneReconciler {
     kernel_path: PathBuf,
     initrd_path: PathBuf,
     addons_path: PathBuf,
-    tasks: Arc<Mutex<HashMap<Uuid, ZoneReconcilerEntry>>>,
+    tasks: Arc<RwLock<HashMap<Uuid, ZoneReconcilerEntry>>>,
     zone_reconciler_notify: Sender<Uuid>,
     zone_reconcile_lock: Arc<RwLock<()>>,
     ip_assignment: IpAssignment,
@@ -99,7 +92,7 @@ impl ZoneReconciler {
             kernel_path,
             initrd_path,
             addons_path: modules_path,
-            tasks: Arc::new(Mutex::new(HashMap::new())),
+            tasks: Arc::new(RwLock::new(HashMap::new())),
             zone_reconciler_notify,
             zone_reconcile_lock: Arc::new(RwLock::with_max_readers((), PARALLEL_LIMIT)),
             ip_assignment,
@@ -125,7 +118,7 @@ impl ZoneReconciler {
                                 error!("failed to start zone reconciler task {}: {}", uuid, error);
                             }
 
-                            let map = self.tasks.lock().await;
+                            let map = self.tasks.read().await;
                             if let Some(entry) = map.get(&uuid) {
                                 if let Err(error) = entry.sender.send(()).await {
                                     error!("failed to notify zone reconciler task {}: {}", uuid, error);
@@ -271,7 +264,7 @@ impl ZoneReconciler {
 
             if destroyed {
                 self.zones.remove(uuid).await?;
-                let mut map = self.tasks.lock().await;
+                let mut map = self.tasks.write().await;
                 map.remove(&uuid);
             } else {
                 self.zones.update(uuid, zone.clone()).await?;
@@ -337,7 +330,7 @@ impl ZoneReconciler {
     }
 
     async fn launch_task_if_needed(&self, uuid: Uuid) -> Result<()> {
-        let mut map = self.tasks.lock().await;
+        let mut map = self.tasks.write().await;
         match map.entry(uuid) {
             Entry::Occupied(_) => {}
             Entry::Vacant(entry) => {
@@ -350,7 +343,7 @@ impl ZoneReconciler {
     async fn launch_task(&self, uuid: Uuid) -> Result<ZoneReconcilerEntry> {
         let this = self.clone();
         let (sender, mut receiver) = channel(10);
-        let task = tokio::task::spawn(async move {
+        tokio::task::spawn(async move {
             'notify_loop: loop {
                 if receiver.recv().await.is_none() {
                     break 'notify_loop;
@@ -372,7 +365,7 @@ impl ZoneReconciler {
                 }
             }
         });
-        Ok(ZoneReconcilerEntry { task, sender })
+        Ok(ZoneReconcilerEntry { sender })
     }
 }
 

crates/zone/src/exec.rs

@@ -71,16 +71,21 @@ impl ZoneExecTask {
         if start.tty {
             let pty = Pty::new().map_err(|error| anyhow!("unable to allocate pty: {}", error))?;
             pty.resize(Size::new(24, 80))?;
-            let mut child = ChildDropGuard {
-                inner: pty_process::Command::new(exe)
+            let pts = pty
+                .pts()
+                .map_err(|error| anyhow!("unable to allocate pts: {}", error))?;
+            let child = std::panic::catch_unwind(move || {
+                let pts = pts;
+                pty_process::Command::new(exe)
                     .args(cmd)
                     .envs(env)
                     .current_dir(dir)
-                    .spawn(
-                        &pty.pts()
-                            .map_err(|error| anyhow!("unable to allocate pts: {}", error))?,
-                    )
-                    .map_err(|error| anyhow!("failed to spawn: {}", error))?,
+                    .spawn(&pts)
+            })
+            .map_err(|_| anyhow!("internal error"))
+            .map_err(|error| anyhow!("failed to spawn: {}", error))??;
+            let mut child = ChildDropGuard {
+                inner: child,
                 kill: true,
             };
             let pid = child
@@ -165,16 +170,19 @@ impl ZoneExecTask {
             let _ = join!(pty_read_task);
             stdin_task.abort();
         } else {
-            let mut child = Command::new(exe)
-                .args(cmd)
-                .envs(env)
-                .current_dir(dir)
-                .stdin(Stdio::piped())
-                .stdout(Stdio::piped())
-                .stderr(Stdio::piped())
-                .kill_on_drop(true)
-                .spawn()
-                .map_err(|error| anyhow!("failed to spawn: {}", error))?;
+            let mut child = std::panic::catch_unwind(|| {
+                Command::new(exe)
+                    .args(cmd)
+                    .envs(env)
+                    .current_dir(dir)
+                    .stdin(Stdio::piped())
+                    .stdout(Stdio::piped())
+                    .stderr(Stdio::piped())
+                    .kill_on_drop(true)
+                    .spawn()
+            })
+            .map_err(|_| anyhow!("internal error"))
+            .map_err(|error| anyhow!("failed to spawn: {}", error))??;
 
             let pid = child.id().ok_or_else(|| anyhow!("pid is not provided"))?;
             let mut stdin = child