Merge pull request #641 from remram44/fix-timeout

Fix timeout

src/common/process.rs

@@ -0,0 +1,142 @@
+/*
+ * This file is part of the uutils coreutils package.
+ *
+ * (c) Maciej Dziardziel <fiedzia@gmail.com>
+ *
+ * For the full copyright and license information, please view the LICENSE file
+ * that was distributed with this source code.
+ */
+
+#![feature(process_id)]
+
+extern crate libc;
+extern crate time;
+
+use libc::{c_int, pid_t};
+use std::fmt;
+use std::io;
+use std::process::Child;
+use std::sync::{Arc, Condvar, Mutex};
+use std::thread;
+use time::{Duration, get_time};
+
+// This is basically sys::unix::process::ExitStatus
+#[derive(PartialEq, Eq, Clone, Copy, Debug)]
+pub enum ExitStatus {
+    Code(i32),
+    Signal(i32),
+}
+
+impl ExitStatus {
+    fn from_status(status: c_int) -> ExitStatus {
+        if status & 0x7F != 0 { // WIFSIGNALED(status)
+            ExitStatus::Signal(status & 0x7F)
+        } else {
+            ExitStatus::Code(status & 0xFF00 >> 8)
+        }
+    }
+
+    pub fn success(&self) -> bool {
+        match *self {
+            ExitStatus::Code(code) => code == 0,
+            _ => false,
+        }
+    }
+
+    pub fn code(&self) -> Option<i32> {
+        match *self {
+            ExitStatus::Code(code) => Some(code),
+            _ => None,
+        }
+    }
+
+    pub fn signal(&self) -> Option<i32> {
+        match *self {
+            ExitStatus::Signal(code) => Some(code),
+            _ => None,
+        }
+    }
+}
+
+impl fmt::Display for ExitStatus {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match *self {
+            ExitStatus::Code(code) => write!(f, "exit code: {}", code),
+            ExitStatus::Signal(code) => write!(f, "exit code: {}", code),
+        }
+    }
+}
+
+/// Missing methods for Child objects
+pub trait ChildExt {
+    /// Send a signal to a Child process.
+    fn send_signal(&mut self, signal: usize) -> io::Result<()>;
+
+    /// Wait for a process to finish or return after the specified duration.
+    fn wait_or_timeout(&mut self, timeout: f64) -> io::Result<Option<ExitStatus>>;
+}
+
+impl ChildExt for Child {
+    fn send_signal(&mut self, signal: usize) -> io::Result<()> {
+        if unsafe { libc::funcs::posix88::signal::kill(self.id() as pid_t,
+                                                       signal as i32) } != 0 {
+            Err(io::Error::last_os_error())
+        } else {
+            Ok(())
+        }
+    }
+
+    fn wait_or_timeout(&mut self, timeout: f64) -> io::Result<Option<ExitStatus>> {
+        // The result will be written to that Option, protected by a Mutex
+        // Then the Condvar will be signaled
+        let state = Arc::new((
+            Mutex::new(Option::None::<io::Result<ExitStatus>>),
+            Condvar::new(),
+        ));
+
+        // Start the waiting thread
+        let state_th = state.clone();
+        let pid_th = self.id();
+        thread::spawn(move || {
+            let &(ref lock_th, ref cvar_th) = &*state_th;
+            // Child::wait() would need a &mut to self, can't use that...
+            // use waitpid() directly, with our own ExitStatus
+            let mut status: c_int = 0;
+            let r = unsafe { libc::waitpid(pid_th as i32, &mut status, 0) };
+            // Fill the Option and notify on the Condvar
+            let mut exitstatus_th = lock_th.lock().unwrap();
+            if r != pid_th as c_int {
+                *exitstatus_th = Some(Err(io::Error::last_os_error()));
+            } else {
+                let s = ExitStatus::from_status(status);
+                *exitstatus_th = Some(Ok(s));
+            }
+            cvar_th.notify_one();
+        });
+
+        // Main thread waits
+        let &(ref lock, ref cvar) = &*state;
+        let mut exitstatus = lock.lock().unwrap();
+        // Condvar::wait_timeout_ms() can wake too soon, in this case wait again
+        let target = get_time() +
+            Duration::seconds(timeout as i64) +
+            Duration::nanoseconds((timeout * 1.0e-6) as i64);
+        while exitstatus.is_none() {
+            let now = get_time();
+            if now >= target {
+                return Ok(None)
+            }
+            let ms = (target - get_time()).num_milliseconds() as u32;
+            exitstatus = cvar.wait_timeout_ms(exitstatus, ms).unwrap().0;
+        }
+
+        // Turn Option<Result<ExitStatus>> into Result<Option<ExitStatus>>
+        match exitstatus.take() {
+            Some(r) => match r {
+                Ok(s) => Ok(Some(s)),
+                Err(e) => Err(e),
+            },
+            None => panic!(),
+        }
+    }
+}

src/mv/mv.rs

@@ -165,12 +165,11 @@ pub fn uumain(args: Vec<String>) -> i32 {
 }
 
 fn help(usage: &str) {
-    let msg = format!("a0{} {1}\n\n \
+    println!("a0{} {1}\n\n \
                        Usage: {0} SOURCE DEST\n  \
                          or:  {0} SOURCE... DIRECTORY \
                        \n\
                        {2}", NAME, VERSION, usage);
-    println!("{}", msg);
 }
 
 fn exec(files: &[PathBuf], b: Behaviour) -> i32 {

src/sleep/sleep.rs

@@ -20,8 +20,8 @@ use std::u32::MAX as U32_MAX;
 #[macro_use]
 mod util;
 
-#[path = "../common/time.rs"]
+#[path = "../common/parse_time.rs"]
-mod time;
+mod parse_time;
 
 static NAME: &'static str = "sleep";
 static VERSION: &'static str = "1.0.0";
@@ -67,7 +67,7 @@ specified by the sum of their values.", NAME, VERSION);
 
 fn sleep(args: Vec<String>) {
     let sleep_time = args.iter().fold(0.0, |result, arg|
-        match time::from_str(&arg[..]) {
+        match parse_time::from_str(&arg[..]) {
             Ok(m) => m + result,
             Err(f) => crash!(1, "{}", f),
         });

src/timeout/deps.mk

@@ -0,0 +1 @@
+DEPLIBS += time

src/timeout/timeout.rs

@@ -1,5 +1,5 @@
 #![crate_name = "timeout"]
-#![feature(collections, core, io, old_io, rustc_private)]
+#![feature(process_id)]
 
 /*
  * This file is part of the uutils coreutils package.
@@ -12,20 +12,27 @@
 
 extern crate getopts;
 extern crate libc;
+extern crate time;
 
-use std::old_io::{PathDoesntExist, FileNotFound};
+use libc::pid_t;
-use std::old_io::process::{Command, ExitStatus, ExitSignal, InheritFd};
+use std::io::{ErrorKind, Write};
+use std::process::{Command, Stdio};
+use std::os::unix::process::ExitStatusExt;
 
 #[path = "../common/util.rs"]
 #[macro_use]
 mod util;
 
-#[path = "../common/time.rs"]
+#[path = "../common/parse_time.rs"]
-mod time;
+mod parse_time;
 
 #[path = "../common/signals.rs"]
 mod signals;
 
+#[path = "../common/process.rs"]
+mod process;
+use process::ChildExt;
+
 extern {
     pub fn setpgid(_: libc::pid_t, _: libc::pid_t) -> libc::c_int;
 }
@@ -38,15 +45,14 @@ static ERR_EXIT_STATUS: i32 = 125;
 pub fn uumain(args: Vec<String>) -> i32 {
     let program = args[0].clone();
 
-    let opts = [
+    let mut opts = getopts::Options::new();
-        getopts::optflag("", "preserve-status", "exit with the same status as COMMAND, even when the command times out"),
+    opts.optflag("", "preserve-status", "exit with the same status as COMMAND, even when the command times out");
-        getopts::optflag("", "foreground", "when not running timeout directly from a shell prompt, allow COMMAND to read from the TTY and get TTY signals; in this mode, children of COMMAND will not be timed out"),
+    opts.optflag("", "foreground", "when not running timeout directly from a shell prompt, allow COMMAND to read from the TTY and get TTY signals; in this mode, children of COMMAND will not be timed out");
-        getopts::optopt("k", "kill-after", "also send a KILL signal if COMMAND is still running this long after the initial signal was sent", "DURATION"),
+    opts.optopt("k", "kill-after", "also send a KILL signal if COMMAND is still running this long after the initial signal was sent", "DURATION");
-        getopts::optflag("s", "signal", "specify the signal to be sent on timeout; SIGNAL may be a name like 'HUP' or a number; see 'kill -l' for a list of signals"),
+    opts.optflag("s", "signal", "specify the signal to be sent on timeout; SIGNAL may be a name like 'HUP' or a number; see 'kill -l' for a list of signals");
-        getopts::optflag("h", "help", "display this help and exit"),
+    opts.optflag("h", "help", "display this help and exit");
-        getopts::optflag("V", "version", "output version information and exit")
+    opts.optflag("V", "version", "output version information and exit");
-    ];
+    let matches = match opts.parse(&args[1..]) {
-    let matches = match getopts::getopts(args.tail(), &opts) {
         Ok(m) => m,
         Err(f) => {
             crash!(ERR_EXIT_STATUS, "{}", f)
@@ -58,7 +64,7 @@ pub fn uumain(args: Vec<String>) -> i32 {
 Usage:
   {} [OPTION] DURATION COMMAND [ARG]...
 
-{}", NAME, VERSION, program, getopts::usage("Start COMMAND, and kill it if still running after DURATION.", &opts));
+{}", NAME, VERSION, program, &opts.usage("Start COMMAND, and kill it if still running after DURATION."));
     } else if matches.opt_present("version") {
         println!("{} {}", NAME, VERSION);
     } else if matches.free.len() < 2 {
@@ -69,7 +75,7 @@ Usage:
         let status = matches.opt_present("preserve-status");
         let foreground = matches.opt_present("foreground");
         let kill_after = match matches.opt_str("kill-after") {
-            Some(tstr) => match time::from_str(tstr.as_slice()) {
+            Some(tstr) => match parse_time::from_str(&tstr) {
                 Ok(time) => time,
                 Err(f) => {
                     show_error!("{}", f);
@@ -79,7 +85,7 @@ Usage:
             None => 0f64
         };
         let signal = match matches.opt_str("signal") {
-            Some(sigstr) => match signals::signal_by_name_or_value(sigstr.as_slice()) {
+            Some(sigstr) => match signals::signal_by_name_or_value(&sigstr) {
                 Some(sig) => sig,
                 None => {
                     show_error!("invalid signal '{}'", sigstr);
@@ -88,14 +94,14 @@ Usage:
             },
             None => signals::signal_by_name_or_value("TERM").unwrap()
         };
-        let duration = match time::from_str(matches.free[0].as_slice()) {
+        let duration = match parse_time::from_str(&matches.free[0]) {
             Ok(time) => time,
             Err(f) => {
                 show_error!("{}", f);
                 return ERR_EXIT_STATUS;
             }
         };
-        return timeout(matches.free[1].as_slice(), &matches.free[2..], duration, signal, kill_after, foreground, status);
+        return timeout(&matches.free[1], &matches.free[2..], duration, signal, kill_after, foreground, status);
     }
 
     0
@@ -106,14 +112,14 @@ fn timeout(cmdname: &str, args: &[String], duration: f64, signal: usize, kill_af
         unsafe { setpgid(0, 0) };
     }
     let mut process = match Command::new(cmdname).args(args)
-                                                 .stdin(InheritFd(0))
+                                                 .stdin(Stdio::inherit())
-                                                 .stdout(InheritFd(1))
+                                                 .stdout(Stdio::inherit())
-                                                 .stderr(InheritFd(2))
+                                                 .stderr(Stdio::inherit())
                                                  .spawn() {
         Ok(p) => p,
         Err(err) => {
             show_error!("failed to execute process: {}", err);
-            if err.kind == FileNotFound || err.kind == PathDoesntExist {
+            if err.kind() == ErrorKind::NotFound {
                 // XXX: not sure which to use
                 return 127;
             } else {
@@ -122,37 +128,33 @@ fn timeout(cmdname: &str, args: &[String], duration: f64, signal: usize, kill_af
             }
         }
     };
-    process.set_timeout(Some((duration * 1000f64) as u64));  // FIXME: this ignores the f64...
+    match process.wait_or_timeout(duration) {
-    match process.wait() {
+        Ok(Some(status)) => status.code().unwrap_or_else(|| status.signal().unwrap()),
-        Ok(status) => match status {
+        Ok(None) => {
-            ExitStatus(stat) => stat as i32,
+            return_if_err!(ERR_EXIT_STATUS, process.send_signal(signal));
-            ExitSignal(stat) => stat as i32
+            match process.wait_or_timeout(kill_after) {
-        },
+                Ok(Some(status)) => {
-        Err(_) => {
-            return_if_err!(ERR_EXIT_STATUS, process.signal(signal as isize));
-            process.set_timeout(Some((kill_after * 1000f64) as u64));
-            match process.wait() {
-                Ok(status) => {
                     if preserve_status {
-                        match status {
+                        status.code().unwrap_or_else(|| status.signal().unwrap())
-                            ExitStatus(stat) => stat as i32,
-                            ExitSignal(stat) => stat as i32
-                        }
                     } else {
                         124
                     }
-                }
+                },
-                Err(_) => {
+                Ok(None) => {
                     if kill_after == 0f64 {
                         // XXX: this may not be right
                         return 124;
                     }
-                    return_if_err!(ERR_EXIT_STATUS, process.signal(signals::signal_by_name_or_value("KILL").unwrap() as isize));
+                    return_if_err!(ERR_EXIT_STATUS, process.send_signal(signals::signal_by_name_or_value("KILL").unwrap()));
-                    process.set_timeout(None);
                     return_if_err!(ERR_EXIT_STATUS, process.wait());
                     137
-                }
+                },
+                Err(_) => return 124,
             }
-        }
+        },
+        Err(_) => {
+            return_if_err!(ERR_EXIT_STATUS, process.send_signal(signal));
+            ERR_EXIT_STATUS
+        },
     }
 }