From 3849835ff9659ee81f669c84ce1f3628d8abfd9f Mon Sep 17 00:00:00 2001 From: elkowar <5300871+elkowar@users.noreply.github.com> Date: Fri, 23 Oct 2020 20:28:18 +0200 Subject: [PATCH] Stop script-vars and close windows on eww kill --- Cargo.lock | 25 +++++++- Cargo.toml | 1 + src/app.rs | 3 + src/config/script_var.rs | 17 ++++- src/eww_state.rs | 9 +-- src/main.rs | 13 +++- src/script_var_handler.rs | 126 ++++++++++++++++++++++---------------- src/widgets/mod.rs | 4 +- 8 files changed, 129 insertions(+), 69 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b738ad1..7507e74 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -247,6 +247,16 @@ dependencies = [ "syn 1.0.44", ] +[[package]] +name = "ctrlc" +version = "3.1.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b57a92e9749e10f25a171adcebfafe72991d45e7ec2dcb853e8f83d9dafaeb08" +dependencies = [ + "nix 0.18.0", + "winapi 0.3.9", +] + [[package]] name = "debug_stub_derive" version = "0.3.0" @@ -300,6 +310,7 @@ dependencies = [ "anyhow", "bincode", "crossbeam-channel", + "ctrlc", "debug_stub_derive", "derive_more", "extend", @@ -317,7 +328,7 @@ dependencies = [ "libc", "log 0.4.11", "maplit", - "nix", + "nix 0.19.0", "notify", "num", "popol", @@ -1049,6 +1060,18 @@ dependencies = [ "winapi 0.3.9", ] +[[package]] +name = "nix" +version = "0.18.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "83450fe6a6142ddd95fb064b746083fc4ef1705fe81f64a64e1d4b39f54a1055" +dependencies = [ + "bitflags", + "cc", + "cfg-if", + "libc", +] + [[package]] name = "nix" version = "0.19.0" diff --git a/Cargo.toml b/Cargo.toml index 5aa16dc..8a6b5f6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -45,6 +45,7 @@ popol = "0.3" nix = "0.19" smart-default = "0.6" filedescriptor = "0.7" +ctrlc = { version = "3.1", features = [ "termination" ] } [dev-dependencies] pretty_assertions = "0.6.1" diff --git a/src/app.rs b/src/app.rs index f995d29..324ff67 100644 --- a/src/app.rs +++ b/src/app.rs @@ -61,6 +61,9 @@ impl App { EwwCommand::ReloadCss(css) => self.load_css(&css), EwwCommand::KillServer => { log::info!("Received kill command, stopping server!"); + self.script_var_handler.stop(); + self.windows.values().for_each(|w| w.gtk_window.close()); + script_var_process::on_application_death(); std::process::exit(0); } EwwCommand::OpenWindow { window_name, pos, size } => self.open_window(&window_name, pos, size), diff --git a/src/config/script_var.rs b/src/config/script_var.rs index 42d789b..ac225c0 100644 --- a/src/config/script_var.rs +++ b/src/config/script_var.rs @@ -1,3 +1,5 @@ +use std::process::Command; + use anyhow::*; use crate::ensure_xml_tag_is; @@ -11,6 +13,12 @@ pub struct PollScriptVar { pub interval: std::time::Duration, } +impl PollScriptVar { + pub fn run_once(&self) -> Result { + run_command(&self.command) + } +} + #[derive(Clone, Debug, PartialEq)] pub struct TailScriptVar { pub name: VarName, @@ -33,7 +41,7 @@ impl ScriptVar { pub fn initial_value(&self) -> Result { match self { - ScriptVar::Poll(x) => Ok(crate::run_command(&x.command)?), + ScriptVar::Poll(x) => Ok(run_command(&x.command)?), ScriptVar::Tail(_) => Ok(PrimitiveValue::from_string(String::new())), } } @@ -51,3 +59,10 @@ impl ScriptVar { } } } + +/// Run a command and get the output +fn run_command(cmd: &str) -> Result { + let output = String::from_utf8(Command::new("/bin/sh").arg("-c").arg(cmd).output()?.stdout)?; + let output = output.trim_matches('\n'); + Ok(PrimitiveValue::from(output)) +} diff --git a/src/eww_state.rs b/src/eww_state.rs index 91b26e9..b9aa901 100644 --- a/src/eww_state.rs +++ b/src/eww_state.rs @@ -4,7 +4,7 @@ use crate::{ value::{AttrName, AttrValueElement, VarName}, }; use anyhow::*; -use std::{collections::HashMap, process::Command, sync::Arc}; +use std::{collections::HashMap, sync::Arc}; use crate::value::{AttrValue, PrimitiveValue}; @@ -171,10 +171,3 @@ impl EwwState { } } } - -/// Run a command and get the output -pub fn run_command(cmd: &str) -> Result { - let output = String::from_utf8(Command::new("/bin/sh").arg("-c").arg(cmd).output()?.stdout)?; - let output = output.trim_matches('\n'); - Ok(PrimitiveValue::from(output)) -} diff --git a/src/main.rs b/src/main.rs index 3c6ea78..e8c5063 100644 --- a/src/main.rs +++ b/src/main.rs @@ -70,6 +70,7 @@ lazy_static::lazy_static! { fn main() { pretty_env_logger::init(); + if let Err(e) = try_main() { eprintln!("{:?}", e); } @@ -181,6 +182,13 @@ fn try_main() -> Result<()> { do_detach()?; } + ctrlc::set_handler(|| { + println!("Shutting down eww daemon..."); + script_var_handler::script_var_process::on_application_death(); + std::process::exit(0); + }) + .context("Error setting signal hook")?; + initialize_server(action)?; } } @@ -344,13 +352,12 @@ fn do_detach() -> Result<()> { let fd = file.as_raw_fd(); if nix::unistd::isatty(1)? { - nix::unistd::dup2(std::io::stdout().as_raw_fd(), fd)?; + nix::unistd::dup2(fd, std::io::stdout().as_raw_fd())?; } if nix::unistd::isatty(2)? { - nix::unistd::dup2(std::io::stderr().as_raw_fd(), fd)?; + nix::unistd::dup2(fd, std::io::stderr().as_raw_fd())?; } - nix::unistd::setsid().context("Failed to run setsid")?; Ok(()) } diff --git a/src/script_var_handler.rs b/src/script_var_handler.rs index 6fca8a2..e9c5ad0 100644 --- a/src/script_var_handler.rs +++ b/src/script_var_handler.rs @@ -1,12 +1,14 @@ -use std::{collections::HashMap, ffi::CString, io::BufReader, time::Duration}; +use std::{collections::HashMap, time::Duration}; -use crate::{app, config, eww_state, util, value::PrimitiveValue}; +use crate::{app, config, util, value::PrimitiveValue}; use anyhow::*; use app::EwwCommand; use glib; use itertools::Itertools; use scheduled_executor; -use std::{io::BufRead, os::unix::io::AsRawFd}; +use std::io::BufRead; + +use self::script_var_process::ScriptVarProcess; /// Handler that manages running and updating [ScriptVar]s pub struct ScriptVarHandler { @@ -68,8 +70,7 @@ impl ScriptVarHandler { var.interval, glib::clone!(@strong var, @strong evt_send => move |_| { let result: Result<_> = try { - let output = eww_state::run_command(&var.command)?; - evt_send.send(app::EwwCommand::UpdateVar(var.name.clone(), output))?; + evt_send.send(app::EwwCommand::UpdateVar(var.name.clone(), var.run_once()?))?; }; util::print_result_err("while running script-var command", &result); }), @@ -85,25 +86,18 @@ impl ScriptVarHandler { log::info!("initializing handler for tail script vars"); let mut sources = popol::Sources::with_capacity(tail_script_vars.len()); - // TODO clean up this unnecessary vec, it really should not be needed. - // should be possibel to just keep a BufReader in TailVarProcess directly - let mut command_children = Vec::new(); - let mut command_out_handles: HashMap<_, BufReader> = HashMap::new(); + let mut script_var_processes: HashMap<_, ScriptVarProcess> = HashMap::new(); for var in tail_script_vars { - match TailVarProcess::run(&var.command) { + match ScriptVarProcess::run(&var.command) { Ok(process) => { - command_out_handles.insert(var.name.clone(), BufReader::new(process.out_fd.try_clone()?)); - command_children.push(process); + sources.register(var.name.clone(), process.stdout_reader.get_ref(), popol::interest::READ); + script_var_processes.insert(var.name.clone(), process); } Err(err) => eprintln!("Failed to launch script-var command for tail: {:?}", err), } } - for (var_name, handle) in command_out_handles.iter() { - sources.register(var_name.clone(), handle.get_ref(), popol::interest::READ); - } - let mut events = popol::Events::with_capacity(tail_script_vars.len()); let evt_send = self.evt_send.clone(); // TODO this is rather ugly @@ -113,26 +107,24 @@ impl ScriptVarHandler { sources.wait(&mut events)?; for (var_name, event) in events.iter() { if event.readable { - let handle = command_out_handles + let handle = script_var_processes .get_mut(var_name) .with_context(|| format!("No command output handle found for variable '{}'", var_name))?; let mut buffer = String::new(); - handle.read_line(&mut buffer)?; + handle.stdout_reader.read_line(&mut buffer)?; evt_send.send(EwwCommand::UpdateVar( - var_name.clone(), + var_name.to_owned(), PrimitiveValue::from_string(buffer.trim_matches('\n').to_owned()), ))?; } else if event.hangup { - command_out_handles.remove(var_name); + script_var_processes.remove(var_name); sources.unregister(var_name); } } }; util::print_result_err("in script-var tail handler thread", &result); } - - // stop child processes after exit - command_children.drain(..).for_each(|process| process.kill()); + script_var_processes.values().for_each(|process| process.kill()); }); self.tail_handler_thread = Some(thread_handle); Ok(()) @@ -145,42 +137,68 @@ impl Drop for ScriptVarHandler { } } -#[derive(Debug)] -struct TailVarProcess { - pid: nix::unistd::Pid, - out_fd: filedescriptor::FileDescriptor, -} +pub mod script_var_process { + use anyhow::*; + use nix::{ + sys::{signal, wait}, + unistd::Pid, + }; + use std::{io::BufReader, process::Stdio, sync::Mutex}; -impl TailVarProcess { - pub fn run(command: &str) -> Result { - use nix::unistd::*; + use crate::util; - let pipe = filedescriptor::Pipe::new()?; + lazy_static::lazy_static! { + static ref SCRIPT_VAR_CHILDREN: Mutex> = Mutex::new(Vec::new()); + } - match unsafe { fork()? } { - ForkResult::Child => { - std::mem::drop(pipe.read); - dup2(pipe.write.as_raw_fd(), std::io::stdout().as_raw_fd())?; - setpgid(Pid::from_raw(0), Pid::from_raw(0))?; - execv( - CString::new("/bin/sh")?.as_ref(), - &[CString::new("/bin/sh")?, CString::new("-c")?, CString::new(command)?], - )?; - unreachable!("Child fork called exec, thus the process was replaced by the command the user provided"); - } - ForkResult::Parent { child, .. } => { - std::mem::drop(pipe.write); - setpgid(child, child)?; - Ok(TailVarProcess { - pid: child, - out_fd: pipe.read, - }) - } + fn terminate_pid(pid: u32) { + println!("Killing pid: {}", pid); + let result = signal::kill(Pid::from_raw(pid as i32), signal::SIGTERM); + util::print_result_err("While killing tail-var child processes", &result); + let wait_result = wait::waitpid(Pid::from_raw(pid as i32), None); + util::print_result_err("While killing tail-var child processes", &wait_result); + } + + /// This function should be called in the signal handler, killing all child processes. + pub fn on_application_death() { + SCRIPT_VAR_CHILDREN + .lock() + .unwrap() + .drain(..) + .for_each(|pid| terminate_pid(pid)); + } + + pub struct ScriptVarProcess { + child: std::process::Child, + pub stdout_reader: BufReader, + } + + impl ScriptVarProcess { + pub(super) fn run(command: &str) -> Result { + println!("Running {}", command); + let mut child = std::process::Command::new("/bin/sh") + .arg("-c") + .arg(command) + .stdout(Stdio::piped()) + .stderr(Stdio::inherit()) + .stdin(Stdio::null()) + .spawn()?; + SCRIPT_VAR_CHILDREN.lock().unwrap().push(child.id()); + Ok(ScriptVarProcess { + stdout_reader: BufReader::new(child.stdout.take().unwrap()), + child, + }) + } + + pub(super) fn kill(&self) { + SCRIPT_VAR_CHILDREN.lock().unwrap().retain(|item| *item != self.child.id()); + terminate_pid(self.child.id()); } } - pub fn kill(self) { - let result = nix::sys::signal::kill(self.pid, Some(nix::sys::signal::SIGTERM)); - util::print_result_err("Killing tail-var process", &result); + impl Drop for ScriptVarProcess { + fn drop(&mut self) { + self.kill(); + } } } diff --git a/src/widgets/mod.rs b/src/widgets/mod.rs index 71700fa..daaf13b 100644 --- a/src/widgets/mod.rs +++ b/src/widgets/mod.rs @@ -16,9 +16,9 @@ const CMD_STRING_PLACEHODLER: &str = "{}"; /// Run a command that was provided as an attribute. This command may use a /// placeholder ('{}') which will be replaced by the value provided as [`arg`] -pub fn run_command(cmd: &str, arg: T) { +pub(self) fn run_command(cmd: &str, arg: T) { let cmd = cmd.replace(CMD_STRING_PLACEHODLER, &format!("{}", arg)); - if let Err(e) = Command::new("/bin/sh").arg("-c").arg(cmd).output() { + if let Err(e) = Command::new("/bin/sh").arg("-c").arg(cmd).spawn() { eprintln!("{}", e); } }