From 0ed57b3896c21f0fe3a8b43ac385db2626ddcd86 Mon Sep 17 00:00:00 2001 From: Remi Rampin Date: Tue, 30 Jun 2015 15:21:40 -0400 Subject: [PATCH 1/8] Fix simple typo in mv.rs --- src/mv/mv.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/mv/mv.rs b/src/mv/mv.rs index cabb0ec0a..839a1f474 100644 --- a/src/mv/mv.rs +++ b/src/mv/mv.rs @@ -165,12 +165,11 @@ pub fn uumain(args: Vec) -> 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 { From 66a40d555de5d9308ba2578e836591fb5e6dafa1 Mon Sep 17 00:00:00 2001 From: Remi Rampin Date: Tue, 30 Jun 2015 15:23:55 -0400 Subject: [PATCH 2/8] Update timeout.rs to new io --- src/timeout/timeout.rs | 51 ++++++++++++++++++------------------------ 1 file changed, 22 insertions(+), 29 deletions(-) diff --git a/src/timeout/timeout.rs b/src/timeout/timeout.rs index 6a8a5e479..ea65bf8c2 100644 --- a/src/timeout/timeout.rs +++ b/src/timeout/timeout.rs @@ -1,5 +1,4 @@ #![crate_name = "timeout"] -#![feature(collections, core, io, old_io, rustc_private)] /* * This file is part of the uutils coreutils package. @@ -13,8 +12,9 @@ extern crate getopts; extern crate libc; -use std::old_io::{PathDoesntExist, FileNotFound}; -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] @@ -38,15 +38,14 @@ static ERR_EXIT_STATUS: i32 = 125; pub fn uumain(args: Vec) -> i32 { let program = args[0].clone(); - let opts = [ - getopts::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"), - getopts::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"), - getopts::optflag("h", "help", "display this help and exit"), - getopts::optflag("V", "version", "output version information and exit") - ]; - let matches = match getopts::getopts(args.tail(), &opts) { + let mut opts = getopts::Options::new(); + opts.optflag("", "preserve-status", "exit with the same status as COMMAND, even when the command times 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"); + opts.optopt("k", "kill-after", "also send a KILL signal if COMMAND is still running this long after the initial signal was sent", "DURATION"); + 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"); + opts.optflag("h", "help", "display this help and exit"); + opts.optflag("V", "version", "output version information and exit"); + let matches = match opts.parse(&args[1..]) { Ok(m) => m, Err(f) => { crash!(ERR_EXIT_STATUS, "{}", f) @@ -58,7 +57,7 @@ pub fn uumain(args: Vec) -> 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 +68,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 time::from_str(&tstr) { Ok(time) => time, Err(f) => { show_error!("{}", f); @@ -79,7 +78,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 +87,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 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 +105,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)) - .stdout(InheritFd(1)) - .stderr(InheritFd(2)) + .stdin(Stdio::inherit()) + .stdout(Stdio::inherit()) + .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 { @@ -124,20 +123,14 @@ 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() { - Ok(status) => match status { - ExitStatus(stat) => stat as i32, - ExitSignal(stat) => stat as i32 - }, + Ok(status) => status.code().unwrap_or_else(|| status.signal().unwrap()), 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 { - ExitStatus(stat) => stat as i32, - ExitSignal(stat) => stat as i32 - } + status.code().unwrap_or_else(|| status.signal().unwrap()) } else { 124 } From 28302555b8cc980b4c263d7e471851fddf94e75a Mon Sep 17 00:00:00 2001 From: Remi Rampin Date: Tue, 30 Jun 2015 15:33:17 -0400 Subject: [PATCH 3/8] Replace missing signal() by libc kill() directly --- src/timeout/timeout.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/timeout/timeout.rs b/src/timeout/timeout.rs index ea65bf8c2..e1dc2798c 100644 --- a/src/timeout/timeout.rs +++ b/src/timeout/timeout.rs @@ -1,4 +1,5 @@ #![crate_name = "timeout"] +#![feature(process_id)] /* * This file is part of the uutils coreutils package. @@ -12,6 +13,7 @@ extern crate getopts; extern crate libc; +use libc::pid_t; use std::io::{ErrorKind, Write}; use std::process::{Command, Stdio}; use std::os::unix::process::ExitStatusExt; @@ -125,7 +127,9 @@ fn timeout(cmdname: &str, args: &[String], duration: f64, signal: usize, kill_af match process.wait() { Ok(status) => status.code().unwrap_or_else(|| status.signal().unwrap()), Err(_) => { - return_if_err!(ERR_EXIT_STATUS, process.signal(signal as isize)); + if unsafe { libc::funcs::posix88::signal::kill(process.id() as pid_t, signal as i32) } != 0 { + return ERR_EXIT_STATUS; + } process.set_timeout(Some((kill_after * 1000f64) as u64)); match process.wait() { Ok(status) => { @@ -140,7 +144,9 @@ fn timeout(cmdname: &str, args: &[String], duration: f64, signal: usize, kill_af // 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)); + if unsafe { libc::funcs::posix88::signal::kill(process.id() as pid_t, signals::signal_by_name_or_value("KILL").unwrap() as i32) } != 0 { + return ERR_EXIT_STATUS; + } process.set_timeout(None); return_if_err!(ERR_EXIT_STATUS, process.wait()); 137 From 13f5994e15d5a85c53042bc3c0d2e9a578950aad Mon Sep 17 00:00:00 2001 From: Remi Rampin Date: Tue, 30 Jun 2015 16:17:02 -0400 Subject: [PATCH 4/8] Rename time.rs to parse_time.rs Conflicts with crate time. --- src/common/{time.rs => parse_time.rs} | 0 src/sleep/sleep.rs | 6 +++--- src/timeout/timeout.rs | 8 ++++---- 3 files changed, 7 insertions(+), 7 deletions(-) rename src/common/{time.rs => parse_time.rs} (100%) diff --git a/src/common/time.rs b/src/common/parse_time.rs similarity index 100% rename from src/common/time.rs rename to src/common/parse_time.rs diff --git a/src/sleep/sleep.rs b/src/sleep/sleep.rs index d75ac59fe..70abc233e 100644 --- a/src/sleep/sleep.rs +++ b/src/sleep/sleep.rs @@ -20,8 +20,8 @@ use std::u32::MAX as U32_MAX; #[macro_use] mod util; -#[path = "../common/time.rs"] -mod time; +#[path = "../common/parse_time.rs"] +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) { 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), }); diff --git a/src/timeout/timeout.rs b/src/timeout/timeout.rs index e1dc2798c..7c4f7f44e 100644 --- a/src/timeout/timeout.rs +++ b/src/timeout/timeout.rs @@ -22,8 +22,8 @@ use std::os::unix::process::ExitStatusExt; #[macro_use] mod util; -#[path = "../common/time.rs"] -mod time; +#[path = "../common/parse_time.rs"] +mod parse_time; #[path = "../common/signals.rs"] mod signals; @@ -70,7 +70,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) { + Some(tstr) => match parse_time::from_str(&tstr) { Ok(time) => time, Err(f) => { show_error!("{}", f); @@ -89,7 +89,7 @@ Usage: }, None => signals::signal_by_name_or_value("TERM").unwrap() }; - let duration = match time::from_str(&matches.free[0]) { + let duration = match parse_time::from_str(&matches.free[0]) { Ok(time) => time, Err(f) => { show_error!("{}", f); From cce3d5171dc80cfe4fefd410320f5ce7238a0df1 Mon Sep 17 00:00:00 2001 From: Remi Rampin Date: Tue, 30 Jun 2015 17:53:12 -0400 Subject: [PATCH 5/8] ChildExt for send_signal() and wait_or_timeout() --- src/common/process.rs | 140 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 140 insertions(+) create mode 100644 src/common/process.rs diff --git a/src/common/process.rs b/src/common/process.rs new file mode 100644 index 000000000..5895904b3 --- /dev/null +++ b/src/common/process.rs @@ -0,0 +1,140 @@ +/* + * This file is part of the uutils coreutils package. + * + * (c) Maciej Dziardziel + * + * 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 { + match *self { + ExitStatus::Code(code) => Some(code), + _ => None, + } + } + + pub fn signal(&self) -> Option { + 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_ms: usize) -> io::Result>; +} + +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_ms: usize) -> io::Result> { + // 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::>), + 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::milliseconds(timeout_ms 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> into Result> + match exitstatus.take() { + Some(r) => match r { + Ok(s) => Ok(Some(s)), + Err(e) => Err(e), + }, + None => panic!(), + } + } +} From d806ab48093fcd44fd1cf6fbf8bdab51068f982d Mon Sep 17 00:00:00 2001 From: Remi Rampin Date: Tue, 30 Jun 2015 17:58:34 -0400 Subject: [PATCH 6/8] Use ChildExt to kill from timeout.rs --- src/timeout/deps.mk | 1 + src/timeout/timeout.rs | 13 +++++++------ 2 files changed, 8 insertions(+), 6 deletions(-) create mode 100644 src/timeout/deps.mk diff --git a/src/timeout/deps.mk b/src/timeout/deps.mk new file mode 100644 index 000000000..b6534caec --- /dev/null +++ b/src/timeout/deps.mk @@ -0,0 +1 @@ +DEPLIBS += time diff --git a/src/timeout/timeout.rs b/src/timeout/timeout.rs index 7c4f7f44e..2434eba6a 100644 --- a/src/timeout/timeout.rs +++ b/src/timeout/timeout.rs @@ -12,6 +12,7 @@ extern crate getopts; extern crate libc; +extern crate time; use libc::pid_t; use std::io::{ErrorKind, Write}; @@ -28,6 +29,10 @@ 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; } @@ -127,9 +132,7 @@ fn timeout(cmdname: &str, args: &[String], duration: f64, signal: usize, kill_af match process.wait() { Ok(status) => status.code().unwrap_or_else(|| status.signal().unwrap()), Err(_) => { - if unsafe { libc::funcs::posix88::signal::kill(process.id() as pid_t, signal as i32) } != 0 { - return ERR_EXIT_STATUS; - } + return_if_err!(ERR_EXIT_STATUS, process.send_signal(signal)); process.set_timeout(Some((kill_after * 1000f64) as u64)); match process.wait() { Ok(status) => { @@ -144,9 +147,7 @@ fn timeout(cmdname: &str, args: &[String], duration: f64, signal: usize, kill_af // XXX: this may not be right return 124; } - if unsafe { libc::funcs::posix88::signal::kill(process.id() as pid_t, signals::signal_by_name_or_value("KILL").unwrap() as i32) } != 0 { - return ERR_EXIT_STATUS; - } + 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 From dc40480e6e38f9c03b22855a165adc2b32907630 Mon Sep 17 00:00:00 2001 From: Remi Rampin Date: Tue, 30 Jun 2015 18:39:25 -0400 Subject: [PATCH 7/8] Switch wait_or_timeout() to f64 --- src/common/process.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/common/process.rs b/src/common/process.rs index 5895904b3..1a90aa3c2 100644 --- a/src/common/process.rs +++ b/src/common/process.rs @@ -73,7 +73,7 @@ pub trait ChildExt { 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_ms: usize) -> io::Result>; + fn wait_or_timeout(&mut self, timeout: f64) -> io::Result>; } impl ChildExt for Child { @@ -86,7 +86,7 @@ impl ChildExt for Child { } } - fn wait_or_timeout(&mut self, timeout_ms: usize) -> io::Result> { + fn wait_or_timeout(&mut self, timeout: f64) -> io::Result> { // The result will be written to that Option, protected by a Mutex // Then the Condvar will be signaled let state = Arc::new(( @@ -118,7 +118,9 @@ impl ChildExt for Child { 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::milliseconds(timeout_ms as i64); + 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 { From 0691965d6334ca3ef12400444b43950f8fa8c1af Mon Sep 17 00:00:00 2001 From: Remi Rampin Date: Tue, 30 Jun 2015 18:39:41 -0400 Subject: [PATCH 8/8] Use ChildExt to wait from timeout.rs --- src/timeout/timeout.rs | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/src/timeout/timeout.rs b/src/timeout/timeout.rs index 2434eba6a..6b3a43b76 100644 --- a/src/timeout/timeout.rs +++ b/src/timeout/timeout.rs @@ -128,31 +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() { - Ok(status) => status.code().unwrap_or_else(|| status.signal().unwrap()), - Err(_) => { + match process.wait_or_timeout(duration) { + Ok(Some(status)) => status.code().unwrap_or_else(|| status.signal().unwrap()), + Ok(None) => { return_if_err!(ERR_EXIT_STATUS, process.send_signal(signal)); - process.set_timeout(Some((kill_after * 1000f64) as u64)); - match process.wait() { - Ok(status) => { + match process.wait_or_timeout(kill_after) { + Ok(Some(status)) => { if preserve_status { status.code().unwrap_or_else(|| status.signal().unwrap()) } else { 124 } - } - Err(_) => { + }, + Ok(None) => { if kill_after == 0f64 { // XXX: this may not be right return 124; } 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 + }, } }