perf(utils): Add timeout to `utils::exec_cmd` (#2171)

* perf(utils): Add timeout to `utils::exec_cmd`

This adds a timeout to any command executed using the `utils::exec_cmd`.
The initial time limit is hard coded to 500ms but if required we can
make this configurable. Have also switched the tests to be a bit more
granular on which systems they are ignored.

* Terminate the processes if they timeout

Cargo.lock

@@ -559,9 +559,9 @@ checksum = "830d08ce1d1d941e6b30645f1a0eb5643013d835ce3779a5fc208261dbe10f55"
 
 [[package]]
 name = "libc"
-version = "0.2.81"
+version = "0.2.82"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "1482821306169ec4d07f6aca392a4681f66c75c9918aa49641a2595db64053cb"
+checksum = "89203f3fba0a3795506acaad8ebce3c80c0af93f994d5a1d7a0b1eeb23271929"
 
 [[package]]
 name = "libdbus-sys"
@@ -928,6 +928,17 @@ dependencies = [
  "unicode-xid 0.2.1",
 ]
 
+[[package]]
+name = "process_control"
+version = "3.0.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "9cac0d971172c65b01208ef9567f52892fc89b1c4530c50e997c6f29da7ee961"
+dependencies = [
+ "crossbeam-channel",
+ "libc",
+ "winapi",
+]
+
 [[package]]
 name = "quick-xml"
 version = "0.20.0"
@@ -1281,6 +1292,7 @@ dependencies = [
  "path-slash",
  "pest",
  "pest_derive",
+ "process_control",
  "quick-xml",
  "rand 0.8.2",
  "rayon",

Cargo.toml

@@ -64,6 +64,7 @@ indexmap = "1.6.1"
 notify-rust = { version = "4.2.2", optional = true }
 semver = "0.11.0"
 which = "4.0.2"
+process_control = { version = "3.0.0", features = ["crossbeam-channel"] }
 
 # Optional/http:
 attohttpc = { version = "0.16.1", optional = true, default-features = false, features = ["tls", "form"] }

src/utils.rs

@@ -1,8 +1,9 @@
+use process_control::{ChildExt, Timeout};
 use std::fs::File;
 use std::io::{Read, Result};
 use std::path::Path;
-use std::process::Command;
+use std::process::{Command, Stdio};
-use std::time::Instant;
+use std::time::{Duration, Instant};
 
 use crate::context::Shell;
 
@@ -258,15 +259,31 @@ fn internal_exec_cmd(cmd: &str, args: &[&str]) -> Option<CommandOutput> {
             log::trace!("Using {:?} as {:?}", full_path, cmd);
             full_path
         }
-        Err(e) => {
+        Err(error) => {
-            log::trace!("Unable to find {:?} in PATH, {:?}", cmd, e);
+            log::trace!("Unable to find {:?} in PATH, {:?}", cmd, error);
             return None;
         }
     };
 
+    let time_limit = Duration::from_millis(500);
+
     let start = Instant::now();
-    match Command::new(full_path).args(args).output() {
+
-        Ok(output) => {
+    let process = match Command::new(full_path)
+        .args(args)
+        .stderr(Stdio::piped())
+        .stdout(Stdio::piped())
+        .spawn()
+    {
+        Ok(process) => process,
+        Err(error) => {
+            log::info!("Unable to run {:?}, {:?}", cmd, error);
+            return None;
+        }
+    };
+
+    match process.with_output_timeout(time_limit).terminating().wait() {
+        Ok(Some(output)) => {
             let stdout_string = String::from_utf8(output.stdout).unwrap();
             let stderr_string = String::from_utf8(output.stderr).unwrap();
 
@@ -287,6 +304,10 @@ fn internal_exec_cmd(cmd: &str, args: &[&str]) -> Option<CommandOutput> {
                 stderr: stderr_string,
             })
         }
+        Ok(None) => {
+            log::warn!("Executing command {:?} timed out", cmd);
+            None
+        }
         Err(error) => {
             log::info!("Executing command {:?} failed by: {:?}", cmd, error);
             None
@@ -295,7 +316,6 @@ fn internal_exec_cmd(cmd: &str, args: &[&str]) -> Option<CommandOutput> {
 }
 
 #[cfg(test)]
-#[cfg(not(windows))] // While the exec_cmd should work on Windows these tests assume a Unix-like environment.
 mod tests {
     use super::*;
 
@@ -310,7 +330,11 @@ mod tests {
         assert_eq!(result, expected)
     }
 
+    // While the exec_cmd should work on Windows some of these tests assume a Unix-like
+    // environment.
+
     #[test]
+    #[cfg(not(windows))]
     fn exec_no_output() {
         let result = internal_exec_cmd("true", &[]);
         let expected = Some(CommandOutput {
@@ -322,6 +346,7 @@ mod tests {
     }
 
     #[test]
+    #[cfg(not(windows))]
     fn exec_with_output_stdout() {
         let result = internal_exec_cmd("/bin/sh", &["-c", "echo hello"]);
         let expected = Some(CommandOutput {
@@ -333,6 +358,7 @@ mod tests {
     }
 
     #[test]
+    #[cfg(not(windows))]
     fn exec_with_output_stderr() {
         let result = internal_exec_cmd("/bin/sh", &["-c", "echo hello >&2"]);
         let expected = Some(CommandOutput {
@@ -344,6 +370,7 @@ mod tests {
     }
 
     #[test]
+    #[cfg(not(windows))]
     fn exec_with_output_both() {
         let result = internal_exec_cmd("/bin/sh", &["-c", "echo hello; echo world >&2"]);
         let expected = Some(CommandOutput {
@@ -355,6 +382,7 @@ mod tests {
     }
 
     #[test]
+    #[cfg(not(windows))]
     fn exec_with_non_zero_exit_code() {
         let result = internal_exec_cmd("false", &[]);
         let expected = None;
@@ -362,6 +390,15 @@ mod tests {
         assert_eq!(result, expected)
     }
 
+    #[test]
+    #[cfg(not(windows))]
+    fn exec_slow_command() {
+        let result = internal_exec_cmd("sleep", &["500"]);
+        let expected = None;
+
+        assert_eq!(result, expected)
+    }
+
     #[test]
     fn test_color_sequence_wrappers() {
         let test0 = "\x1b2mhellomynamekeyes\x1b2m"; // BEGIN: \x1b     END: m