diff options
author | Thomas O'Donnell <andytom@users.noreply.github.com> | 2021-01-21 22:59:14 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-01-21 22:59:14 +0100 |
commit | 5cf1c8a7bdc9cd385d37dd1310b09099a966796d (patch) | |
tree | 81f66e27246c6f1c3df9c5b274956d7bf1c6943f | |
parent | b5fd517972ffc3d0b2c108b0d3cc4218e10ce648 (diff) |
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
-rw-r--r-- | Cargo.lock | 16 | ||||
-rw-r--r-- | Cargo.toml | 1 | ||||
-rw-r--r-- | src/utils.rs | 51 |
3 files changed, 59 insertions, 9 deletions
diff --git a/Cargo.lock b/Cargo.lock index d3f70ee75..dbc09ff36 100644 --- a/Cargo.lock +++ b/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" @@ -929,6 +929,17 @@ dependencies = [ ] [[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" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -1281,6 +1292,7 @@ dependencies = [ "path-slash", "pest", "pest_derive", + "process_control", "quick-xml", "rand 0.8.2", "rayon", diff --git a/Cargo.toml b/Cargo.toml index a6c26a345..da3af3a91 100644 --- a/Cargo.toml +++ b/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"] } diff --git a/src/utils.rs b/src/utils.rs index b866a2bde..68435288c 100644 --- a/src/utils.rs +++ b/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::time::Instant; +use std::process::{Command, Stdio}; +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) => { - log::trace!("Unable to find {:?} in PATH, {:?}", cmd, e); + Err(error) => { + 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; @@ -363,6 +391,15 @@ mod tests { } #[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 let test1 = "\x1b]330;mlol\x1b]0m"; // BEGIN: \x1b END: m |