From 3e9144657b5f79764c762a0f1e8120a96bcce1f5 Mon Sep 17 00:00:00 2001 From: Manos Pitsidianakis Date: Sat, 23 Mar 2024 10:43:58 +0200 Subject: [PATCH] meli: store children process metadata Store children process metadata. Pid and command lines can then be shown in the UI and in logs. Signed-off-by: Manos Pitsidianakis --- meli/src/accounts.rs | 13 +++-- meli/src/mail/compose.rs | 12 +++-- meli/src/mail/view.rs | 16 +++++-- meli/src/mail/view/envelope.rs | 28 +++++++---- meli/src/mail/view/filters.rs | 12 +++-- meli/src/notifications.rs | 6 ++- meli/src/state.rs | 88 ++++++++++++++++++++++------------ meli/src/terminal/embedded.rs | 2 +- meli/src/types.rs | 15 ++++-- tools/src/embedded.rs | 4 +- 10 files changed, 132 insertions(+), 64 deletions(-) diff --git a/meli/src/accounts.rs b/meli/src/accounts.rs index ec67bef8..3ee8e22e 100644 --- a/meli/src/accounts.rs +++ b/meli/src/accounts.rs @@ -53,7 +53,10 @@ use crate::command::actions::AccountAction; use crate::{ conf::{AccountConf, FileMailboxConf}, jobs::{JobId, JoinHandle}, - types::UIEvent::{self, EnvelopeRemove, EnvelopeRename, EnvelopeUpdate, Notification}, + types::{ + ForkType, + UIEvent::{self, EnvelopeRemove, EnvelopeRename, EnvelopeUpdate, Notification}, + }, MainLoopHandler, StatusEvent, ThreadEvent, }; @@ -781,9 +784,11 @@ impl Account { StatusEvent::DisplayMessage(format!("Running command {}", refresh_command)), ))); self.main_loop_handler - .send(ThreadEvent::UIEvent(UIEvent::Fork( - crate::ForkType::Generic(child), - ))); + .send(ThreadEvent::UIEvent(UIEvent::Fork(ForkType::Generic { + id: refresh_command.to_string().into(), + command: Some(refresh_command.clone().into()), + child, + }))); return Ok(()); } let refresh_job = self.backend.write().unwrap().refresh(mailbox_hash); diff --git a/meli/src/mail/compose.rs b/meli/src/mail/compose.rs index c09794de..18edfa99 100644 --- a/meli/src/mail/compose.rs +++ b/meli/src/mail/compose.rs @@ -1876,10 +1876,11 @@ impl Component for Composer { }; if *account_settings!(context[self.account_hash].composing.embedded_pty) { + let command = [editor, f.path().display().to_string()].join(" "); match crate::terminal::embedded::create_pty( self.embedded_dimensions.0, self.embedded_dimensions.1, - [editor, f.path().display().to_string()].join(" "), + &command, ) { Ok(terminal) => { self.embedded_pty = Some(EmbeddedPty { @@ -1891,14 +1892,17 @@ impl Component for Composer { context .replies .push_back(UIEvent::ChangeMode(UIMode::Embedded)); - context.replies.push_back(UIEvent::Fork(ForkType::Embedded( - self.embedded_pty + context.replies.push_back(UIEvent::Fork(ForkType::Embedded { + id: "editor".into(), + command: Some(command.into()), + pid: self + .embedded_pty .as_ref() .unwrap() .lock() .unwrap() .child_pid, - ))); + })); self.mode = ViewMode::EmbeddedPty; } Err(err) => { diff --git a/meli/src/mail/view.rs b/meli/src/mail/view.rs index 1f3dbad7..c0236e5b 100644 --- a/meli/src/mail/view.rs +++ b/meli/src/mail/view.rs @@ -726,12 +726,16 @@ impl Component for MailView { .spawn() { Ok(child) => { - context.children.push(child); + context + .children + .entry(url_launcher.to_string().into()) + .or_default() + .push(child); } Err(err) => { context.replies.push_back(UIEvent::StatusEvent( StatusEvent::DisplayMessage(format!( - "Couldn't launch {:?}: {}", + "Couldn't launch {}: {}", url_launcher, err )), )); @@ -766,11 +770,15 @@ impl Component for MailView { .stdout(Stdio::piped()) .spawn() { - Ok(child) => context.children.push(child), + Ok(child) => context + .children + .entry(url_launcher.to_string().into()) + .or_default() + .push(child), Err(err) => { context.replies.push_back(UIEvent::StatusEvent( StatusEvent::DisplayMessage(format!( - "Couldn't launch {:?}: {}", + "Couldn't launch {}: {}", url_launcher, err )), )); diff --git a/meli/src/mail/view/envelope.rs b/meli/src/mail/view/envelope.rs index a9fb0587..a59324ec 100644 --- a/meli/src/mail/view/envelope.rs +++ b/meli/src/mail/view/envelope.rs @@ -152,8 +152,8 @@ impl EnvelopeView { format!("Failed to start html filter process: {}", filter_invocation,) .into(), ), - source: None, body: err.to_string().into(), + source: Some(err.into()), kind: Some(NotificationType::Error(melib::ErrorKind::External)), })); // [ref:FIXME]: add `v` configurable shortcut @@ -1294,15 +1294,15 @@ impl Component for EnvelopeView { } match save_attachment(&path, &self.mail.bytes) { Err(err) => { + log::error!("Failed to create file at {}: {err}", path.display()); context.replies.push_back(UIEvent::Notification { title: Some( format!("Failed to create file at {}", path.display()).into(), ), - source: None, body: err.to_string().into(), + source: Some(err), kind: Some(NotificationType::Error(melib::ErrorKind::External)), }); - log::error!("Failed to create file at {}: {err}", path.display()); return true; } Ok(()) => { @@ -1336,15 +1336,15 @@ impl Component for EnvelopeView { } match save_attachment(&path, &u.decode(Default::default())) { Err(err) => { + log::error!("Failed to create file at {}: {err}", path.display()); context.replies.push_back(UIEvent::Notification { title: Some( format!("Failed to create file at {}", path.display()).into(), ), - source: None, body: err.to_string().into(), + source: Some(err), kind: Some(NotificationType::Error(melib::ErrorKind::External)), }); - log::error!("Failed to create file at {}: {err}", path.display()); } Ok(()) => { context.replies.push_back(UIEvent::Notification { @@ -1365,15 +1365,15 @@ impl Component for EnvelopeView { } match save_attachment(&path, &self.mail.bytes) { Err(err) => { + log::error!("Failed to create file at {}: {err}", path.display()); context.replies.push_back(UIEvent::Notification { title: Some( format!("Failed to create file at {}", path.display()).into(), ), - source: None, body: err.to_string().into(), + source: Some(err), kind: Some(NotificationType::Error(melib::ErrorKind::External)), }); - log::error!("Failed to create file at {}: {err}", path.display()); return true; } Ok(()) => { @@ -1466,7 +1466,11 @@ impl Component for EnvelopeView { match res { Ok((p, child)) => { context.temp_files.push(p); - context.children.push(child); + context + .children + .entry(command.into()) + .or_default() + .push(child); } Err(err) => { context.replies.push_back(UIEvent::StatusEvent( @@ -1560,13 +1564,17 @@ impl Component for EnvelopeView { .spawn() { Ok(child) => { - context.children.push(child); + context + .children + .entry(url_launcher.to_string().into()) + .or_default() + .push(child); } Err(err) => { context.replies.push_back(UIEvent::Notification { title: Some(format!("Failed to launch {:?}", url_launcher).into()), - source: None, body: err.to_string().into(), + source: Some(err.into()), kind: Some(NotificationType::Error(melib::ErrorKind::External)), }); } diff --git a/meli/src/mail/view/filters.rs b/meli/src/mail/view/filters.rs index 0326f89c..d598a57f 100644 --- a/meli/src/mail/view/filters.rs +++ b/meli/src/mail/view/filters.rs @@ -343,11 +343,15 @@ impl ViewFilter { }); match res { Ok((p, child)) => { - context - .replies - .push_back(UIEvent::StatusEvent(StatusEvent::UpdateSubStatus(command))); + context.replies.push_back(UIEvent::StatusEvent( + StatusEvent::UpdateSubStatus(command.clone()), + )); context.temp_files.push(p); - context.children.push(child); + context + .children + .entry(command.into()) + .or_default() + .push(child); } Err(err) => { context.replies.push_back(UIEvent::StatusEvent( diff --git a/meli/src/notifications.rs b/meli/src/notifications.rs index 4d625b64..ef36f94b 100644 --- a/meli/src/notifications.rs +++ b/meli/src/notifications.rs @@ -231,7 +231,11 @@ impl Component for NotificationCommand { .spawn() { Ok(child) => { - context.children.push(child); + context + .children + .entry(stringify!(NotificationCommand).into()) + .or_default() + .push(child); } Err(err) => { log::error!("Could not run notification script: {err}."); diff --git a/meli/src/state.rs b/meli/src/state.rs index 7ca49ca0..b5c4d029 100644 --- a/meli/src/state.rs +++ b/meli/src/state.rs @@ -35,6 +35,7 @@ //! [`ThreadEvent`]. use std::{ + borrow::Cow, collections::BTreeSet, env, os::fd::{AsRawFd, FromRawFd, OwnedFd}, @@ -143,8 +144,8 @@ pub struct Context { receiver: Receiver, input_thread: InputHandler, current_dir: PathBuf, - pub children: Vec, - + /// Children processes + pub children: IndexMap, Vec>, pub temp_files: Vec, } @@ -250,7 +251,7 @@ impl Context { unrealized: IndexSet::default(), temp_files: Vec::new(), current_dir: std::env::current_dir().unwrap(), - children: vec![], + children: IndexMap::default(), input_thread: InputHandler { pipe: input_thread_pipe, @@ -293,18 +294,38 @@ impl Drop for State { // When done, restore the defaults to avoid messing with the terminal. self.screen.switch_to_main_screen(); use nix::sys::wait::{waitpid, WaitPidFlag}; - for child in self.context.children.iter_mut() { - if let Err(err) = waitpid( - nix::unistd::Pid::from_raw(child.id() as i32), - Some(WaitPidFlag::WNOHANG), - ) { - log::warn!("Failed to wait on subprocess {}: {}", child.id(), err); - } + for (id, pid, err) in self + .context + .children + .iter_mut() + .flat_map(|(i, v)| v.drain(..).map(move |v| (i, v))) + .filter_map(|(id, child)| { + if let Err(err) = waitpid( + nix::unistd::Pid::from_raw(child.id() as i32), + Some(WaitPidFlag::WNOHANG), + ) { + Some((id, child.id(), err)) + } else { + None + } + }) + { + log::trace!("Failed to wait on subprocess {} ({}): {}", id, pid, err); } - if let Some(ForkType::Embedded(child_pid)) = self.child.take() { + if let Some(ForkType::Embedded { id, command, pid }) = self.child.take() { /* Try wait, we don't want to block */ - if let Err(e) = waitpid(child_pid, Some(WaitPidFlag::WNOHANG)) { - log::warn!("Failed to wait on subprocess {}: {}", child_pid, e); + if let Err(err) = waitpid(pid, Some(WaitPidFlag::WNOHANG)) { + log::trace!( + "Failed to wait on embedded process {} {} ({}): {}", + id, + if let Some(v) = command.as_ref() { + v.as_ref() + } else { + "" + }, + pid, + err + ); } } } @@ -428,7 +449,7 @@ impl State { unrealized: IndexSet::default(), temp_files: Vec::new(), current_dir: std::env::current_dir()?, - children: vec![], + children: IndexMap::default(), input_thread: InputHandler { pipe: input_thread_pipe, @@ -1005,8 +1026,12 @@ impl State { self.context.restore_input(); return; } - UIEvent::Fork(ForkType::Generic(child)) => { - self.context.children.push(child); + UIEvent::Fork(ForkType::Generic { + id, + command: _, + child, + }) => { + self.context.children.entry(id).or_default().push(child); return; } UIEvent::Fork(child) => { @@ -1194,24 +1219,27 @@ impl State { pub fn try_wait_on_child(&mut self) -> Option { let should_return_flag = match self.child { - Some(ForkType::NewDraft(_, ref mut c)) => { - let w = c.try_wait(); + Some(ForkType::Generic { + ref id, + ref command, + ref mut child, + }) => { + let w = child.try_wait(); match w { Ok(Some(_)) => true, Ok(None) => false, Err(err) => { - log::error!("Failed to wait on editor process: {err}"); - return None; - } - } - } - Some(ForkType::Generic(ref mut c)) => { - let w = c.try_wait(); - match w { - Ok(Some(_)) => true, - Ok(None) => false, - Err(err) => { - log::error!("Failed to wait on child process: {err}"); + log::error!( + "Failed to wait on child process {} {} ({}): {}", + id, + if let Some(v) = command.as_ref() { + v.as_ref() + } else { + "" + }, + child.id(), + err + ); return None; } } diff --git a/meli/src/terminal/embedded.rs b/meli/src/terminal/embedded.rs index c8b98279..b14f920f 100644 --- a/meli/src/terminal/embedded.rs +++ b/meli/src/terminal/embedded.rs @@ -78,7 +78,7 @@ ioctl_none_bad!( /// Create a new pseudoterminal (PTY) with given width, size and execute /// `command` in it. -pub fn create_pty(width: usize, height: usize, command: String) -> Result>> { +pub fn create_pty(width: usize, height: usize, command: &str) -> Result>> { #[cfg(not(target_os = "macos"))] let (frontend_fd, backend_name): (nix::pty::PtyMaster, String) = { // Open a new PTY frontend diff --git a/meli/src/types.rs b/meli/src/types.rs index 660cc946..c828652d 100644 --- a/meli/src/types.rs +++ b/meli/src/types.rs @@ -100,9 +100,16 @@ pub enum ForkType { /// Already finished fork, we only want to restore input/output Finished, /// Embedded pty - Embedded(Pid), - Generic(std::process::Child), - NewDraft(File, std::process::Child), + Embedded { + id: Cow<'static, str>, + command: Option>, + pid: Pid, + }, + Generic { + id: Cow<'static, str>, + command: Option>, + child: std::process::Child, + }, } #[derive(Clone, Copy, Debug, Eq, PartialEq)] @@ -140,8 +147,8 @@ pub enum UIEvent { Command(String), Notification { title: Option>, - source: Option, body: Cow<'static, str>, + source: Option, kind: Option, }, Action(Action), diff --git a/tools/src/embedded.rs b/tools/src/embedded.rs index 8a0c7e5c..cab12717 100644 --- a/tools/src/embedded.rs +++ b/tools/src/embedded.rs @@ -167,7 +167,7 @@ impl Component for EmbeddedContainer { } else { let theme_default = crate::conf::value(context, "theme_default"); grid.clear_area(area, theme_default); - match create_pty(area.width(), area.height(), self.command.clone()) { + match create_pty(area.width(), area.height(), &self.command) { Ok(embedded_pty) => { //embedded_pty.lock().unwrap().set_log_file(self.log_file.take()); self.embedded_pty = Some(EmbeddedPty::Running(embedded_pty)); @@ -187,8 +187,8 @@ impl Component for EmbeddedContainer { Err(err) => { context.replies.push_back(UIEvent::Notification { title: Some("Failed to create pseudoterminal".into()), - source: None, body: err.to_string().into(), + source: Some(err), kind: Some(NotificationType::Error(melib::error::ErrorKind::External)), }); }