Stop script-vars and close windows on eww kill

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"

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"

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),

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<PrimitiveValue> {
+        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<PrimitiveValue> {
         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<PrimitiveValue> {
+    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))
+}

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<PrimitiveValue> {
-    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))
-}

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(())
 }
 

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(), var.run_once()?))?;
-                            evt_send.send(app::EwwCommand::UpdateVar(var.name.clone(), output))?;
                         };
                         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.
+        let mut script_var_processes: HashMap<_, ScriptVarProcess> = HashMap::new();
-        // should be possibel to just keep a BufReader in TailVarProcess directly
-        let mut command_children = Vec::new();
-        let mut command_out_handles: HashMap<_, BufReader<filedescriptor::FileDescriptor>> = 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()?));
+                    sources.register(var.name.clone(), process.stdout_reader.get_ref(), popol::interest::READ);
-                    command_children.push(process);
+                    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);
             }
-
+            script_var_processes.values().for_each(|process| process.kill());
-            // stop child processes after exit
-            command_children.drain(..).for_each(|process| process.kill());
         });
         self.tail_handler_thread = Some(thread_handle);
         Ok(())
@@ -145,42 +137,68 @@ impl Drop for ScriptVarHandler {
     }
 }
 
-#[derive(Debug)]
+pub mod script_var_process {
-struct TailVarProcess {
+    use anyhow::*;
-    pid: nix::unistd::Pid,
+    use nix::{
-    out_fd: filedescriptor::FileDescriptor,
+        sys::{signal, wait},
-}
+        unistd::Pid,
+    };
+    use std::{io::BufReader, process::Stdio, sync::Mutex};
 
-impl TailVarProcess {
+    use crate::util;
-    pub fn run(command: &str) -> Result<Self> {
-        use nix::unistd::*;
 
-        let pipe = filedescriptor::Pipe::new()?;
+    lazy_static::lazy_static! {
-
+        static ref SCRIPT_VAR_CHILDREN: Mutex<Vec<u32>> = 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);
+    fn terminate_pid(pid: u32) {
-                setpgid(child, child)?;
+        println!("Killing pid: {}", pid);
-                Ok(TailVarProcess {
+        let result = signal::kill(Pid::from_raw(pid as i32), signal::SIGTERM);
-                    pid: child,
+        util::print_result_err("While killing tail-var child processes", &result);
-                    out_fd: pipe.read,
+        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<std::process::ChildStdout>,
+    }
+
+    impl ScriptVarProcess {
+        pub(super) fn run(command: &str) -> Result<Self> {
+            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) {
+    impl Drop for ScriptVarProcess {
-        let result = nix::sys::signal::kill(self.pid, Some(nix::sys::signal::SIGTERM));
+        fn drop(&mut self) {
-        util::print_result_err("Killing tail-var process", &result);
+            self.kill();
+        }
     }
 }

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<T: std::fmt::Display>(cmd: &str, arg: T) {
+pub(self) fn run_command<T: std::fmt::Display>(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);
     }
 }