From abfd00036785dd31c9619959ce6df812c9df4982 Mon Sep 17 00:00:00 2001 From: Ulrich Hornung Date: Tue, 12 Mar 2024 19:04:15 +0100 Subject: [PATCH 1/4] avoid sending twice same signal to child process --- src/uu/timeout/src/timeout.rs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/uu/timeout/src/timeout.rs b/src/uu/timeout/src/timeout.rs index 958bc647e..0de1e3706 100644 --- a/src/uu/timeout/src/timeout.rs +++ b/src/uu/timeout/src/timeout.rs @@ -202,14 +202,15 @@ 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); + match foreground { + true => _ = process.send_signal(signal), + false => { + _ = 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_group(continued_signal); + } } } } From 38b15c973653243d99cc5f814d0b23ea2f385ddd Mon Sep 17 00:00:00 2001 From: Ulrich Hornung Date: Tue, 12 Mar 2024 19:04:46 +0100 Subject: [PATCH 2/4] wait for child to stop after sending signal --- src/uu/timeout/src/timeout.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/uu/timeout/src/timeout.rs b/src/uu/timeout/src/timeout.rs index 0de1e3706..ccc97403d 100644 --- a/src/uu/timeout/src/timeout.rs +++ b/src/uu/timeout/src/timeout.rs @@ -343,8 +343,15 @@ fn timeout( send_signal(process, signal, foreground); match kill_after { None => { + let status = process.wait()?; if preserve_status { - Err(ExitStatus::SignalSent(signal).into()) + if let Some(ec) = status.code() { + Err(ec.into()) + } else if let Some(sc) = status.signal() { + Err(ExitStatus::SignalSent(sc.try_into().unwrap()).into()) + } else { + Err(ExitStatus::CommandTimedOut.into()) + } } else { Err(ExitStatus::CommandTimedOut.into()) } From a2a375d0ddf67a5847f714cf3e9ca019aa64a18e Mon Sep 17 00:00:00 2001 From: Ulrich Hornung Date: Tue, 12 Mar 2024 19:05:51 +0100 Subject: [PATCH 3/4] using other than TERM/KILL signal to see if --preserve-status works --- tests/by-util/test_timeout.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/by-util/test_timeout.rs b/tests/by-util/test_timeout.rs index 6c4a00eb5..f46cb0fec 100644 --- a/tests/by-util/test_timeout.rs +++ b/tests/by-util/test_timeout.rs @@ -93,6 +93,18 @@ fn test_preserve_status() { .no_stdout(); } +#[test] +fn test_preserve_status_even_when_send_signal() { + // When sending CONT signal, process doesn't get killed or stopped. + // So, expected result is success and code 0. + new_ucmd!() + .args(&["-s", "CONT", "--preserve-status", ".1", "sleep", "5"]) + .succeeds() + .code_is(0) + .no_stderr() + .no_stdout(); +} + #[test] fn test_dont_overflow() { new_ucmd!() From 7e22f9991346b13e0896a25d3572e28b63c2b05c Mon Sep 17 00:00:00 2001 From: Ulrich Hornung Date: Tue, 12 Mar 2024 19:08:51 +0100 Subject: [PATCH 4/4] remove second sh process to have timeout waiting for the right process --- tests/by-util/test_timeout.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/by-util/test_timeout.rs b/tests/by-util/test_timeout.rs index f46cb0fec..896065a20 100644 --- a/tests/by-util/test_timeout.rs +++ b/tests/by-util/test_timeout.rs @@ -163,10 +163,10 @@ fn test_kill_subprocess() { "10", "sh", "-c", - "sh -c \"trap 'echo xyz' TERM; sleep 30\"", + "trap 'echo inside_trap' TERM; sleep 30", ]) .fails() .code_is(124) - .stdout_contains("xyz") + .stdout_contains("inside_trap") .stderr_contains("Terminated"); }