Minor improvements

COPYRIGHT

@@ -1,4 +1,4 @@
-Copyright (c) 2020 Dylan Iuzzolino
+Copyright (c) 2020 dylni (https://github.com/dylni)
 
 Licensed under the Apache License, Version 2.0 <LICENSE-APACHE> or the MIT
 license <LICENSE-MIT>, at your option. All files in this project may not be

LICENSE-MIT

@@ -1,6 +1,6 @@
 MIT License
 
-Copyright (c) 2020 Dylan Iuzzolino
+Copyright (c) 2020 dylni (https://github.com/dylni)
 
 Permission is hereby granted, free of charge, to any person obtaining a copy
 of this software and associated documentation files (the "Software"), to deal

README.md

@@ -11,9 +11,9 @@ Methods for setting limits are available on [`ChildExt`], which is implemented
 for [`Child`]. They each return a builder of options to configure how the limit
 should be applied.
 
-***Warning****: This crate should not be used for security. There are
-many ways that a process can bypass resource limits. The limits are only
-intended for simple restriction of harmless processes.*
+***Warning**: This crate should not be used for security. There are many ways
+that a process can bypass resource limits. The limits are only intended for
+simple restriction of harmless processes.*
 
 [![GitHub Build Status](https://github.com/dylni/process_control/workflows/build/badge.svg?branch=master)](https://github.com/dylni/process_control/actions?query=branch%3Amaster)
 

src/lib.rs

@@ -15,12 +15,6 @@
 //! only intended for simple restriction of harmless processes.
 //! </div>
 //!
-//! # Implementation
-//!
-//! All traits are [sealed], meaning that they can only be implemented by this
-//! crate. Otherwise, backward compatibility would be more difficult to
-//! maintain for new features.
-//!
 //! # Features
 //!
 //! These features are optional and can be enabled or disabled in a
@@ -32,14 +26,20 @@
 //!   Changes the implementation to use crate [crossbeam-channel] for better
 //!   performance.
 //!
+//! # Implementation
+//!
+//! All traits are [sealed], meaning that they can only be implemented by this
+//! crate. Otherwise, backward compatibility would be more difficult to
+//! maintain for new features.
+//!
 //! # Comparable Crates
 //!
 //! - [wait-timeout] -
 //!   Made for a related purpose but does not provide the same functionality.
 //!   Processes cannot be terminated automatically, and there is no counterpart
-//!   of [`Child::wait_with_output`] to read output while setting a timeout.
-//!   This crate aims to fill in those gaps and simplify the implementation,
-//!   now that [`Receiver::recv_timeout`] exists.
+//!   of [`ChildExt::controlled_with_output`] to read output while setting a
+//!   timeout. This crate aims to fill in those gaps and simplify the
+//!   implementation, now that [`Receiver::recv_timeout`] exists.
 //!
 //! # Examples
 //!

src/unix.rs

@@ -3,6 +3,7 @@ use std::fmt;
 use std::fmt::Display;
 use std::fmt::Formatter;
 use std::io;
+use std::mem;
 use std::mem::MaybeUninit;
 use std::os::raw::c_int;
 use std::os::unix::process::ExitStatusExt;
@@ -13,10 +14,12 @@ use std::time::Duration;
 
 #[cfg(all(target_env = "gnu", target_os = "linux"))]
 use libc::__rlimit_resource_t;
+use libc::id_t;
 use libc::pid_t;
 use libc::CLD_EXITED;
 use libc::EINTR;
 use libc::ESRCH;
+use libc::EXIT_SUCCESS;
 use libc::P_PID;
 use libc::SIGKILL;
 use libc::WEXITED;
@@ -31,6 +34,12 @@ if_memory_limit! {
     use libc::RLIMIT_AS;
 }
 
+macro_rules! static_assert {
+    ( $condition:expr ) => {
+        const _: () = [()][if $condition { 0 } else { 1 }];
+    };
+}
+
 #[cfg(any(
     all(target_env = "musl", target_os = "linux"),
     target_os = "android",
@@ -48,7 +57,7 @@ pub(super) struct ExitStatus {
 
 impl ExitStatus {
     pub(super) const fn success(self) -> bool {
-        self.exited && self.value == 0
+        self.exited && self.value == EXIT_SUCCESS
     }
 
     fn get_value(self, exit: bool) -> Option<c_int> {
@@ -108,6 +117,12 @@ where
     F: 'static + FnOnce() -> R + Send,
     R: 'static + Send,
 {
+    let time_limit = if let Some(time_limit) = time_limit {
+        time_limit
+    } else {
+        return Ok(Some(run_fn()));
+    };
+
     let (result_sender, result_receiver) = {
         #[cfg(feature = "crossbeam-channel")]
         {
@@ -120,14 +135,10 @@ where
             mpsc::channel()
         }
     };
-    let _ =
-        thread::Builder::new().spawn(move || result_sender.send(run_fn()))?;
-
-    Ok(time_limit
-        .map(|x| result_receiver.recv_timeout(x).ok())
-        .unwrap_or_else(|| {
-            Some(result_receiver.recv().expect("channel was disconnected"))
-        }))
+
+    thread::Builder::new()
+        .spawn(move || result_sender.send(run_fn()))
+        .map(|_| result_receiver.recv_timeout(time_limit).ok())
 }
 
 const INVALID_PID_ERROR: &str = "process identifier is invalid";
@@ -137,7 +148,15 @@ struct RawPid(pid_t);
 
 impl RawPid {
     fn new(process: &Child) -> Self {
-        Self(process.id().try_into().expect(INVALID_PID_ERROR))
+        let pid: u32 = process.id();
+        Self(pid.try_into().expect(INVALID_PID_ERROR))
+    }
+
+    const fn as_id(&self) -> id_t {
+        static_assert!(pid_t::MAX == i32::MAX);
+        static_assert!(mem::size_of::<pid_t>() <= mem::size_of::<id_t>());
+
+        self.0 as _
     }
 }
 
@@ -223,11 +242,7 @@ impl SharedHandle {
             }
         }
 
-        #[cfg_attr(
-            any(target_os = "illumos", target_os = "solaris"),
-            allow(clippy::useless_conversion)
-        )]
-        let pid = self.pid.0.try_into().expect(INVALID_PID_ERROR);
+        let pid = self.pid.as_id();
         run_with_time_limit(
             move || loop {
                 let mut process_info = MaybeUninit::uninit();

src/windows.rs

@@ -21,7 +21,7 @@ use windows_sys::Win32::Foundation::ERROR_ACCESS_DENIED;
 use windows_sys::Win32::Foundation::ERROR_INVALID_HANDLE;
 use windows_sys::Win32::Foundation::ERROR_INVALID_PARAMETER;
 use windows_sys::Win32::Foundation::HANDLE;
-use windows_sys::Win32::Foundation::STATUS_PENDING;
+use windows_sys::Win32::Foundation::STILL_ACTIVE;
 use windows_sys::Win32::Foundation::WAIT_TIMEOUT;
 use windows_sys::Win32::System::JobObjects::AssignProcessToJobObject;
 use windows_sys::Win32::System::JobObjects::CreateJobObjectW;
@@ -45,7 +45,6 @@ type BOOL = i32;
 type DWORD = u32;
 
 const EXIT_SUCCESS: DWORD = 0;
-const STILL_ACTIVE: DWORD = STATUS_PENDING as _;
 const TRUE: BOOL = 1;
 
 trait ReplaceNone<T> {
@@ -134,7 +133,7 @@ impl RawHandle {
                     ERROR_ACCESS_DENIED => {
                         matches!(
                             self.get_exit_code(),
-                            Ok(x) if x != STILL_ACTIVE,
+                            Ok(x) if x.try_into() != Ok(STILL_ACTIVE),
                         )
                     }
                     // This error is usually decoded to [ErrorKind::Other]:
@@ -197,71 +196,72 @@ impl SharedHandle {
     fn set_memory_limit(&mut self) -> io::Result<()> {
         self.job_handle.close()?;
 
-        if let Some(memory_limit) = self.memory_limit {
-            let job_handle =
-                unsafe { CreateJobObjectW(ptr::null(), ptr::null_mut()) };
-            if job_handle == 0 {
-                return Err(io::Error::last_os_error());
-            }
-            self.job_handle.0.replace_none(RawHandle(job_handle));
-
-            let job_information = JOBOBJECT_EXTENDED_LIMIT_INFORMATION {
-                BasicLimitInformation: JOBOBJECT_BASIC_LIMIT_INFORMATION {
-                    PerProcessUserTimeLimit: 0,
-                    PerJobUserTimeLimit: 0,
-                    LimitFlags: JOB_OBJECT_LIMIT_JOB_MEMORY,
-                    MinimumWorkingSetSize: 0,
-                    MaximumWorkingSetSize: 0,
-                    ActiveProcessLimit: 0,
-                    Affinity: 0,
-                    PriorityClass: 0,
-                    SchedulingClass: 0,
-                },
-                IoInfo: IO_COUNTERS {
-                    ReadOperationCount: 0,
-                    WriteOperationCount: 0,
-                    OtherOperationCount: 0,
-                    ReadTransferCount: 0,
-                    WriteTransferCount: 0,
-                    OtherTransferCount: 0,
-                },
-                ProcessMemoryLimit: 0,
-                JobMemoryLimit: memory_limit,
-                PeakProcessMemoryUsed: 0,
-                PeakJobMemoryUsed: 0,
-            };
-            let job_information_ptr: *const _ = &job_information;
-            let result = check_syscall(unsafe {
-                SetInformationJobObject(
-                    job_handle,
-                    JobObjectExtendedLimitInformation,
-                    job_information_ptr.cast(),
-                    mem::size_of_val(&job_information)
-                        .try_into()
-                        .expect("job information too large for WinAPI"),
-                )
-            });
-            match result {
-                Ok(()) => {}
-                // This error will occur when the job has a low memory limit.
-                Err(ref error) => {
-                    return if raw_os_error(error)
-                        == Some(ERROR_INVALID_PARAMETER)
-                    {
-                        self.job_handle.close()?;
-                        unsafe { self.handle.terminate() }
-                    } else {
-                        result
-                    };
-                }
-            }
+        let memory_limit = if let Some(memory_limit) = self.memory_limit {
+            memory_limit
+        } else {
+            return Ok(());
+        };
 
-            check_syscall(unsafe {
-                AssignProcessToJobObject(job_handle, self.handle.0)
-            })?;
+        let job_handle =
+            unsafe { CreateJobObjectW(ptr::null(), ptr::null_mut()) };
+        if job_handle == 0 {
+            return Err(io::Error::last_os_error());
+        }
+        self.job_handle.0.replace_none(RawHandle(job_handle));
+
+        let job_information = JOBOBJECT_EXTENDED_LIMIT_INFORMATION {
+            BasicLimitInformation: JOBOBJECT_BASIC_LIMIT_INFORMATION {
+                PerProcessUserTimeLimit: 0,
+                PerJobUserTimeLimit: 0,
+                LimitFlags: JOB_OBJECT_LIMIT_JOB_MEMORY,
+                MinimumWorkingSetSize: 0,
+                MaximumWorkingSetSize: 0,
+                ActiveProcessLimit: 0,
+                Affinity: 0,
+                PriorityClass: 0,
+                SchedulingClass: 0,
+            },
+            IoInfo: IO_COUNTERS {
+                ReadOperationCount: 0,
+                WriteOperationCount: 0,
+                OtherOperationCount: 0,
+                ReadTransferCount: 0,
+                WriteTransferCount: 0,
+                OtherTransferCount: 0,
+            },
+            ProcessMemoryLimit: 0,
+            JobMemoryLimit: memory_limit,
+            PeakProcessMemoryUsed: 0,
+            PeakJobMemoryUsed: 0,
+        };
+        let job_information_ptr: *const _ = &job_information;
+        let result = check_syscall(unsafe {
+            SetInformationJobObject(
+                job_handle,
+                JobObjectExtendedLimitInformation,
+                job_information_ptr.cast(),
+                mem::size_of_val(&job_information)
+                    .try_into()
+                    .expect("job information too large for WinAPI"),
+            )
+        });
+        match result {
+            Ok(()) => {}
+            // This error will occur when the job has a low memory limit.
+            Err(ref error) => {
+                return if raw_os_error(error) == Some(ERROR_INVALID_PARAMETER)
+                {
+                    self.job_handle.close()?;
+                    unsafe { self.handle.terminate() }
+                } else {
+                    result
+                };
+            }
         }
 
-        Ok(())
+        check_syscall(unsafe {
+            AssignProcessToJobObject(job_handle, self.handle.0)
+        })
     }
 
     pub(super) fn wait(&mut self) -> io::Result<Option<ExitStatus>> {

tests/integration.rs

@@ -156,14 +156,14 @@ macro_rules! test {
         test!($control.strict_errors(), $($token)*);
     }};
     ( $control:expr , $terminator:expr , terminating: true, $($token:tt)* ) =>
-    {{
+    {
         test!(
             $control.terminate_for_timeout(),
             $terminator,
             terminating: false,
             $($token)*
-        );
-    }};
+        )
+    };
     (
         $control:expr ,
         $terminator:expr ,
@@ -331,7 +331,7 @@ fn test_large_output() -> io::Result<()> {
         .arg(
             r"for (my $i = 0; $i < $ARGV[0]; $i++) {
                 print 'a' x $ARGV[1];
-                print STDERR 'a' x $ARGV[1];
+                print STDERR 'b' x $ARGV[1];
             }",
         )
         .arg("--")
@@ -350,14 +350,14 @@ fn test_large_output() -> io::Result<()> {
 
     assert_eq!(Some(0), output.status.code());
 
-    assert_eq!(OUTPUT_LENGTH, output.stdout.len());
-    assert_eq!(OUTPUT_LENGTH, output.stderr.len());
+    test_output(output.stdout, b'a');
+    test_output(output.stderr, b'b');
 
-    assert!(output
-        .stdout
-        .into_iter()
-        .chain(output.stderr)
-        .all(|x| x == b'a'));
+    return Ok(());
 
-    Ok(())
+    #[track_caller]
+    fn test_output(output: Vec<u8>, byte: u8) {
+        assert_eq!(OUTPUT_LENGTH, output.len());
+        assert!(output.into_iter().all(|x| x == byte));
+    }
 }