diff --git a/Cargo.toml b/Cargo.toml index cb3a059..b12a18e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -5,14 +5,14 @@ authors = ["dylni"] edition = "2021" rust-version = "1.58.0" description = """ -Methods for ergonomically running processes with timeouts +Ergonomically run processes with limits """ readme = "README.md" repository = "https://github.com/dylni/process_control" license = "MIT OR Apache-2.0" keywords = ["kill", "process", "terminate", "timeout", "wait"] categories = ["concurrency", "os"] -exclude = [".*", "/rustfmt.toml", "/tests"] +exclude = [".*", "tests.rs", "/rustfmt.toml", "/src/bin", "/tests"] [package.metadata.docs.rs] rustc-args = ["--cfg", "process_control_docs_rs"] @@ -26,7 +26,7 @@ crossbeam-channel = { version = "0.5", optional = true } libc = "0.2.120" [target.'cfg(all(unix, any(target_os = "espidf", target_os = "horizon", target_os = "openbsd", target_os = "redox", target_os = "tvos", target_os = "vxworks")))'.dependencies] -signal-hook = { version = "0.3" } +signal-hook = "0.3" [target.'cfg(windows)'.dependencies] windows-sys = { version = "0.36", features = ["Win32_Foundation", "Win32_Security", "Win32_System_JobObjects", "Win32_System_Threading", "Win32_System_WindowsProgramming"] } diff --git a/src/control.rs b/src/control.rs index 2a86d30..1ccfce6 100644 --- a/src/control.rs +++ b/src/control.rs @@ -61,12 +61,11 @@ macro_rules! r#impl { impl$(<$lifetime>)? Control for $struct$(<$lifetime>)? { type Result = $return_type; - if_memory_limit! { - #[inline] - fn memory_limit(mut self, limit: usize) -> Self { - self.memory_limit = Some(limit); - self - } + #[cfg(any(doc, process_control_memory_limit))] + #[inline] + fn memory_limit(mut self, limit: usize) -> Self { + self.memory_limit = Some(limit); + self } #[inline] @@ -92,7 +91,7 @@ macro_rules! r#impl { let _ = self.process.stdin.take(); let mut result = $wait_fn(&mut self); - macro_rules! try_run { + macro_rules! run_if_ok { ( $get_result_fn:expr ) => { if result.is_ok() { if let Err(error) = $get_result_fn() { @@ -110,10 +109,10 @@ macro_rules! r#impl { imp::terminate_if_running(&mut self.process) .and_then(|()| self.process.wait()); if self.strict_errors { - try_run!(|| next_result); + run_if_ok!(|| next_result); } } - try_run!(|| self.process.try_wait()); + run_if_ok!(|| self.process.try_wait()); result } @@ -168,46 +167,53 @@ r#impl!( Self::run_wait, ); +struct Reader(Option>>>); + +impl Reader { + fn spawn(source: Option) -> io::Result + where + R: 'static + Read + Send, + { + source + .map(|mut source| { + thread::Builder::new().spawn(move || { + let mut buffer = Vec::new(); + let _ = source.read_to_end(&mut buffer)?; + Ok(buffer) + }) + }) + .transpose() + .map(Self) + } + + fn join(self) -> io::Result> { + self.0 + .map(|x| x.join().unwrap_or_else(|x| panic::resume_unwind(x))) + .transpose() + .map(|x| x.unwrap_or_else(Vec::new)) + } +} + impl OutputControl { fn run_wait_with_output(&mut self) -> WaitResult { - let stdout_reader = spawn_reader(&mut self.process.stdout)?; - let stderr_reader = spawn_reader(&mut self.process.stderr)?; + macro_rules! reader { + ( $stream:ident ) => { + Reader::spawn(self.process.$stream.take()) + }; + } + + let stdout_reader = reader!(stdout)?; + let stderr_reader = reader!(stderr)?; - return self - .run_wait()? + self.run_wait()? .map(|status| { Ok(Output { status, - stdout: join_reader(stdout_reader)?, - stderr: join_reader(stderr_reader)?, + stdout: stdout_reader.join()?, + stderr: stderr_reader.join()?, }) }) - .transpose(); - - type Reader = Option>>>; - - fn spawn_reader(source: &mut Option) -> io::Result - where - R: 'static + Read + Send, - { - source - .take() - .map(|mut source| { - thread::Builder::new().spawn(move || { - let mut buffer = Vec::new(); - let _ = source.read_to_end(&mut buffer)?; - Ok(buffer) - }) - }) - .transpose() - } - - fn join_reader(reader: Reader) -> io::Result> { - reader - .map(|x| x.join().unwrap_or_else(|x| panic::resume_unwind(x))) - .transpose() - .map(|x| x.unwrap_or_else(Vec::new)) - } + .transpose() } } diff --git a/src/lib.rs b/src/lib.rs index 734b407..c164bb2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -92,15 +92,6 @@ use std::process; use std::process::Child; use std::time::Duration; -macro_rules! if_memory_limit { - ( $($item:item)+ ) => { - $( - #[cfg(process_control_memory_limit)] - $item - )+ - }; -} - mod control; #[cfg_attr(unix, path = "unix/mod.rs")] @@ -284,7 +275,6 @@ impl Display for ExitStatus { impl From for ExitStatus { #[inline] fn from(value: process::ExitStatus) -> Self { - #[cfg_attr(windows, allow(clippy::useless_conversion))] Self(value.into()) } } @@ -343,29 +333,28 @@ pub trait Control: private::Sealed { /// [`wait`]: Self::wait type Result; - if_memory_limit! { - /// Sets the total virtual memory limit for the process in bytes. - /// - /// If the process attempts to allocate memory in excess of this limit, - /// the allocation will fail. The type of failure will depend on the - /// platform, and the process might terminate if it cannot handle it. - /// - /// Small memory limits are safe, but they might prevent the operating - /// system from starting the process. - #[cfg_attr( - process_control_docs_rs, - doc(cfg(any( - target_os = "android", - all( - target_os = "linux", - any(target_env = "gnu", target_env = "musl"), - ), - windows, - ))) - )] - #[must_use] - fn memory_limit(self, limit: usize) -> Self; - } + /// Sets the total virtual memory limit for the process in bytes. + /// + /// If the process attempts to allocate memory in excess of this limit, the + /// allocation will fail. The type of failure will depend on the platform, + /// and the process might terminate if it cannot handle it. + /// + /// Small memory limits are safe, but they might prevent the operating + /// system from starting the process. + #[cfg(any(doc, process_control_memory_limit))] + #[cfg_attr( + process_control_docs_rs, + doc(cfg(any( + target_os = "android", + all( + target_os = "linux", + any(target_env = "gnu", target_env = "musl"), + ), + windows, + ))) + )] + #[must_use] + fn memory_limit(self, limit: usize) -> Self; /// Sets the total time limit for the process in milliseconds. /// diff --git a/src/unix/exit_status.rs b/src/unix/exit_status.rs index 5b3d73a..21c75d0 100644 --- a/src/unix/exit_status.rs +++ b/src/unix/exit_status.rs @@ -89,7 +89,9 @@ impl ExitStatus { impl Display for ExitStatus { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { match self.kind { - ExitStatusKind::Continued => write!(f, "continued (WIFCONTINUED)"), + ExitStatusKind::Continued => { + f.write_str("continued (WIFCONTINUED)") + } ExitStatusKind::Dumped => { write!(f, "signal: {} (core dumped)", self.value) } @@ -99,7 +101,7 @@ impl Display for ExitStatus { write!(f, "stopped (not terminated) by signal: {}", self.value) } #[cfg(process_control_unix_waitid)] - ExitStatusKind::Trapped => write!(f, "trapped"), + ExitStatusKind::Trapped => f.write_str("trapped"), ExitStatusKind::Uncategorized => { write!(f, "uncategorized wait status: {}", self.value) } diff --git a/src/unix/mod.rs b/src/unix/mod.rs index 62611e4..ecf23c9 100644 --- a/src/unix/mod.rs +++ b/src/unix/mod.rs @@ -27,6 +27,15 @@ pub(super) use exit_status::ExitStatus; mod wait; +macro_rules! if_memory_limit { + ( $($item:item)+ ) => { + $( + #[cfg(process_control_memory_limit)] + $item + )+ + }; +} + if_memory_limit! { use std::convert::TryFrom; use std::ptr; diff --git a/src/unix/wait/common.rs b/src/unix/wait/common.rs index da4ea22..f8c2e29 100644 --- a/src/unix/wait/common.rs +++ b/src/unix/wait/common.rs @@ -10,13 +10,9 @@ use signal_hook::iterator::Signals; use crate::WaitResult; -use super::run_with_time_limit; - use super::super::ExitStatus; use super::super::Handle; -// https://github.com/rust-lang/rust-clippy/issues/3340 -#[allow(clippy::useless_transmute)] unsafe fn transmute_lifetime_mut<'a, T>(value: &mut T) -> &'a mut T where T: ?Sized, @@ -28,8 +24,6 @@ fn run_on_drop(drop_fn: F) -> impl Drop where F: FnOnce(), { - return Dropper(ManuallyDrop::new(drop_fn)); - struct Dropper(ManuallyDrop) where F: FnOnce(); @@ -42,6 +36,8 @@ where (unsafe { ManuallyDrop::take(&mut self.0) })(); } } + + Dropper(ManuallyDrop::new(drop_fn)) } pub(in super::super) fn wait( @@ -57,7 +53,7 @@ pub(in super::super) fn wait( }); let process = Arc::clone(&process); - run_with_time_limit( + super::run_with_time_limit( move || { let mut signals = Signals::new([SIGCHLD])?; loop { diff --git a/src/unix/wait/waitid.rs b/src/unix/wait/waitid.rs index f3edfaf..f14e17e 100644 --- a/src/unix/wait/waitid.rs +++ b/src/unix/wait/waitid.rs @@ -12,14 +12,12 @@ use super::super::check_syscall; use super::super::ExitStatus; use super::super::Handle; -use super::run_with_time_limit; - pub(in super::super) fn wait( handle: &mut Handle<'_>, time_limit: Option, ) -> WaitResult { let pid = handle.pid.as_id(); - run_with_time_limit( + super::run_with_time_limit( move || loop { let mut process_info = MaybeUninit::uninit(); check_result!(check_syscall(unsafe { diff --git a/src/windows/exit_status.rs b/src/windows/exit_status.rs index 7cabb89..bb91d50 100644 --- a/src/windows/exit_status.rs +++ b/src/windows/exit_status.rs @@ -4,14 +4,13 @@ use std::fmt::Formatter; use std::os::windows::process::ExitStatusExt; use std::process; -use super::DWORD; use super::EXIT_SUCCESS; #[derive(Copy, Clone, Debug, Eq, PartialEq)] -pub(crate) struct ExitStatus(DWORD); +pub(crate) struct ExitStatus(u32); impl ExitStatus { - pub(super) const fn new(exit_code: DWORD) -> Self { + pub(super) const fn new(exit_code: u32) -> Self { Self(exit_code) } @@ -19,7 +18,7 @@ impl ExitStatus { self.0 == EXIT_SUCCESS } - pub(crate) fn code(self) -> Option { + pub(crate) fn code(self) -> Option { Some(self.0) } } diff --git a/src/windows/mod.rs b/src/windows/mod.rs index cd3a7a2..64281b7 100644 --- a/src/windows/mod.rs +++ b/src/windows/mod.rs @@ -1,9 +1,4 @@ -use std::any; -use std::cell::Cell; use std::convert::TryInto; -use std::fmt; -use std::fmt::Debug; -use std::fmt::Formatter; use std::io; use std::iter::FusedIterator; use std::marker::PhantomData; @@ -17,6 +12,7 @@ use std::time::Instant; use windows_sys::Win32::Foundation::CloseHandle; use windows_sys::Win32::Foundation::DuplicateHandle; +use windows_sys::Win32::Foundation::BOOL; use windows_sys::Win32::Foundation::DUPLICATE_SAME_ACCESS; use windows_sys::Win32::Foundation::ERROR_ACCESS_DENIED; use windows_sys::Win32::Foundation::ERROR_INVALID_HANDLE; @@ -59,17 +55,10 @@ macro_rules! assert_matches { }}; } -// https://github.com/microsoft/windows-rs/issues/881 -#[allow(clippy::upper_case_acronyms)] -type BOOL = i32; -#[allow(clippy::upper_case_acronyms)] -type DWORD = u32; -type NonZeroDword = NonZeroU32; - -const EXIT_SUCCESS: DWORD = 0; +const EXIT_SUCCESS: u32 = 0; const TRUE: BOOL = 1; -fn raw_os_error(error: &io::Error) -> Option { +fn raw_os_error(error: &io::Error) -> Option { error.raw_os_error().and_then(|x| x.try_into().ok()) } @@ -89,7 +78,7 @@ impl RawHandle { Self(process.as_raw_handle() as _) } - unsafe fn get_exit_code(&self) -> io::Result { + unsafe fn get_exit_code(&self) -> io::Result { let mut exit_code = 0; check_syscall(unsafe { GetExitCodeProcess(self.0, &mut exit_code) })?; Ok(exit_code) @@ -123,6 +112,7 @@ impl RawHandle { unsafe impl Send for RawHandle {} unsafe impl Sync for RawHandle {} +#[derive(Debug)] struct JobHandle(Option); impl JobHandle { @@ -145,12 +135,6 @@ impl JobHandle { } } -impl Debug for JobHandle { - fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - any::type_name::>>().fmt(f) - } -} - impl Drop for JobHandle { fn drop(&mut self) { let _ = self.close(); @@ -174,14 +158,14 @@ impl TimeLimits { impl FusedIterator for TimeLimits {} impl Iterator for TimeLimits { - type Item = NonZeroDword; + type Item = NonZeroU32; fn next(&mut self) -> Option { let time_limit = if let Some(time_limit) = self.time_limit { time_limit } else { - const NON_ZERO_INFINITE: NonZeroDword = - if let Some(result) = NonZeroDword::new(INFINITE) { + const NON_ZERO_INFINITE: NonZeroU32 = + if let Some(result) = NonZeroU32::new(INFINITE) { result } else { unreachable!(); @@ -194,11 +178,11 @@ impl Iterator for TimeLimits { .saturating_sub(self.start.elapsed()) .as_millis() .try_into() - .unwrap_or(DWORD::MAX); + .unwrap_or(u32::MAX); if time_limit == INFINITE { time_limit -= 1; } - NonZeroDword::new(time_limit) + NonZeroU32::new(time_limit) } } diff --git a/tests/common/mod.rs b/tests/common/mod.rs index cd10700..1c58b9c 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -31,7 +31,6 @@ macro_rules! assert_matches { }}; } -#[cfg_attr(not(process_control_memory_limit), allow(unused_macro_rules))] macro_rules! test { ( command: $command:expr , $($token:tt)* ) => {{ test!(@output $command, controlled, $($token)*); diff --git a/tests/common/windows.rs b/tests/common/windows.rs index 41b5107..37e3b1a 100644 --- a/tests/common/windows.rs +++ b/tests/common/windows.rs @@ -4,6 +4,7 @@ use std::process::Child; use windows_sys::Win32::Foundation::CloseHandle; use windows_sys::Win32::Foundation::DuplicateHandle; +use windows_sys::Win32::Foundation::BOOL; use windows_sys::Win32::Foundation::DUPLICATE_SAME_ACCESS; use windows_sys::Win32::Foundation::HANDLE; use windows_sys::Win32::Foundation::WAIT_TIMEOUT; @@ -11,9 +12,6 @@ use windows_sys::Win32::System::Threading::GetCurrentProcess; use windows_sys::Win32::System::Threading::WaitForSingleObject; use windows_sys::Win32::System::Threading::WAIT_OBJECT_0; -#[allow(clippy::upper_case_acronyms)] -type BOOL = i32; - const TRUE: BOOL = 1; fn check_syscall(result: BOOL) -> io::Result<()> { diff --git a/tests/integration.rs b/tests/integration.rs index 7892d2e..13cde49 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -32,6 +32,12 @@ fn test_large_output() -> io::Result<()> { const BUFFER_LENGTH: usize = 1024; const OUTPUT_LENGTH: usize = BUFFER_COUNT * BUFFER_LENGTH; + #[track_caller] + fn test_output(output: Vec, byte: u8) { + assert_eq!(OUTPUT_LENGTH, output.len()); + assert!(output.into_iter().all(|x| x == byte)); + } + let process = Command::new("perl") .arg("-e") .arg( @@ -59,13 +65,7 @@ fn test_large_output() -> io::Result<()> { test_output(output.stdout, b'a'); test_output(output.stderr, b'b'); - return Ok(()); - - #[track_caller] - fn test_output(output: Vec, byte: u8) { - assert_eq!(OUTPUT_LENGTH, output.len()); - assert!(output.into_iter().all(|x| x == byte)); - } + Ok(()) } #[test] diff --git a/tests/memory_limit.rs b/tests/memory_limit.rs index f909d38..f63b69f 100644 --- a/tests/memory_limit.rs +++ b/tests/memory_limit.rs @@ -14,7 +14,7 @@ mod common; use common::MEMORY_LIMIT; use common::SHORT_TIME_LIMIT; -fn create_memory_limit_command(bytes: usize) -> Command { +fn create_command(bytes: usize) -> Command { let mut command = Command::new("perl"); let _ = command .arg("-e") @@ -28,9 +28,9 @@ fn create_memory_limit_command(bytes: usize) -> Command { } #[test] -fn test_memory_limit() -> io::Result<()> { +fn test_accept() -> io::Result<()> { test!( - command: create_memory_limit_command(MEMORY_LIMIT), + command: create_command(MEMORY_LIMIT), memory_limit: 2 * MEMORY_LIMIT, terminating: false, expected_result: Some(Some(0)), @@ -39,9 +39,9 @@ fn test_memory_limit() -> io::Result<()> { } #[test] -fn test_memory_limit_exceeded() -> io::Result<()> { +fn test_reject() -> io::Result<()> { test!( - command: create_memory_limit_command(MEMORY_LIMIT), + command: create_command(MEMORY_LIMIT), memory_limit: MEMORY_LIMIT, terminating: false, expected_result: Some(Some(1)), @@ -63,9 +63,9 @@ macro_rules! memory_limit_0_result { } #[test] -fn test_memory_limit_0() -> io::Result<()> { +fn test_0() -> io::Result<()> { test!( - command: create_memory_limit_command(MEMORY_LIMIT), + command: create_command(MEMORY_LIMIT), memory_limit: 0, terminating: false, expected_result: Some(memory_limit_0_result!()), @@ -74,9 +74,9 @@ fn test_memory_limit_0() -> io::Result<()> { } #[test] -fn test_memory_limit_1() -> io::Result<()> { +fn test_1() -> io::Result<()> { test!( - command: create_memory_limit_command(MEMORY_LIMIT), + command: create_command(MEMORY_LIMIT), memory_limit: 1, terminating: false, expected_result: Some(memory_limit_0_result!()), diff --git a/tests/time_limit.rs b/tests/time_limit.rs index de3f52b..2e4874d 100644 --- a/tests/time_limit.rs +++ b/tests/time_limit.rs @@ -11,7 +11,7 @@ use common::LONG_TIME_LIMIT; use common::SHORT_TIME_LIMIT; #[test] -fn test_time_limit() -> io::Result<()> { +fn test_accept() -> io::Result<()> { test!( command: common::create_time_limit_command(SHORT_TIME_LIMIT), time_limit: LONG_TIME_LIMIT, @@ -22,7 +22,7 @@ fn test_time_limit() -> io::Result<()> { } #[test] -fn test_time_limit_expired() -> io::Result<()> { +fn test_reject() -> io::Result<()> { test!( command: common::create_time_limit_command(LONG_TIME_LIMIT), time_limit: SHORT_TIME_LIMIT, @@ -33,7 +33,7 @@ fn test_time_limit_expired() -> io::Result<()> { } #[test] -fn test_terminating_time_limit() -> io::Result<()> { +fn test_terminating_accept() -> io::Result<()> { test!( command: common::create_time_limit_command(SHORT_TIME_LIMIT), time_limit: LONG_TIME_LIMIT, @@ -44,7 +44,7 @@ fn test_terminating_time_limit() -> io::Result<()> { } #[test] -fn test_terminating_time_limit_expired() -> io::Result<()> { +fn test_terminating_reject() -> io::Result<()> { test!( command: common::create_time_limit_command(LONG_TIME_LIMIT), time_limit: SHORT_TIME_LIMIT,