mirror of
https://github.com/uutils/coreutils
synced 2024-10-15 12:24:09 +00:00
Merge pull request #4315 from miles170/issue-4176-fix-timeout
timeout: fix subprocess is never terminated
This commit is contained in:
commit
dad474bfa0
|
@ -196,6 +196,22 @@ fn report_if_verbose(signal: usize, cmd: &str, verbose: bool) {
|
|||
}
|
||||
}
|
||||
|
||||
fn send_signal(process: &mut Child, signal: usize, foreground: bool) {
|
||||
// NOTE: GNU timeout doesn't check for errors of signal.
|
||||
// The subprocess might have exited just after the timeout.
|
||||
// Sending a signal now would return "No such process", but we should still try to kill the children.
|
||||
_ = process.send_signal(signal);
|
||||
if !foreground {
|
||||
_ = process.send_signal_group(signal);
|
||||
let kill_signal = signal_by_name_or_value("KILL").unwrap();
|
||||
let continued_signal = signal_by_name_or_value("CONT").unwrap();
|
||||
if signal != kill_signal && signal != continued_signal {
|
||||
_ = process.send_signal(continued_signal);
|
||||
_ = process.send_signal_group(continued_signal);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Wait for a child process and send a kill signal if it does not terminate.
|
||||
///
|
||||
/// This function waits for the child `process` for the time period
|
||||
|
@ -217,10 +233,11 @@ fn report_if_verbose(signal: usize, cmd: &str, verbose: bool) {
|
|||
/// If there is a problem sending the `SIGKILL` signal or waiting for
|
||||
/// the process after that signal is sent.
|
||||
fn wait_or_kill_process(
|
||||
mut process: Child,
|
||||
process: &mut Child,
|
||||
cmd: &str,
|
||||
duration: Duration,
|
||||
preserve_status: bool,
|
||||
foreground: bool,
|
||||
verbose: bool,
|
||||
) -> std::io::Result<i32> {
|
||||
match process.wait_or_timeout(duration) {
|
||||
|
@ -234,7 +251,7 @@ fn wait_or_kill_process(
|
|||
Ok(None) => {
|
||||
let signal = signal_by_name_or_value("KILL").unwrap();
|
||||
report_if_verbose(signal, cmd, verbose);
|
||||
process.send_signal(signal)?;
|
||||
send_signal(process, signal, foreground);
|
||||
process.wait()?;
|
||||
Ok(ExitStatus::SignalSent(signal).into())
|
||||
}
|
||||
|
@ -300,7 +317,7 @@ fn timeout(
|
|||
|
||||
enable_pipe_errors()?;
|
||||
|
||||
let mut process = process::Command::new(&cmd[0])
|
||||
let process = &mut process::Command::new(&cmd[0])
|
||||
.args(&cmd[1..])
|
||||
.stdin(Stdio::inherit())
|
||||
.stdout(Stdio::inherit())
|
||||
|
@ -335,7 +352,7 @@ fn timeout(
|
|||
.into()),
|
||||
Ok(None) => {
|
||||
report_if_verbose(signal, &cmd[0], verbose);
|
||||
process.send_signal(signal)?;
|
||||
send_signal(process, signal, foreground);
|
||||
match kill_after {
|
||||
None => {
|
||||
if preserve_status {
|
||||
|
@ -350,6 +367,7 @@ fn timeout(
|
|||
&cmd[0],
|
||||
kill_after,
|
||||
preserve_status,
|
||||
foreground,
|
||||
verbose,
|
||||
) {
|
||||
Ok(status) => Err(status.into()),
|
||||
|
@ -363,11 +381,8 @@ fn timeout(
|
|||
}
|
||||
Err(_) => {
|
||||
// We're going to return ERR_EXIT_STATUS regardless of
|
||||
// whether `send_signal()` succeeds or fails, so just
|
||||
// ignore the return value.
|
||||
process
|
||||
.send_signal(signal)
|
||||
.map_err(|e| USimpleError::new(ExitStatus::TimeoutFailed.into(), format!("{e}")))?;
|
||||
// whether `send_signal()` succeeds or fails
|
||||
send_signal(process, signal, foreground);
|
||||
Err(ExitStatus::TimeoutFailed.into())
|
||||
}
|
||||
}
|
||||
|
|
|
@ -48,6 +48,9 @@ pub trait ChildExt {
|
|||
/// send the signal to an unrelated process that recycled the PID.
|
||||
fn send_signal(&mut self, signal: usize) -> io::Result<()>;
|
||||
|
||||
/// Send a signal to a process group.
|
||||
fn send_signal_group(&mut self, signal: usize) -> io::Result<()>;
|
||||
|
||||
/// Wait for a process to finish or return after the specified duration.
|
||||
/// A `timeout` of zero disables the timeout.
|
||||
fn wait_or_timeout(&mut self, timeout: Duration) -> io::Result<Option<ExitStatus>>;
|
||||
|
@ -62,6 +65,18 @@ impl ChildExt for Child {
|
|||
}
|
||||
}
|
||||
|
||||
fn send_signal_group(&mut self, signal: usize) -> io::Result<()> {
|
||||
// Ignore the signal, so we don't go into a signal loop.
|
||||
if unsafe { libc::signal(signal as i32, libc::SIG_IGN) } != 0 {
|
||||
return Err(io::Error::last_os_error());
|
||||
}
|
||||
if unsafe { libc::kill(0, signal as i32) } != 0 {
|
||||
Err(io::Error::last_os_error())
|
||||
} else {
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
fn wait_or_timeout(&mut self, timeout: Duration) -> io::Result<Option<ExitStatus>> {
|
||||
if timeout == Duration::from_micros(0) {
|
||||
return self.wait().map(Some);
|
||||
|
|
|
@ -138,3 +138,19 @@ fn test_kill_after_long() {
|
|||
.no_stdout()
|
||||
.no_stderr();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_kill_subprocess() {
|
||||
new_ucmd!()
|
||||
.args(&[
|
||||
// Make sure the CI can spawn the subprocess.
|
||||
"10",
|
||||
"sh",
|
||||
"-c",
|
||||
"sh -c \"trap 'echo xyz' TERM; sleep 30\"",
|
||||
])
|
||||
.fails()
|
||||
.code_is(124)
|
||||
.stdout_contains("xyz")
|
||||
.stderr_contains("Terminated");
|
||||
}
|
||||
|
|
Loading…
Reference in a new issue