Skip to content

Commit 6958c54

Browse files
committed
fix(timeout): Fix wrong signal handling
Fixed an issue where timeout would not force-kill due to wrong error handling on signal(), and also made error handle more robust for internal errors.
1 parent 7742133 commit 6958c54

File tree

2 files changed

+13
-16
lines changed

2 files changed

+13
-16
lines changed

src/uu/timeout/src/timeout.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -250,8 +250,7 @@ fn wait_or_kill_process(
250250
process.wait()?;
251251
Ok(ExitStatus::SignalSent(signal).into())
252252
}
253-
Ok(WaitOrTimeoutRet::CustomSignaled) => unreachable!(), // We did not set it up.
254-
Err(_) => Ok(ExitStatus::TimeoutFailed.into()),
253+
Ok(WaitOrTimeoutRet::CustomSignaled) | Err(_) => Ok(ExitStatus::TimeoutFailed.into()),
255254
}
256255
}
257256

src/uucore/src/lib/features/process.rs

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ use crate::pipes::pipe;
1616
use ::{
1717
nix::sys::select::FdSet,
1818
nix::sys::select::select,
19-
nix::sys::signal::Signal,
2019
nix::sys::signal::{self, signal},
2120
nix::sys::time::TimeVal,
2221
std::fs::File,
@@ -31,6 +30,7 @@ use ::{
3130
use libc::{c_int, gid_t, pid_t, uid_t};
3231
#[cfg(not(target_os = "redox"))]
3332
use nix::errno::Errno;
33+
use nix::sys::signal::Signal;
3434
use std::{io, process::Child};
3535

3636
/// Not all platforms support uncapped times (read: macOS). However,
@@ -144,23 +144,21 @@ pub enum WaitOrTimeoutRet {
144144

145145
impl ChildExt for Child {
146146
fn send_signal(&mut self, signal: usize) -> io::Result<()> {
147-
if unsafe { libc::kill(self.id() as pid_t, signal as i32) } == 0 {
148-
Ok(())
149-
} else {
150-
Err(io::Error::last_os_error())
151-
}
147+
nix::Error::result(unsafe { libc::kill(self.id() as pid_t, signal as i32) })?;
148+
Ok(())
152149
}
153150

154151
fn send_signal_group(&mut self, signal: usize) -> io::Result<()> {
155-
// Ignore the signal, so we don't go into a signal loop.
156-
if unsafe { libc::signal(signal as i32, libc::SIG_IGN) } == usize::MAX {
157-
return Err(io::Error::last_os_error());
158-
}
159-
if unsafe { libc::kill(0, signal as i32) } == 0 {
160-
Ok(())
161-
} else {
162-
Err(io::Error::last_os_error())
152+
// Ignore the signal, so we don't go into a signal loop. Some signals will fail
153+
// the call because they cannot be ignored, but they insta-kill so it's fine.
154+
if signal != Signal::SIGSTOP as _ && signal != Signal::SIGKILL as _ {
155+
let err = unsafe { libc::signal(signal as i32, libc::SIG_IGN) } == usize::MAX;
156+
if err {
157+
return Err(io::Error::last_os_error());
158+
}
163159
}
160+
nix::Error::result(unsafe { libc::kill(0, signal as i32) })?;
161+
Ok(())
164162
}
165163

166164
#[cfg(feature = "pipes")]

0 commit comments

Comments
 (0)