diff --git a/src/libnative/io/process.rs b/src/libnative/io/process.rs index 28f0817670b8f..463f9f8bedded 100644 --- a/src/libnative/io/process.rs +++ b/src/libnative/io/process.rs @@ -134,6 +134,18 @@ impl rtio::RtioProcess for Process { } fn kill(&mut self, signum: int) -> Result<(), io::IoError> { + // On linux (and possibly other unices), a process that has exited will + // continue to accept signals because it is "defunct". The delivery of + // signals will only fail once the child has been reaped. For this + // reason, if the process hasn't exited yet, then we attempt to collect + // their status with WNOHANG. + if self.exit_code.is_none() { + match waitpid_nowait(self.pid) { + Some(code) => { self.exit_code = Some(code); } + None => {} + } + } + // if the process has finished, and therefore had waitpid called, // and we kill it, then on unix we might ending up killing a // newer process that happens to have the same (re-used) id @@ -662,6 +674,31 @@ fn free_handle(_handle: *()) { // unix has no process handle object, just a pid } +#[cfg(unix)] +fn translate_status(status: c_int) -> p::ProcessExit { + #[cfg(target_os = "linux")] + #[cfg(target_os = "android")] + mod imp { + pub fn WIFEXITED(status: i32) -> bool { (status & 0xff) == 0 } + pub fn WEXITSTATUS(status: i32) -> i32 { (status >> 8) & 0xff } + pub fn WTERMSIG(status: i32) -> i32 { status & 0x7f } + } + + #[cfg(target_os = "macos")] + #[cfg(target_os = "freebsd")] + mod imp { + pub fn WIFEXITED(status: i32) -> bool { (status & 0x7f) == 0 } + pub fn WEXITSTATUS(status: i32) -> i32 { status >> 8 } + pub fn WTERMSIG(status: i32) -> i32 { status & 0o177 } + } + + if imp::WIFEXITED(status) { + p::ExitStatus(imp::WEXITSTATUS(status) as int) + } else { + p::ExitSignal(imp::WTERMSIG(status) as int) + } +} + /** * Waits for a process to exit and returns the exit code, failing * if there is no process with the specified id. @@ -723,33 +760,31 @@ fn waitpid(pid: pid_t) -> p::ProcessExit { #[cfg(unix)] fn waitpid_os(pid: pid_t) -> p::ProcessExit { use std::libc::funcs::posix01::wait; - - #[cfg(target_os = "linux")] - #[cfg(target_os = "android")] - mod imp { - pub fn WIFEXITED(status: i32) -> bool { (status & 0xff) == 0 } - pub fn WEXITSTATUS(status: i32) -> i32 { (status >> 8) & 0xff } - pub fn WTERMSIG(status: i32) -> i32 { status & 0x7f } + let mut status = 0 as c_int; + match retry(|| unsafe { wait::waitpid(pid, &mut status, 0) }) { + -1 => fail!("unknown waitpid error: {}", super::last_error()), + _ => translate_status(status), } + } +} - #[cfg(target_os = "macos")] - #[cfg(target_os = "freebsd")] - mod imp { - pub fn WIFEXITED(status: i32) -> bool { (status & 0x7f) == 0 } - pub fn WEXITSTATUS(status: i32) -> i32 { status >> 8 } - pub fn WTERMSIG(status: i32) -> i32 { status & 0o177 } - } +fn waitpid_nowait(pid: pid_t) -> Option { + return waitpid_os(pid); + // This code path isn't necessary on windows + #[cfg(windows)] + fn waitpid_os(_pid: pid_t) -> Option { None } + + #[cfg(unix)] + fn waitpid_os(pid: pid_t) -> Option { + use std::libc::funcs::posix01::wait; let mut status = 0 as c_int; - match retry(|| unsafe { wait::waitpid(pid, &mut status, 0) }) { - -1 => fail!("unknown waitpid error: {}", super::last_error()), - _ => { - if imp::WIFEXITED(status) { - p::ExitStatus(imp::WEXITSTATUS(status) as int) - } else { - p::ExitSignal(imp::WTERMSIG(status) as int) - } - } + match retry(|| unsafe { + wait::waitpid(pid, &mut status, libc::WNOHANG) + }) { + n if n == pid => Some(translate_status(status)), + 0 => None, + n => fail!("unknown waitpid error `{}`: {}", n, super::last_error()), } } } diff --git a/src/libstd/io/process.rs b/src/libstd/io/process.rs index 2da79eba7bc03..4f4d2d793f6b0 100644 --- a/src/libstd/io/process.rs +++ b/src/libstd/io/process.rs @@ -331,7 +331,9 @@ impl Process { /// signals (SIGTERM/SIGKILL/SIGINT) are translated to `TerminateProcess`. /// /// Additionally, a signal number of 0 can check for existence of the target - /// process. + /// process. Note, though, that on some platforms signals will continue to + /// be successfully delivered if the child has exited, but not yet been + /// reaped. pub fn kill(id: libc::pid_t, signal: int) -> IoResult<()> { LocalIo::maybe_raise(|io| io.kill(id, signal)) } @@ -342,8 +344,16 @@ impl Process { /// Sends the specified signal to the child process, returning whether the /// signal could be delivered or not. /// - /// Note that this is purely a wrapper around libuv's `uv_process_kill` - /// function. + /// Note that signal 0 is interpreted as a poll to check whether the child + /// process is still alive or not. If an error is returned, then the child + /// process has exited. + /// + /// On some unix platforms signals will continue to be received after a + /// child has exited but not yet been reaped. In order to report the status + /// of signal delivery correctly, unix implementations may invoke + /// `waitpid()` with `WNOHANG` in order to reap the child as necessary. + /// + /// # Errors /// /// If the signal delivery fails, the corresponding error is returned. pub fn signal(&mut self, signal: int) -> IoResult<()> { @@ -833,4 +843,17 @@ mod tests { p.signal_kill().unwrap(); assert!(!p.wait().success()); }) + + iotest!(fn test_zero() { + let mut p = sleeper(); + p.signal_kill().unwrap(); + for _ in range(0, 20) { + if p.signal(0).is_err() { + assert!(!p.wait().success()); + return + } + timer::sleep(100); + } + fail!("never saw the child go away"); + }) } diff --git a/src/libstd/libc.rs b/src/libstd/libc.rs index 42221f074491d..972002fe34e23 100644 --- a/src/libstd/libc.rs +++ b/src/libstd/libc.rs @@ -2356,6 +2356,8 @@ pub mod consts { pub static CLOCK_REALTIME: c_int = 0; pub static CLOCK_MONOTONIC: c_int = 1; + + pub static WNOHANG: c_int = 1; } pub mod posix08 { } @@ -2802,6 +2804,8 @@ pub mod consts { pub static CLOCK_REALTIME: c_int = 0; pub static CLOCK_MONOTONIC: c_int = 4; + + pub static WNOHANG: c_int = 1; } pub mod posix08 { } @@ -3187,6 +3191,8 @@ pub mod consts { pub static PTHREAD_CREATE_JOINABLE: c_int = 1; pub static PTHREAD_CREATE_DETACHED: c_int = 2; pub static PTHREAD_STACK_MIN: size_t = 8192; + + pub static WNOHANG: c_int = 1; } pub mod posix08 { } diff --git a/src/test/run-pass/core-run-destroy.rs b/src/test/run-pass/core-run-destroy.rs index 87d3c337bddb0..83232b408731d 100644 --- a/src/test/run-pass/core-run-destroy.rs +++ b/src/test/run-pass/core-run-destroy.rs @@ -22,6 +22,12 @@ extern crate native; extern crate green; extern crate rustuv; +use std::io::Process; + +macro_rules! succeed( ($e:expr) => ( + match $e { Ok(..) => {}, Err(e) => fail!("failure: {}", e) } +) ) + macro_rules! iotest ( { fn $name:ident() $b:block $($a:attr)* } => ( mod $name { @@ -53,28 +59,29 @@ fn start(argc: int, argv: **u8) -> int { } iotest!(fn test_destroy_once() { - #[cfg(not(target_os="android"))] - static mut PROG: &'static str = "echo"; - - #[cfg(target_os="android")] - static mut PROG: &'static str = "ls"; // android don't have echo binary - - let mut p = unsafe {Process::new(PROG, []).unwrap()}; - p.signal_exit().unwrap(); // this shouldn't crash (and nor should the destructor) + let mut p = sleeper(); + match p.signal_exit() { + Ok(()) => {} + Err(e) => fail!("error: {}", e), + } }) +#[cfg(unix)] +pub fn sleeper() -> Process { + Process::new("sleep", [~"1000"]).unwrap() +} +#[cfg(windows)] +pub fn sleeper() -> Process { + // There's a `timeout` command on windows, but it doesn't like having + // its output piped, so instead just ping ourselves a few times with + // gaps inbetweeen so we're sure this process is alive for awhile + Process::new("ping", [~"127.0.0.1", ~"-n", ~"1000"]).unwrap() +} + iotest!(fn test_destroy_twice() { - #[cfg(not(target_os="android"))] - static mut PROG: &'static str = "echo"; - #[cfg(target_os="android")] - static mut PROG: &'static str = "ls"; // android don't have echo binary - - let mut p = match unsafe{Process::new(PROG, [])} { - Ok(p) => p, - Err(e) => fail!("wut: {}", e), - }; - p.signal_exit().unwrap(); // this shouldnt crash... - p.signal_exit().unwrap(); // ...and nor should this (and nor should the destructor) + let mut p = sleeper(); + succeed!(p.signal_exit()); // this shouldnt crash... + let _ = p.signal_exit(); // ...and nor should this (and nor should the destructor) }) pub fn test_destroy_actually_kills(force: bool) {