Repository: GuillaumeGomez/sysinfo
Pull request #865 (2022-10-17 18:48:54)

Add wait() to process
Fixes https://github.com/GuillaumeGomez/sysinfo/issues/858

Note: This is incomplete and will update in sometime.


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 52709b1..30eda3d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,10 @@
+# 0.26.5
+
+ * Windows: Fix disk information retrieval.
+ * Linux: Improve `Process` document.
+ * Linux: Fix a compilation error if the `apple-sandbox` feature is enabled.
+ * Internal code improvements.
+
 # 0.26.4
 
  * Add `SystemExt::distribution_id` method.
diff --git a/Cargo.toml b/Cargo.toml
index cef3263..0f12934 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "sysinfo"
-version = "0.26.4"
+version = "0.26.5"
 authors = ["Guillaume Gomez <guillaume1.gomez@gmail.com>"]
 
 description = "Library to get system information such as processes, CPUs, disks, components and networks"
diff --git a/README.md b/README.md
index 27437c5..e44a731 100644
--- a/README.md
+++ b/README.md
@@ -107,6 +107,28 @@ By default, `sysinfo` uses multiple threads. However, this can increase the memo
 platforms (macOS for example). The behavior can be disabled by setting `default-features = false`
 in `Cargo.toml` (which disables the `multithread` cargo feature).
 
+### Good practice / Performance tips
+
+Most of the time, you don't want all information provided by `sysinfo` but just a subset of it.
+In this case, it's recommended to use `refresh_specifics(...)` methods with only what you need
+to have much better performance.
+
+Another issues frequently encountered: unless you know what you're doing, it's almost all the
+time better to instantiate the `System` struct once and use this one instance through your
+program. The reason is because a lot of information needs a previous measure to be computed
+(the CPU usage for example). Another example why it's much better: in case you want to list
+all running processes, `sysinfo` needs to allocate all memory for the `Process` struct list,
+which takes quite some time on the first run.
+
+If your program needs to use a lot of file descriptors, you'd better use:
+
+```rust,no_run
+sysinfo::set_open_files_limit(0);
+```
+
+as `sysinfo` keeps a number of file descriptors open to have better performance on some
+targets when refreshing processes.
+
 ### Running on Raspberry Pi
 
 It'll be difficult to build on Raspberry Pi. A good way-around is to cross-build, then send the
diff --git a/src/apple/app_store/process.rs b/src/apple/app_store/process.rs
index 8c3348e..7c43697 100644
--- a/src/apple/app_store/process.rs
+++ b/src/apple/app_store/process.rs
@@ -79,4 +79,6 @@ impl ProcessExt for Process {
     fn group_id(&self) -> Option<Gid> {
         None
     }
+
+    fn wait(&self) {}
 }
diff --git a/src/apple/macos/process.rs b/src/apple/macos/process.rs
index 3728269..fff9c1f 100644
--- a/src/apple/macos/process.rs
+++ b/src/apple/macos/process.rs
@@ -184,6 +184,20 @@ impl ProcessExt for Process {
     fn group_id(&self) -> Option<Gid> {
         self.group_id
     }
+
+    fn wait(&self) {
+        let mut status = 0;
+        // attempt waiting
+        unsafe {
+            if 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);
+                }
+            }
+        }
+    }
 }
 
 #[allow(deprecated)] // Because of libc::mach_absolute_time.
diff --git a/src/freebsd/process.rs b/src/freebsd/process.rs
index 9dabaa0..b0dda0f 100644
--- a/src/freebsd/process.rs
+++ b/src/freebsd/process.rs
@@ -5,6 +5,8 @@ use crate::{DiskUsage, Gid, Pid, ProcessExt, ProcessRefreshKind, ProcessStatus,
 use std::fmt;
 use std::path::{Path, PathBuf};
 
+use libc::kill;
+
 use super::utils::{get_sys_value_str, WrapMap};
 
 #[doc(hidden)]
@@ -141,6 +143,20 @@ impl ProcessExt for Process {
     fn group_id(&self) -> Option<Gid> {
         Some(self.group_id)
     }
+
+    fn wait(&self) {
+        let mut status = 0;
+        // attempt waiting
+        unsafe {
+            if 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) unsafe fn get_process_data(
diff --git a/src/linux/process.rs b/src/linux/process.rs
index b71d97c..d7d61b5 100644
--- a/src/linux/process.rs
+++ b/src/linux/process.rs
@@ -219,6 +219,20 @@ impl ProcessExt for Process {
     fn group_id(&self) -> Option<Gid> {
         self.group_id
     }
+
+    fn wait(&self) {
+        let mut status = 0;
+        // attempt waiting
+        unsafe {
+            if 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 compute_cpu_usage(p: &mut Process, total_time: f32, max_value: f32) {
diff --git a/src/linux/system.rs b/src/linux/system.rs
index 2241327..3c4fce3 100644
--- a/src/linux/system.rs
+++ b/src/linux/system.rs
@@ -157,6 +157,7 @@ pub struct System {
     mem_available: u64,
     mem_buffers: u64,
     mem_page_cache: u64,
+    mem_shmem: u64,
     mem_slab_reclaimable: u64,
     swap_total: u64,
     swap_free: u64,
@@ -229,6 +230,7 @@ impl SystemExt for System {
             mem_available: 0,
             mem_buffers: 0,
             mem_page_cache: 0,
+            mem_shmem: 0,
             mem_slab_reclaimable: 0,
             swap_total: 0,
             swap_free: 0,
@@ -249,14 +251,20 @@ impl SystemExt for System {
 
     fn refresh_memory(&mut self) {
         if let Ok(data) = get_all_data("/proc/meminfo", 16_385) {
+            let mut mem_available_found = false;
+
             for line in data.split('\n') {
                 let mut iter = line.split(':');
                 let field = match iter.next() {
                     Some("MemTotal") => &mut self.mem_total,
                     Some("MemFree") => &mut self.mem_free,
-                    Some("MemAvailable") => &mut self.mem_available,
+                    Some("MemAvailable") => {
+                        mem_available_found = true;
+                        &mut self.mem_available
+                    }
                     Some("Buffers") => &mut self.mem_buffers,
                     Some("Cached") => &mut self.mem_page_cache,
+                    Some("Shmem") => &mut self.mem_shmem,
                     Some("SReclaimable") => &mut self.mem_slab_reclaimable,
                     Some("SwapTotal") => &mut self.swap_total,
                     Some("SwapFree") => &mut self.swap_free,
@@ -269,6 +277,17 @@ impl SystemExt for System {
                     }
                 }
             }
+
+            // Linux < 3.14 may not have MemAvailable in /proc/meminfo
+            // So it should fallback to the old way of estimating available memory
+            // https://github.com/KittyKatt/screenFetch/issues/386#issuecomment-249312716
+            if !mem_available_found {
+                self.mem_available = self.mem_free
+                    + self.mem_buffers
+                    + self.mem_page_cache
+                    + self.mem_slab_reclaimable
+                    - self.mem_shmem;
+            }
         }
     }
 
@@ -383,11 +402,7 @@ impl SystemExt for System {
     }
 
     fn used_memory(&self) -> u64 {
-        self.mem_total
-            - self.mem_free
-            - self.mem_buffers
-            - self.mem_page_cache
-            - self.mem_slab_reclaimable
+        self.mem_total - self.mem_available
     }
 
     fn total_swap(&self) -> u64 {
diff --git a/src/traits.rs b/src/traits.rs
index 3ad5402..8b70583 100644
--- a/src/traits.rs
+++ b/src/traits.rs
@@ -412,6 +412,21 @@ pub trait ProcessExt: Debug {
     /// }
     /// ```
     fn group_id(&self) -> Option<Gid>;
+
+    /// Wait for process termination.
+    ///
+    ///```no_run
+    /// use sysinfo::{Pid, ProcessExt, System, SystemExt};
+    ///
+    /// let mut s = System::new_all();
+    ///
+    /// if let Some(process) = s.process(Pid::from(1337)) {
+    ///     eprintln!("Waiting for pid 1337");
+    ///     process.wait();
+    ///     eprintln!("Pid 1337 exited");
+    /// }
+    ///````
+    fn wait(&self);
 }
 
 /// Contains all the methods of the [`Cpu`][crate::Cpu] struct.
@@ -1251,8 +1266,8 @@ pub trait SystemExt: Sized + Debug + Default + Send + Sync {
     /// or [`std::env::consts::OS`].
     ///
     /// See also
-    /// - https://www.freedesktop.org/software/systemd/man/os-release.html#ID=
-    /// - https://doc.rust-lang.org/std/env/consts/constant.OS.html
+    /// - <https://www.freedesktop.org/software/systemd/man/os-release.html#ID=>
+    /// - <https://doc.rust-lang.org/std/env/consts/constant.OS.html>
     ///
     /// **Important**: this information is computed every time this function is called.
     ///
diff --git a/src/unknown/process.rs b/src/unknown/process.rs
index c4656d3..4a12ec8 100644
--- a/src/unknown/process.rs
+++ b/src/unknown/process.rs
@@ -89,4 +89,6 @@ impl ProcessExt for Process {
     fn group_id(&self) -> Option<Gid> {
         None
     }
+
+    fn wait(&self) {}
 }
diff --git a/src/windows/process.rs b/src/windows/process.rs
index 888bc71..7561c65 100644
--- a/src/windows/process.rs
+++ b/src/windows/process.rs
@@ -1,5 +1,6 @@
 // Take a look at the license at the top of the repository in the LICENSE file.
 
+use crate::sys::system::is_proc_running;
 use crate::sys::utils::to_str;
 use crate::{DiskUsage, Gid, Pid, ProcessExt, ProcessRefreshKind, ProcessStatus, Signal, Uid};
 
@@ -547,6 +548,21 @@ impl ProcessExt for Process {
     fn group_id(&self) -> Option<Gid> {
         None
     }
+
+    fn wait(&self) {
+        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;
+                }
+                std::thread::sleep(std::time::Duration::from_millis(10));
+            }
+        } else {
+            // In this case, we can't do anything so we just return.
+            sysinfo_debug!("can't wait on this process so returning");
+        }
+    }
 }
 
 #[inline]
diff --git a/src/windows/system.rs b/src/windows/system.rs
index 205c212..718b6da 100644
--- a/src/windows/system.rs
+++ b/src/windows/system.rs
@@ -508,7 +508,7 @@ impl Default for System {
     }
 }
 
-fn is_proc_running(handle: HANDLE) -> bool {
+pub(crate) fn is_proc_running(handle: HANDLE) -> bool {
     let mut exit_code = 0;
     unsafe {
         let ret = GetExitCodeProcess(handle, &mut exit_code);


Test patch:
diff --git a/tests/process.rs b/tests/process.rs
index c200a60..1e0e74b 100644
--- a/tests/process.rs
+++ b/tests/process.rs
@@ -378,3 +378,78 @@ fn test_refresh_process() {
     // Checks that the process is still listed.
     assert!(s.process(pid).is_some());
 }
+
+#[test]
+fn test_wait_child() {
+    if !sysinfo::System::IS_SUPPORTED || cfg!(feature = "apple-sandbox") {
+        return;
+    }
+    let p = if cfg!(target_os = "windows") {
+        std::process::Command::new("waitfor")
+            .arg("/t")
+            .arg("300")
+            .arg("RefreshProcess")
+            .stdout(std::process::Stdio::null())
+            .spawn()
+            .unwrap()
+    } else {
+        std::process::Command::new("sleep")
+            .arg("300")
+            .stdout(std::process::Stdio::null())
+            .spawn()
+            .unwrap()
+    };
+
+    let before = std::time::Instant::now();
+    let pid = Pid::from_u32(p.id() as u32);
+
+    let mut s = sysinfo::System::new();
+    s.refresh_process(pid);
+    let process = s.process(pid).unwrap();
+
+    // Kill the child process.
+    process.kill();
+    // Wait for child process should work.
+    process.wait();
+
+    // Child process should not be present.
+    assert!(!s.refresh_process(pid));
+    assert!(before.elapsed() < std::time::Duration::from_millis(1000));
+}
+
+#[test]
+fn test_wait_non_child() {
+    if !sysinfo::System::IS_SUPPORTED || cfg!(feature = "apple-sandbox") {
+        return;
+    }
+
+    // spawn non child process.
+    let p = if !cfg!(target_os = "linux") {
+        return;
+    } else {
+        std::process::Command::new("setsid")
+            .arg("-w")
+            .arg("sleep")
+            .arg("2")
+            .stdout(std::process::Stdio::null())
+            .spawn()
+            .unwrap()
+    };
+    let pid = Pid::from_u32(p.id());
+
+    let before = std::time::Instant::now();
+
+    let mut s = sysinfo::System::new();
+    s.refresh_process(pid);
+    let process = s.process(pid).expect("Process not found!");
+
+    // Wait for a non child process.
+    process.wait();
+
+    // Child process should not be present.
+    assert!(!s.refresh_process(pid));
+
+    // should wait for 2s.
+    assert!(before.elapsed() > std::time::Duration::from_millis(2000));
+    assert!(before.elapsed() < std::time::Duration::from_millis(3000));
+}