Merge pull request #1435 from GuillaumeGomez/wait-pid

Make `Process::wait` return `Option<ExitStatus>`
GuillaumeGomez · Dec 20, 2024 · b4a2cf0 · b4a2cf0
2 parents 7c48259 + 7753154
commit b4a2cf0
Show file tree

Hide file tree

Showing 8 changed files with 65 additions and 53 deletions.
diff --git a/src/common/system.rs b/src/common/system.rs
@@ -4,6 +4,7 @@ use std::collections::{HashMap, HashSet};
 use std::ffi::{OsStr, OsString};
 use std::fmt;
 use std::path::Path;
+use std::process::ExitStatus;
 use std::str::FromStr;
 
 use crate::common::impl_get_set::impl_get_set;
@@ -1188,7 +1189,8 @@ impl Process {
         self.inner.kill_with(signal)
     }
 
-    /// Wait for process termination.
+    /// Wait for process termination and returns its [`ExitStatus`] if it could be retrieved,
+    /// returns `None` otherwise.
     ///
     /// ```no_run
     /// use sysinfo::{Pid, System};
@@ -1197,11 +1199,11 @@ impl Process {
     ///
     /// if let Some(process) = s.process(Pid::from(1337)) {
     ///     println!("Waiting for pid 1337");
-    ///     process.wait();
-    ///     println!("Pid 1337 exited");
+    ///     let exit_status = process.wait();
+    ///     println!("Pid 1337 exited with: {exit_status:?}");
     /// }
     /// ```
-    pub fn wait(&self) {
+    pub fn wait(&self) -> Option<ExitStatus> {
         self.inner.wait()
     }
 

diff --git a/src/unix/apple/app_store/process.rs b/src/unix/apple/app_store/process.rs
@@ -2,6 +2,7 @@
 
 use std::ffi::{OsStr, OsString};
 use std::path::Path;
+use std::process::ExitStatus;
 
 use crate::{DiskUsage, Gid, Pid, ProcessStatus, Signal, Uid};
 
@@ -88,7 +89,9 @@ impl ProcessInner {
         None
     }
 
-    pub(crate) fn wait(&self) {}
+    pub(crate) fn wait(&self) -> Option<ExitStatus> {
+        None
+    }
 
     pub(crate) fn session_id(&self) -> Option<Pid> {
         None

diff --git a/src/unix/apple/macos/process.rs b/src/unix/apple/macos/process.rs
@@ -4,6 +4,7 @@ use std::ffi::{OsStr, OsString};
 use std::mem::{self, MaybeUninit};
 use std::os::unix::ffi::{OsStrExt, OsStringExt};
 use std::path::{Path, PathBuf};
+use std::process::ExitStatus;
 
 use libc::{c_int, c_void, kill};
 
@@ -202,18 +203,8 @@ impl ProcessInner {
         self.effective_group_id
     }
 
-    pub(crate) fn wait(&self) {
-        let mut status = 0;
-        // attempt waiting
-        unsafe {
-            if retry_eintr!(libc::waitpid(self.pid.0, &mut status, 0)) < 0 {
-                // attempt failed (non-child process) so loop until process ends
-                let duration = std::time::Duration::from_millis(10);
-                while kill(self.pid.0, 0) == 0 {
-                    std::thread::sleep(duration);
-                }
-            }
-        }
+    pub(crate) fn wait(&self) -> Option<ExitStatus> {
+        crate::unix::utils::wait_process(self.pid)
     }
 
     pub(crate) fn session_id(&self) -> Option<Pid> {

diff --git a/src/unix/freebsd/process.rs b/src/unix/freebsd/process.rs
@@ -5,8 +5,7 @@ use crate::{DiskUsage, Gid, Pid, Process, ProcessRefreshKind, ProcessStatus, Sig
 use std::ffi::{OsStr, OsString};
 use std::fmt;
 use std::path::{Path, PathBuf};
-
-use libc::kill;
+use std::process::ExitStatus;
 
 use super::utils::{get_sys_value_str, WrapMap};
 
@@ -154,18 +153,8 @@ impl ProcessInner {
         Some(self.effective_group_id)
     }
 
-    pub(crate) fn wait(&self) {
-        let mut status = 0;
-        // attempt waiting
-        unsafe {
-            if retry_eintr!(libc::waitpid(self.pid.0, &mut status, 0)) < 0 {
-                // attempt failed (non-child process) so loop until process ends
-                let duration = std::time::Duration::from_millis(10);
-                while kill(self.pid.0, 0) == 0 {
-                    std::thread::sleep(duration);
-                }
-            }
-        }
+    pub(crate) fn wait(&self) -> Option<ExitStatus> {
+        crate::unix::utils::wait_process(self.pid)
     }
 
     pub(crate) fn session_id(&self) -> Option<Pid> {

diff --git a/src/unix/linux/process.rs b/src/unix/linux/process.rs
@@ -8,10 +8,11 @@ use std::fs::{self, DirEntry, File};
 use std::io::Read;
 use std::os::unix::ffi::OsStrExt;
 use std::path::{Path, PathBuf};
+use std::process::ExitStatus;
 use std::str::{self, FromStr};
 use std::sync::atomic::{AtomicUsize, Ordering};
 
-use libc::{c_ulong, gid_t, kill, uid_t};
+use libc::{c_ulong, gid_t, uid_t};
 
 use crate::sys::system::SystemInfo;
 use crate::sys::utils::{
@@ -167,7 +168,7 @@ impl ProcessInner {
 
     pub(crate) fn kill_with(&self, signal: Signal) -> Option<bool> {
         let c_signal = crate::sys::system::convert_signal(signal)?;
-        unsafe { Some(kill(self.pid.0, c_signal) == 0) }
+        unsafe { Some(libc::kill(self.pid.0, c_signal) == 0) }
     }
 
     pub(crate) fn name(&self) -> &OsStr {
@@ -251,18 +252,8 @@ impl ProcessInner {
         self.effective_group_id
     }
 
-    pub(crate) fn wait(&self) {
-        let mut status = 0;
-        // attempt waiting
-        unsafe {
-            if retry_eintr!(libc::waitpid(self.pid.0, &mut status, 0)) < 0 {
-                // attempt failed (non-child process) so loop until process ends
-                let duration = std::time::Duration::from_millis(10);
-                while kill(self.pid.0, 0) == 0 {
-                    std::thread::sleep(duration);
-                }
-            }
-        }
+    pub(crate) fn wait(&self) -> Option<ExitStatus> {
+        crate::unix::utils::wait_process(self.pid)
     }
 
     pub(crate) fn session_id(&self) -> Option<Pid> {

diff --git a/src/unix/utils.rs b/src/unix/utils.rs
@@ -34,3 +34,24 @@ pub(crate) fn cstr_to_rust_with_size(
         String::from_utf8(s).ok()
     }
 }
+
+#[cfg(all(
+    feature = "system",
+    not(any(target_os = "ios", feature = "apple-sandbox"))
+))]
+pub(crate) fn wait_process(pid: crate::Pid) -> Option<std::process::ExitStatus> {
+    use std::os::unix::process::ExitStatusExt;
+
+    let mut status = 0;
+    // attempt waiting
+    unsafe {
+        if retry_eintr!(libc::waitpid(pid.0, &mut status, 0)) < 0 {
+            // attempt failed (non-child process) so loop until process ends
+            let duration = std::time::Duration::from_millis(10);
+            while libc::kill(pid.0, 0) == 0 {
+                std::thread::sleep(duration);
+            }
+        }
+        Some(std::process::ExitStatus::from_raw(status))
+    }
+}
diff --git a/src/unknown/process.rs b/src/unknown/process.rs
@@ -5,6 +5,7 @@ use crate::{DiskUsage, Gid, Pid, ProcessStatus, Signal, Uid};
 use std::ffi::{OsStr, OsString};
 use std::fmt;
 use std::path::Path;
+use std::process::ExitStatus;
 
 impl fmt::Display for ProcessStatus {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
@@ -98,7 +99,9 @@ impl ProcessInner {
         None
     }
 
-    pub(crate) fn wait(&self) {}
+    pub(crate) fn wait(&self) -> Option<ExitStatus> {
+        None
+    }
 
     pub(crate) fn session_id(&self) -> Option<Pid> {
         None

diff --git a/src/windows/process.rs b/src/windows/process.rs
@@ -11,9 +11,9 @@ use std::fmt;
 use std::io;
 use std::mem::{size_of, zeroed, MaybeUninit};
 use std::os::windows::ffi::OsStringExt;
-use std::os::windows::process::CommandExt;
+use std::os::windows::process::{CommandExt, ExitStatusExt};
 use std::path::{Path, PathBuf};
-use std::process;
+use std::process::{self, ExitStatus};
 use std::ptr::null_mut;
 use std::str;
 use std::sync::{Arc, OnceLock};
@@ -41,9 +41,9 @@ use windows::Win32::System::ProcessStatus::GetModuleFileNameExW;
 use windows::Win32::System::RemoteDesktop::ProcessIdToSessionId;
 use windows::Win32::System::SystemInformation::OSVERSIONINFOEXW;
 use windows::Win32::System::Threading::{
-    GetProcessIoCounters, GetProcessTimes, GetSystemTimes, OpenProcess, OpenProcessToken,
-    CREATE_NO_WINDOW, IO_COUNTERS, PEB, PROCESS_BASIC_INFORMATION, PROCESS_QUERY_INFORMATION,
-    PROCESS_QUERY_LIMITED_INFORMATION, PROCESS_VM_READ,
+    GetExitCodeProcess, GetProcessIoCounters, GetProcessTimes, GetSystemTimes, OpenProcess,
+    OpenProcessToken, CREATE_NO_WINDOW, IO_COUNTERS, PEB, PROCESS_BASIC_INFORMATION,
+    PROCESS_QUERY_INFORMATION, PROCESS_QUERY_LIMITED_INFORMATION, PROCESS_VM_READ,
 };
 use windows::Win32::UI::Shell::CommandLineToArgvW;
 
@@ -404,18 +404,30 @@ impl ProcessInner {
         None
     }
 
-    pub(crate) fn wait(&self) {
+    pub(crate) fn wait(&self) -> Option<ExitStatus> {
         if let Some(handle) = self.get_handle() {
             while is_proc_running(handle) {
                 if get_start_time(handle) != self.start_time() {
                     // PID owner changed so the previous process was finished!
-                    return;
+                    sysinfo_debug!("PID owner changed so cannot get old process exit status");
+                    return None;
                 }
                 std::thread::sleep(std::time::Duration::from_millis(10));
             }
+            let mut exit_status = 0;
+            unsafe {
+                match GetExitCodeProcess(handle, &mut exit_status) {
+                    Ok(_) => Some(ExitStatus::from_raw(exit_status)),
+                    Err(_error) => {
+                        sysinfo_debug!("failed to retrieve process exit status: {_error:?}");
+                        None
+                    }
+                }
+            }
         } else {
             // In this case, we can't do anything so we just return.
             sysinfo_debug!("can't wait on this process so returning");
+            None
         }
     }