From 7452b8d8281bfac80cc855f68981898e65d1f1d1 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Thu, 3 Oct 2024 23:05:52 +0200 Subject: [PATCH] Update `System::refresh_processes` API to give control over when to remove dead processes --- benches/basic.rs | 8 +- examples/simple.rs | 2 +- src/c_interface.rs | 52 +++++------ src/common/system.rs | 97 +++++++++++++++---- src/unix/apple/app_store/process.rs | 4 + src/unix/apple/macos/process.rs | 4 + src/unix/apple/system.rs | 15 ++- src/unix/freebsd/process.rs | 6 +- src/unix/freebsd/system.rs | 17 ++-- src/unix/linux/process.rs | 8 +- src/unix/linux/system.rs | 54 +++++------ src/unknown/process.rs | 4 + src/unknown/system.rs | 10 +- src/windows/process.rs | 4 + src/windows/system.rs | 18 ++-- tests/process.rs | 138 ++++++++++++++++++++-------- tests/system.rs | 20 ++-- 17 files changed, 299 insertions(+), 162 deletions(-) diff --git a/benches/basic.rs b/benches/basic.rs index 4d0f22fef..2853b1bbe 100644 --- a/benches/basic.rs +++ b/benches/basic.rs @@ -33,9 +33,9 @@ fn bench_refresh_all(b: &mut test::Bencher) { fn bench_refresh_processes(b: &mut test::Bencher) { let mut s = sysinfo::System::new(); - s.refresh_processes(sysinfo::ProcessesToUpdate::All); // to load the whole processes list a first time. + s.refresh_processes(sysinfo::ProcessesToUpdate::All, true); // to load the whole processes list a first time. b.iter(move || { - s.refresh_processes(sysinfo::ProcessesToUpdate::All); + s.refresh_processes(sysinfo::ProcessesToUpdate::All, true); }); } @@ -44,7 +44,7 @@ fn bench_refresh_processes(b: &mut test::Bencher) { fn bench_first_refresh_processes(b: &mut test::Bencher) { b.iter(move || { let mut s = sysinfo::System::new(); - s.refresh_processes(sysinfo::ProcessesToUpdate::All); + s.refresh_processes(sysinfo::ProcessesToUpdate::All, true); }); } @@ -57,7 +57,7 @@ fn bench_refresh_process(b: &mut test::Bencher) { // to be sure it'll exist for at least as long as we run let pid = sysinfo::get_current_pid().expect("failed to get current pid"); b.iter(move || { - s.refresh_processes(sysinfo::ProcessesToUpdate::Some(&[pid])); + s.refresh_processes(sysinfo::ProcessesToUpdate::Some(&[pid]), true); }); } diff --git a/examples/simple.rs b/examples/simple.rs index 5a671833c..efd003f3c 100644 --- a/examples/simple.rs +++ b/examples/simple.rs @@ -408,7 +408,7 @@ fn interpret_input( .take(1) .next() { - if sys.refresh_processes(sysinfo::ProcessesToUpdate::Some(&[pid])) != 0 { + if sys.refresh_processes(sysinfo::ProcessesToUpdate::Some(&[pid]), true) != 0 { writeln!(&mut io::stdout(), "Process `{pid}` updated successfully"); } else { writeln!(&mut io::stdout(), "Process `{pid}` couldn't be updated..."); diff --git a/src/c_interface.rs b/src/c_interface.rs index 592949a9c..c1d007f98 100644 --- a/src/c_interface.rs +++ b/src/c_interface.rs @@ -54,7 +54,7 @@ pub extern "C" fn sysinfo_refresh_memory(system: CSystem) { let system: &mut System = system.borrow_mut(); system.refresh_memory(); } - Box::into_raw(system); + let _ = Box::into_raw(system); } } @@ -68,7 +68,7 @@ pub extern "C" fn sysinfo_refresh_cpu(system: CSystem) { let system: &mut System = system.borrow_mut(); system.refresh_cpu_usage(); } - Box::into_raw(system); + let _ = Box::into_raw(system); } } @@ -82,7 +82,7 @@ pub extern "C" fn sysinfo_refresh_all(system: CSystem) { let system: &mut System = system.borrow_mut(); system.refresh_all(); } - Box::into_raw(system); + let _ = Box::into_raw(system); } } @@ -96,9 +96,9 @@ pub extern "C" fn sysinfo_refresh_processes(system: CSystem) { let mut system: Box = Box::from_raw(system as *mut System); { let system: &mut System = system.borrow_mut(); - system.refresh_processes(ProcessesToUpdate::All); + system.refresh_processes(ProcessesToUpdate::All, true); } - Box::into_raw(system); + let _ = Box::into_raw(system); } } @@ -112,9 +112,9 @@ pub extern "C" fn sysinfo_refresh_process(system: CSystem, pid: PID) { let mut system: Box = Box::from_raw(system as *mut System); { let system: &mut System = system.borrow_mut(); - system.refresh_processes(ProcessesToUpdate::Some(&[Pid::from_u32(pid as _)])); + system.refresh_processes(ProcessesToUpdate::Some(&[Pid::from_u32(pid as _)]), true); } - Box::into_raw(system); + let _ = Box::into_raw(system); } } @@ -144,7 +144,7 @@ pub extern "C" fn sysinfo_disks_refresh(disks: CDisks) { let disks: &mut Disks = disks.borrow_mut(); disks.refresh(); } - Box::into_raw(disks); + let _ = Box::into_raw(disks); } } @@ -158,7 +158,7 @@ pub extern "C" fn sysinfo_disks_refresh_list(disks: CDisks) { let disks: &mut Disks = disks.borrow_mut(); disks.refresh_list(); } - Box::into_raw(disks); + let _ = Box::into_raw(disks); } } @@ -169,7 +169,7 @@ pub extern "C" fn sysinfo_total_memory(system: CSystem) -> size_t { unsafe { let system: Box = Box::from_raw(system as *mut System); let ret = system.total_memory() as size_t; - Box::into_raw(system); + let _ = Box::into_raw(system); ret } } @@ -181,7 +181,7 @@ pub extern "C" fn sysinfo_free_memory(system: CSystem) -> size_t { unsafe { let system: Box = Box::from_raw(system as *mut System); let ret = system.free_memory() as size_t; - Box::into_raw(system); + let _ = Box::into_raw(system); ret } } @@ -192,7 +192,7 @@ pub extern "C" fn sysinfo_used_memory(system: CSystem) -> size_t { assert!(!system.is_null()); let system: Box = unsafe { Box::from_raw(system as *mut System) }; let ret = system.used_memory() as size_t; - Box::into_raw(system); + let _ = Box::into_raw(system); ret } @@ -203,7 +203,7 @@ pub extern "C" fn sysinfo_total_swap(system: CSystem) -> size_t { unsafe { let system: Box = Box::from_raw(system as *mut System); let ret = system.total_swap() as size_t; - Box::into_raw(system); + let _ = Box::into_raw(system); ret } } @@ -215,7 +215,7 @@ pub extern "C" fn sysinfo_free_swap(system: CSystem) -> size_t { unsafe { let system: Box = Box::from_raw(system as *mut System); let ret = system.free_swap() as size_t; - Box::into_raw(system); + let _ = Box::into_raw(system); ret } } @@ -227,7 +227,7 @@ pub extern "C" fn sysinfo_used_swap(system: CSystem) -> size_t { unsafe { let system: Box = Box::from_raw(system as *mut System); let ret = system.used_swap() as size_t; - Box::into_raw(system); + let _ = Box::into_raw(system); ret } } @@ -258,7 +258,7 @@ pub extern "C" fn sysinfo_networks_refresh_list(networks: CNetworks) { let networks: &mut Networks = networks.borrow_mut(); networks.refresh_list(); } - Box::into_raw(networks); + let _ = Box::into_raw(networks); } } @@ -272,7 +272,7 @@ pub extern "C" fn sysinfo_networks_refresh(networks: CNetworks) { let networks: &mut Networks = networks.borrow_mut(); networks.refresh(); } - Box::into_raw(networks); + let _ = Box::into_raw(networks); } } @@ -286,7 +286,7 @@ pub extern "C" fn sysinfo_networks_received(networks: CNetworks) -> size_t { let ret = networks.iter().fold(0, |acc: size_t, (_, data)| { acc.saturating_add(data.received() as size_t) }); - Box::into_raw(networks); + let _ = Box::into_raw(networks); ret } } @@ -301,7 +301,7 @@ pub extern "C" fn sysinfo_networks_transmitted(networks: CNetworks) -> size_t { let ret = networks.iter().fold(0, |acc: size_t, (_, data)| { acc.saturating_add(data.transmitted() as size_t) }); - Box::into_raw(networks); + let _ = Box::into_raw(networks); ret } } @@ -333,7 +333,7 @@ pub extern "C" fn sysinfo_cpus_usage( } *length = cpus.len() as c_uint - 1; } - Box::into_raw(system); + let _ = Box::into_raw(system); } } @@ -362,7 +362,7 @@ pub extern "C" fn sysinfo_processes( } entries.len() as size_t }; - Box::into_raw(system); + let _ = Box::into_raw(system); len } } else { @@ -386,7 +386,7 @@ pub extern "C" fn sysinfo_process_by_pid(system: CSystem, pid: PID) -> CProcess } else { std::ptr::null() }; - Box::into_raw(system); + let _ = Box::into_raw(system); ret } } @@ -534,7 +534,7 @@ pub extern "C" fn sysinfo_cpu_vendor_id(system: CSystem) -> RString { } else { std::ptr::null() }; - Box::into_raw(system); + let _ = Box::into_raw(system); c_string } } @@ -554,7 +554,7 @@ pub extern "C" fn sysinfo_cpu_brand(system: CSystem) -> RString { } else { std::ptr::null() }; - Box::into_raw(system); + let _ = Box::into_raw(system); c_string } } @@ -566,7 +566,7 @@ pub extern "C" fn sysinfo_cpu_physical_cores(system: CSystem) -> u32 { unsafe { let system: Box = Box::from_raw(system as *mut System); let count = system.physical_core_count().unwrap_or(0); - Box::into_raw(system); + let _ = Box::into_raw(system); count as u32 } } @@ -582,7 +582,7 @@ pub extern "C" fn sysinfo_cpu_frequency(system: CSystem) -> u64 { .first() .map(|cpu| cpu.frequency()) .unwrap_or(0); - Box::into_raw(system); + let _ = Box::into_raw(system); freq } } diff --git a/src/common/system.rs b/src/common/system.rs index 0e42628ef..588b8c09e 100644 --- a/src/common/system.rs +++ b/src/common/system.rs @@ -107,7 +107,7 @@ impl System { self.refresh_cpu_specifics(kind); } if let Some(kind) = refreshes.processes() { - self.refresh_processes_specifics(ProcessesToUpdate::All, kind); + self.refresh_processes_specifics(ProcessesToUpdate::All, false, kind); } } @@ -262,6 +262,7 @@ impl System { /// # let mut system = System::new(); /// system.refresh_processes_specifics( /// ProcessesToUpdate::All, + /// true, /// ProcessRefreshKind::new() /// .with_memory() /// .with_cpu() @@ -270,8 +271,9 @@ impl System { /// ); /// ``` /// - /// ⚠️ Unless `ProcessesToUpdate::All` is used, dead processes are not removed from - /// the set of processes kept in [`System`]. + /// ⚠️ `remove_dead_processes` works as follows: if an updated process is dead, then it is + /// removed. So if you refresh pids 1, 2 and 3. If 2 and 7 are dead, only 2 will be removed + /// since 7 is not part of the update. /// /// ⚠️ On Linux, `sysinfo` keeps the `stat` files open by default. You can change this behaviour /// by using [`set_open_files_limit`][crate::set_open_files_limit]. @@ -282,11 +284,16 @@ impl System { /// use sysinfo::{ProcessesToUpdate, System}; /// /// let mut s = System::new_all(); - /// s.refresh_processes(ProcessesToUpdate::All); + /// s.refresh_processes(ProcessesToUpdate::All, true); /// ``` - pub fn refresh_processes(&mut self, processes_to_update: ProcessesToUpdate<'_>) -> usize { + pub fn refresh_processes( + &mut self, + processes_to_update: ProcessesToUpdate<'_>, + remove_dead_processes: bool, + ) -> usize { self.refresh_processes_specifics( processes_to_update, + remove_dead_processes, ProcessRefreshKind::new() .with_memory() .with_cpu() @@ -299,8 +306,9 @@ impl System { /// /// Returns the number of updated processes. /// - /// ⚠️ Unless `ProcessesToUpdate::All` is used, dead processes are not removed from - /// the set of processes kept in [`System`]. + /// ⚠️ `remove_dead_processes` works as follows: if an updated process is dead, then it is + /// removed. So if you refresh pids 1, 2 and 3. If 2 and 7 are dead, only 2 will be removed + /// since 7 is not part of the update. /// /// ⚠️ On Linux, `sysinfo` keeps the `stat` files open by default. You can change this behaviour /// by using [`set_open_files_limit`][crate::set_open_files_limit]. @@ -309,15 +317,60 @@ impl System { /// use sysinfo::{ProcessesToUpdate, ProcessRefreshKind, System}; /// /// let mut s = System::new_all(); - /// s.refresh_processes_specifics(ProcessesToUpdate::All, ProcessRefreshKind::new()); + /// s.refresh_processes_specifics( + /// ProcessesToUpdate::All, + /// true, + /// ProcessRefreshKind::everything(), + /// ); /// ``` pub fn refresh_processes_specifics( &mut self, processes_to_update: ProcessesToUpdate<'_>, + remove_dead_processes: bool, refresh_kind: ProcessRefreshKind, ) -> usize { - self.inner - .refresh_processes_specifics(processes_to_update, refresh_kind) + fn update_and_remove(pid: &Pid, processes: &mut HashMap) { + let updated = if let Some(proc) = processes.get_mut(pid) { + proc.inner.switch_updated() + } else { + return; + }; + if !updated { + processes.remove(pid); + } + } + fn update(pid: &Pid, processes: &mut HashMap) { + if let Some(proc) = processes.get_mut(pid) { + proc.inner.switch_updated(); + } + } + + let nb_updated = self + .inner + .refresh_processes_specifics(processes_to_update, refresh_kind); + let processes = self.inner.processes_mut(); + match processes_to_update { + ProcessesToUpdate::All => { + if remove_dead_processes { + processes.retain(|_, v| v.inner.switch_updated()); + } else { + for proc in processes.values_mut() { + proc.inner.switch_updated(); + } + } + } + ProcessesToUpdate::Some(pids) => { + let call = if remove_dead_processes { + update_and_remove + } else { + update + }; + for pid in pids { + call(pid, processes); + } + } + } + nb_updated } /// Returns the process list. @@ -1371,6 +1424,7 @@ impl Process { /// // Refresh CPU usage to get actual value. /// s.refresh_processes_specifics( /// ProcessesToUpdate::All, + /// true, /// ProcessRefreshKind::new().with_cpu() /// ); /// if let Some(process) = s.process(Pid::from(1337)) { @@ -1844,6 +1898,7 @@ assert_eq!(r.", stringify!($name), "().is_some(), false); /// let mut system = System::new(); /// system.refresh_processes_specifics( /// ProcessesToUpdate::All, +/// true, /// ProcessRefreshKind::new().with_exe(UpdateKind::OnlyIfNotSet), /// ); /// ``` @@ -1880,11 +1935,12 @@ impl UpdateKind { /// /// let mut system = System::new(); /// // To refresh all processes: -/// system.refresh_processes(ProcessesToUpdate::All); +/// system.refresh_processes(ProcessesToUpdate::All, true); /// /// // To refresh only the current one: /// system.refresh_processes( /// ProcessesToUpdate::Some(&[get_current_pid().unwrap()]), +/// true, /// ); /// ``` #[derive(Clone, Copy, Debug, PartialEq, Eq)] @@ -1917,6 +1973,7 @@ pub enum ProcessesToUpdate<'a> { /// // We don't want to update the CPU information. /// system.refresh_processes_specifics( /// ProcessesToUpdate::All, +/// true, /// ProcessRefreshKind::everything().without_cpu(), /// ); /// @@ -2393,7 +2450,7 @@ mod test { } let mut s = System::new_all(); let total = s.processes().len() as isize; - s.refresh_processes(ProcessesToUpdate::All); + s.refresh_processes(ProcessesToUpdate::All, false); let new_total = s.processes().len() as isize; // There should be almost no difference in the processes count. assert!( @@ -2415,7 +2472,7 @@ mod test { return; } let mut s = System::new(); - s.refresh_processes(ProcessesToUpdate::All); + s.refresh_processes(ProcessesToUpdate::All, false); for proc_ in s.cpus() { assert_eq!(proc_.frequency(), 0); } @@ -2490,7 +2547,7 @@ mod test { } let mut s = System::new(); - s.refresh_processes(ProcessesToUpdate::All); + s.refresh_processes(ProcessesToUpdate::All, false); // All CPU usage will start at zero until the second refresh assert!(s .processes() @@ -2499,7 +2556,7 @@ mod test { // Wait a bit to update CPU usage values std::thread::sleep(MINIMUM_CPU_UPDATE_INTERVAL); - s.refresh_processes(ProcessesToUpdate::All); + s.refresh_processes(ProcessesToUpdate::All, true); assert!(s .processes() .iter() @@ -2571,9 +2628,15 @@ mod test { { let mut s = System::new(); // First check what happens in case the process isn't already in our process list. - assert_eq!(s.refresh_processes(ProcessesToUpdate::Some(&[_pid])), 1); + assert_eq!( + s.refresh_processes(ProcessesToUpdate::Some(&[_pid]), true), + 1 + ); // Then check that it still returns 1 if the process is already in our process list. - assert_eq!(s.refresh_processes(ProcessesToUpdate::Some(&[_pid])), 1); + assert_eq!( + s.refresh_processes(ProcessesToUpdate::Some(&[_pid]), true), + 1 + ); } } } diff --git a/src/unix/apple/app_store/process.rs b/src/unix/apple/app_store/process.rs index e23c7498d..0a52ef6fe 100644 --- a/src/unix/apple/app_store/process.rs +++ b/src/unix/apple/app_store/process.rs @@ -93,4 +93,8 @@ impl ProcessInner { pub(crate) fn session_id(&self) -> Option { None } + + pub(crate) fn switch_updated(&mut self) -> bool { + false + } } diff --git a/src/unix/apple/macos/process.rs b/src/unix/apple/macos/process.rs index 27d1655d8..08a33dd1b 100644 --- a/src/unix/apple/macos/process.rs +++ b/src/unix/apple/macos/process.rs @@ -226,6 +226,10 @@ impl ProcessInner { } } } + + pub(crate) fn switch_updated(&mut self) -> bool { + std::mem::replace(&mut self.updated, false) + } } #[allow(deprecated)] // Because of libc::mach_absolute_time. diff --git a/src/unix/apple/system.rs b/src/unix/apple/system.rs index 77aaf84f5..2d5014f8c 100644 --- a/src/unix/apple/system.rs +++ b/src/unix/apple/system.rs @@ -260,17 +260,16 @@ impl SystemInner { } #[allow(clippy::type_complexity)] - let (filter, filter_callback, remove_processes): ( + let (filter, filter_callback): ( &[Pid], &(dyn Fn(Pid, &[Pid]) -> bool + Sync + Send), - bool, ) = match processes_to_update { - ProcessesToUpdate::All => (&[], &empty_filter, true), + ProcessesToUpdate::All => (&[], &empty_filter), ProcessesToUpdate::Some(pids) => { if pids.is_empty() { return 0; } - (pids, &real_filter, false) + (pids, &real_filter) } }; @@ -298,10 +297,6 @@ impl SystemInner { entries.into_iter().for_each(|entry| { self.process_list.insert(entry.pid(), entry); }); - if remove_processes { - self.process_list - .retain(|_, proc_| std::mem::replace(&mut proc_.inner.updated, false)); - } nb_updated.into_inner() } else { 0 @@ -316,6 +311,10 @@ impl SystemInner { &self.process_list } + pub(crate) fn processes_mut(&mut self) -> &mut HashMap { + &mut self.process_list + } + pub(crate) fn process(&self, pid: Pid) -> Option<&Process> { self.process_list.get(&pid) } diff --git a/src/unix/freebsd/process.rs b/src/unix/freebsd/process.rs index 3abc3486c..ac946e70b 100644 --- a/src/unix/freebsd/process.rs +++ b/src/unix/freebsd/process.rs @@ -178,6 +178,10 @@ impl ProcessInner { } } } + + pub(crate) fn switch_updated(&mut self) -> bool { + std::mem::replace(&mut self.updated, false) + } } pub(crate) unsafe fn get_process_data( @@ -293,7 +297,7 @@ pub(crate) unsafe fn get_process_data( old_read_bytes: 0, written_bytes: kproc.ki_rusage.ru_oublock as _, old_written_bytes: 0, - updated: false, + updated: true, }, })) } diff --git a/src/unix/freebsd/system.rs b/src/unix/freebsd/system.rs index 80d4c5e5d..74ba25c78 100644 --- a/src/unix/freebsd/system.rs +++ b/src/unix/freebsd/system.rs @@ -131,6 +131,10 @@ impl SystemInner { &self.process_list } + pub(crate) fn processes_mut(&mut self) -> &mut HashMap { + &mut self.process_list + } + pub(crate) fn process(&self, pid: Pid) -> Option<&Process> { self.process_list.get(&pid) } @@ -294,17 +298,16 @@ impl SystemInner { } #[allow(clippy::type_complexity)] - let (filter, filter_callback, remove_processes): ( + let (filter, filter_callback): ( &[Pid], &(dyn Fn(&libc::kinfo_proc, &[Pid]) -> bool + Sync + Send), - bool, ) = match processes_to_update { - ProcessesToUpdate::All => (&[], &empty_filter, true), + ProcessesToUpdate::All => (&[], &empty_filter), ProcessesToUpdate::Some(pids) => { if pids.is_empty() { return 0; } - (pids, &real_filter, false) + (pids, &real_filter) } }; @@ -343,12 +346,6 @@ impl SystemInner { .collect::>() }; - if remove_processes { - // We remove all processes that don't exist anymore. - self.process_list - .retain(|_, v| std::mem::replace(&mut v.inner.updated, false)); - } - for process in new_processes { self.process_list.insert(process.inner.pid, process); } diff --git a/src/unix/linux/process.rs b/src/unix/linux/process.rs index f3438a9e8..cd2a5d056 100644 --- a/src/unix/linux/process.rs +++ b/src/unix/linux/process.rs @@ -278,6 +278,10 @@ impl ProcessInner { pub(crate) fn thread_kind(&self) -> Option { self.thread_kind } + + pub(crate) fn switch_updated(&mut self) -> bool { + std::mem::replace(&mut self.updated, false) + } } pub(crate) fn compute_cpu_usage(p: &mut ProcessInner, total_time: f32, max_value: f32) { @@ -297,10 +301,6 @@ pub(crate) fn compute_cpu_usage(p: &mut ProcessInner, total_time: f32, max_value .min(max_value); } -pub(crate) fn unset_updated(p: &mut ProcessInner) { - p.updated = false; -} - pub(crate) fn set_time(p: &mut ProcessInner, utime: u64, stime: u64) { p.old_utime = p.utime; p.old_stime = p.stime; diff --git a/src/unix/linux/system.rs b/src/unix/linux/system.rs index 1112dfbe1..33001e095 100644 --- a/src/unix/linux/system.rs +++ b/src/unix/linux/system.rs @@ -1,7 +1,7 @@ // Take a look at the license at the top of the repository in the LICENSE file. use crate::sys::cpu::{get_physical_core_count, CpusWrapper}; -use crate::sys::process::{compute_cpu_usage, refresh_procs, unset_updated}; +use crate::sys::process::{compute_cpu_usage, refresh_procs}; use crate::sys::utils::{get_all_utf8_data, to_u64}; use crate::{Cpu, CpuRefreshKind, LoadAvg, MemoryRefreshKind, Pid, Process, ProcessesToUpdate, ProcessRefreshKind}; @@ -173,38 +173,24 @@ impl SystemInner { self.cpus.len() as f32 * 100. } - fn clear_procs(&mut self, refresh_kind: ProcessRefreshKind) { - let (total_time, compute_cpu, max_value) = if refresh_kind.cpu() { - self.cpus - .refresh_if_needed(true, CpuRefreshKind::new().with_cpu_usage()); + fn update_procs_cpu(&mut self, refresh_kind: ProcessRefreshKind) { + if !refresh_kind.cpu() { + return; + } + self.cpus.refresh_if_needed(true, CpuRefreshKind::new().with_cpu_usage()); - if self.cpus.is_empty() { - sysinfo_debug!("cannot compute processes CPU usage: no CPU found..."); - (0., false, 0.) - } else { - let (new, old) = self.cpus.get_global_raw_times(); - let total_time = if old > new { 1 } else { new - old }; - ( - total_time as f32 / self.cpus.len() as f32, - true, - self.get_max_process_cpu_usage(), - ) - } - } else { - (0., false, 0.) - }; + if self.cpus.is_empty() { + sysinfo_debug!("cannot compute processes CPU usage: no CPU found..."); + return; + } + let (new, old) = self.cpus.get_global_raw_times(); + let total_time = if old > new { 1 } else { new - old }; + let total_time = total_time as f32 / self.cpus.len() as f32; + let max_value = self.get_max_process_cpu_usage(); - self.process_list.retain(|_, proc_| { - let proc_ = &mut proc_.inner; - if !proc_.updated { - return false; - } - if compute_cpu { - compute_cpu_usage(proc_, total_time, max_value); - } - unset_updated(proc_); - true - }); + for proc_ in self.process_list.values_mut() { + compute_cpu_usage(&mut proc_.inner, total_time, max_value); + } } fn refresh_cpus(&mut self, only_update_global_cpu: bool, refresh_kind: CpuRefreshKind) { @@ -291,7 +277,7 @@ impl SystemInner { refresh_kind, ); if matches!(processes_to_update, ProcessesToUpdate::All) { - self.clear_procs(refresh_kind); + self.update_procs_cpu(refresh_kind); self.cpus.set_need_cpus_update(); } nb_updated @@ -305,6 +291,10 @@ impl SystemInner { &self.process_list } + pub(crate) fn processes_mut(&mut self) -> &mut HashMap { + &mut self.process_list + } + pub(crate) fn process(&self, pid: Pid) -> Option<&Process> { self.process_list.get(&pid) } diff --git a/src/unknown/process.rs b/src/unknown/process.rs index 00d8e4b4a..3de87f408 100644 --- a/src/unknown/process.rs +++ b/src/unknown/process.rs @@ -103,4 +103,8 @@ impl ProcessInner { pub(crate) fn session_id(&self) -> Option { None } + + pub(crate) fn switch_updated(&mut self) -> bool { + false + } } diff --git a/src/unknown/system.rs b/src/unknown/system.rs index 7395c864a..c5a81fa82 100644 --- a/src/unknown/system.rs +++ b/src/unknown/system.rs @@ -16,13 +16,13 @@ pub const SUPPORTED_SIGNALS: &[crate::Signal] = supported_signals(); pub const MINIMUM_CPU_UPDATE_INTERVAL: Duration = Duration::from_millis(0); pub(crate) struct SystemInner { - processes_list: HashMap, + process_list: HashMap, } impl SystemInner { pub(crate) fn new() -> Self { Self { - processes_list: Default::default(), + process_list: Default::default(), } } @@ -49,7 +49,11 @@ impl SystemInner { // Need to be moved into a "common" file to avoid duplication. pub(crate) fn processes(&self) -> &HashMap { - &self.processes_list + &self.process_list + } + + pub(crate) fn processes_mut(&mut self) -> &mut HashMap { + &mut self.process_list } pub(crate) fn process(&self, _pid: Pid) -> Option<&Process> { diff --git a/src/windows/process.rs b/src/windows/process.rs index 79ff91101..e881906b0 100644 --- a/src/windows/process.rs +++ b/src/windows/process.rs @@ -443,6 +443,10 @@ impl ProcessInner { None } } + + pub(crate) fn switch_updated(&mut self) -> bool { + std::mem::replace(&mut self.updated, false) + } } #[inline] diff --git a/src/windows/system.rs b/src/windows/system.rs index 4cce483ba..a42ed49ea 100644 --- a/src/windows/system.rs +++ b/src/windows/system.rs @@ -10,7 +10,7 @@ use crate::utils::into_iter; use std::cell::UnsafeCell; use std::collections::HashMap; use std::ffi::{OsStr, OsString}; -use std::mem::{replace, size_of, zeroed}; +use std::mem::{size_of, zeroed}; use std::os::windows::ffi::{OsStrExt, OsStringExt}; use std::ptr; use std::sync::atomic::{AtomicUsize, Ordering}; @@ -199,17 +199,16 @@ impl SystemInner { } #[allow(clippy::type_complexity)] - let (filter_array, filter_callback, remove_processes): ( + let (filter_array, filter_callback): ( &[Pid], &(dyn Fn(Pid, &[Pid]) -> bool + Sync + Send), - bool, ) = match processes_to_update { - ProcessesToUpdate::All => (&[], &empty_filter, true), + ProcessesToUpdate::All => (&[], &empty_filter), ProcessesToUpdate::Some(pids) => { if pids.is_empty() { return 0; } - (pids, &real_filter, false) + (pids, &real_filter) } }; @@ -337,11 +336,6 @@ impl SystemInner { for p in processes.into_iter() { self.process_list.insert(p.pid(), p); } - if remove_processes { - // If it comes from `refresh_process` or `refresh_pids`, we don't remove - // dead processes. - self.process_list.retain(|_, v| replace(&mut v.inner.updated, false)); - } nb_updated.into_inner() } } @@ -350,6 +344,10 @@ impl SystemInner { &self.process_list } + pub(crate) fn processes_mut(&mut self) -> &mut HashMap { + &mut self.process_list + } + pub(crate) fn process(&self, pid: Pid) -> Option<&Process> { self.process_list.get(&pid) } diff --git a/tests/process.rs b/tests/process.rs index 869cddfe0..8aa2690e3 100644 --- a/tests/process.rs +++ b/tests/process.rs @@ -37,6 +37,7 @@ fn test_cwd() { let mut s = System::new(); s.refresh_processes_specifics( ProcessesToUpdate::All, + false, ProcessRefreshKind::new().with_cwd(UpdateKind::Always), ); p.kill().expect("Unable to kill process."); @@ -63,6 +64,7 @@ fn test_cmd() { assert!(s.processes().is_empty()); s.refresh_processes_specifics( ProcessesToUpdate::All, + false, ProcessRefreshKind::new().with_cmd(UpdateKind::Always), ); p.kill().expect("Unable to kill process"); @@ -94,6 +96,7 @@ fn build_test_binary(file_name: &str) { } #[test] +#[allow(clippy::zombie_processes)] fn test_environ() { if !sysinfo::IS_SUPPORTED_SYSTEM || cfg!(feature = "apple-sandbox") { return; @@ -112,6 +115,7 @@ fn test_environ() { s.refresh_processes_specifics( ProcessesToUpdate::Some(&[pid]), + false, sysinfo::ProcessRefreshKind::everything(), ); p.kill().expect("Unable to kill process."); @@ -145,6 +149,7 @@ fn test_environ() { s.refresh_processes_specifics( ProcessesToUpdate::All, + false, ProcessRefreshKind::new().with_environ(UpdateKind::Always), ); @@ -169,9 +174,10 @@ fn test_process_refresh() { if !sysinfo::IS_SUPPORTED_SYSTEM || cfg!(feature = "apple-sandbox") { return; } - s.refresh_processes(ProcessesToUpdate::Some(&[ - sysinfo::get_current_pid().expect("failed to get current pid") - ])); + s.refresh_processes( + ProcessesToUpdate::Some(&[sysinfo::get_current_pid().expect("failed to get current pid")]), + false, + ); assert!(s .process(sysinfo::get_current_pid().expect("failed to get current pid")) .is_some()); @@ -213,7 +219,7 @@ fn test_process_disk_usage() { std::thread::sleep(std::time::Duration::from_millis(250)); let mut system = System::new(); assert!(system.processes().is_empty()); - system.refresh_processes(ProcessesToUpdate::All); + system.refresh_processes(ProcessesToUpdate::All, false); assert!(!system.processes().is_empty()); system } @@ -249,7 +255,7 @@ fn test_process_disk_usage() { #[test] fn cpu_usage_is_not_nan() { let mut system = System::new(); - system.refresh_processes(ProcessesToUpdate::All); + system.refresh_processes(ProcessesToUpdate::All, false); if !sysinfo::IS_SUPPORTED_SYSTEM || cfg!(feature = "apple-sandbox") { return; @@ -266,7 +272,7 @@ fn cpu_usage_is_not_nan() { let mut checked = 0; first_pids.into_iter().for_each(|pid| { - system.refresh_processes(ProcessesToUpdate::Some(&[pid])); + system.refresh_processes(ProcessesToUpdate::Some(&[pid]), true); if let Some(p) = system.process(pid) { assert!(!p.cpu_usage().is_nan()); checked += 1; @@ -289,7 +295,7 @@ fn test_process_times() { let pid = Pid::from_u32(p.id() as _); std::thread::sleep(std::time::Duration::from_secs(1)); let mut s = System::new(); - s.refresh_processes(ProcessesToUpdate::All); + s.refresh_processes(ProcessesToUpdate::All, false); p.kill().expect("Unable to kill process."); if let Some(p) = s.process(pid) { @@ -319,7 +325,7 @@ fn test_process_session_id() { return; } let mut s = System::new(); - s.refresh_processes(ProcessesToUpdate::All); + s.refresh_processes(ProcessesToUpdate::All, false); assert!(s.processes().values().any(|p| p.session_id().is_some())); } @@ -336,7 +342,7 @@ fn test_refresh_processes() { // Checks that the process is listed as it should. let mut s = System::new(); - s.refresh_processes(ProcessesToUpdate::All); + s.refresh_processes(ProcessesToUpdate::All, false); assert!(s.process(pid).is_some()); // Check that the process name is not empty. @@ -348,12 +354,12 @@ fn test_refresh_processes() { // Let's give some time to the system to clean up... std::thread::sleep(std::time::Duration::from_secs(1)); - s.refresh_processes(ProcessesToUpdate::All); + s.refresh_processes(ProcessesToUpdate::All, true); // Checks that the process isn't listed anymore. assert!(s.process(pid).is_none()); } -// This test ensures that if we refresh only one process, then no process is removed. +// This test ensures that if we refresh only one process, then only this process is removed. #[test] fn test_refresh_process_doesnt_remove() { if !sysinfo::IS_SUPPORTED_SYSTEM || cfg!(feature = "apple-sandbox") { @@ -370,7 +376,7 @@ fn test_refresh_process_doesnt_remove() { let mut s = System::new_with_specifics( RefreshKind::new().with_processes(sysinfo::ProcessRefreshKind::new()), ); - s.refresh_processes(ProcessesToUpdate::All); + s.refresh_processes(ProcessesToUpdate::All, false); assert!(s.process(pid1).is_some()); assert!(s.process(pid2).is_some()); @@ -384,11 +390,23 @@ fn test_refresh_process_doesnt_remove() { // Let's give some time to the system to clean up... std::thread::sleep(std::time::Duration::from_secs(1)); - assert_eq!(s.refresh_processes(ProcessesToUpdate::Some(&[pid1])), 0); + assert_eq!( + s.refresh_processes(ProcessesToUpdate::Some(&[pid1]), false), + 0 + ); // We check that none of the two processes were removed. assert!(s.process(pid1).is_some()); assert!(s.process(pid2).is_some()); + + assert_eq!( + s.refresh_processes(ProcessesToUpdate::Some(&[pid1]), true), + 0 + ); + + // We check that only `pid1` was removed. + assert!(s.process(pid1).is_none()); + assert!(s.process(pid2).is_some()); } // Checks that `refresh_processes` is adding and removing task. @@ -413,7 +431,7 @@ fn test_refresh_tasks() { // Checks that the task is listed as it should. let mut s = System::new(); - s.refresh_processes(ProcessesToUpdate::All); + s.refresh_processes(ProcessesToUpdate::All, false); assert!(s .process(pid) @@ -432,7 +450,7 @@ fn test_refresh_tasks() { // Let's give some time to the system to clean up... std::thread::sleep(std::time::Duration::from_secs(2)); - s.refresh_processes(ProcessesToUpdate::All); + s.refresh_processes(ProcessesToUpdate::All, true); assert!(!s .process(pid) @@ -449,7 +467,7 @@ fn test_refresh_tasks() { .is_none()); } -// Checks that `refresh_process` is NOT removing dead processes. +// Checks that `refresh_process` is removing dead processes when asked. #[test] fn test_refresh_process() { if !sysinfo::IS_SUPPORTED_SYSTEM || cfg!(feature = "apple-sandbox") { @@ -462,7 +480,7 @@ fn test_refresh_process() { // Checks that the process is listed as it should. let mut s = System::new(); - s.refresh_processes(ProcessesToUpdate::Some(&[pid])); + s.refresh_processes(ProcessesToUpdate::Some(&[pid]), false); assert!(s.process(pid).is_some()); // Check that the process name is not empty. @@ -474,9 +492,19 @@ fn test_refresh_process() { // Let's give some time to the system to clean up... std::thread::sleep(std::time::Duration::from_secs(1)); - assert_eq!(s.refresh_processes(ProcessesToUpdate::Some(&[pid])), 0); + assert_eq!( + s.refresh_processes(ProcessesToUpdate::Some(&[pid]), false), + 0 + ); // Checks that the process is still listed. assert!(s.process(pid).is_some()); + + assert_eq!( + s.refresh_processes(ProcessesToUpdate::Some(&[pid]), true), + 0 + ); + // Checks that the process is not listed anymore. + assert!(s.process(pid).is_none()); } #[test] @@ -490,7 +518,7 @@ fn test_wait_child() { let pid = Pid::from_u32(p.id() as _); let mut s = System::new(); - s.refresh_processes(ProcessesToUpdate::Some(&[pid])); + s.refresh_processes(ProcessesToUpdate::Some(&[pid]), false); let process = s.process(pid).unwrap(); // Kill the child process. @@ -499,7 +527,10 @@ fn test_wait_child() { process.wait(); // Child process should not be present. - assert_eq!(s.refresh_processes(ProcessesToUpdate::Some(&[pid])), 0); + assert_eq!( + s.refresh_processes(ProcessesToUpdate::Some(&[pid]), true), + 0 + ); assert!(before.elapsed() < std::time::Duration::from_millis(1000)); } @@ -526,14 +557,17 @@ fn test_wait_non_child() { let pid = Pid::from_u32(p.id()); let mut s = System::new(); - s.refresh_processes(ProcessesToUpdate::Some(&[pid])); + s.refresh_processes(ProcessesToUpdate::Some(&[pid]), false); let process = s.process(pid).expect("Process not found!"); // Wait for a non child process. process.wait(); // Child process should not be present. - assert_eq!(s.refresh_processes(ProcessesToUpdate::Some(&[pid])), 0); + assert_eq!( + s.refresh_processes(ProcessesToUpdate::Some(&[pid]), true), + 0 + ); // should wait for 2s. assert!( @@ -668,13 +702,13 @@ fn test_process_specific_refresh() { macro_rules! update_specific_and_check { (memory) => { - s.refresh_processes_specifics(ProcessesToUpdate::Some(&[pid]), ProcessRefreshKind::new()); + s.refresh_processes_specifics(ProcessesToUpdate::Some(&[pid]), false, ProcessRefreshKind::new()); { let p = s.process(pid).unwrap(); assert_eq!(p.memory(), 0, "failed 0 check for memory"); assert_eq!(p.virtual_memory(), 0, "failed 0 check for virtual memory"); } - s.refresh_processes_specifics(ProcessesToUpdate::Some(&[pid]), ProcessRefreshKind::new().with_memory()); + s.refresh_processes_specifics(ProcessesToUpdate::Some(&[pid]), false, ProcessRefreshKind::new().with_memory()); { let p = s.process(pid).unwrap(); assert_ne!(p.memory(), 0, "failed non-0 check for memory"); @@ -682,7 +716,7 @@ fn test_process_specific_refresh() { } // And now we check that re-refreshing nothing won't remove the // information. - s.refresh_processes_specifics(ProcessesToUpdate::Some(&[pid]), ProcessRefreshKind::new()); + s.refresh_processes_specifics(ProcessesToUpdate::Some(&[pid]), false, ProcessRefreshKind::new()); { let p = s.process(pid).unwrap(); assert_ne!(p.memory(), 0, "failed non-0 check (number 2) for memory"); @@ -690,7 +724,7 @@ fn test_process_specific_refresh() { } }; ($name:ident, $method:ident, $($extra:tt)+) => { - s.refresh_processes_specifics(ProcessesToUpdate::Some(&[pid]), ProcessRefreshKind::new()); + s.refresh_processes_specifics(ProcessesToUpdate::Some(&[pid]), false, ProcessRefreshKind::new()); { let p = s.process(pid).unwrap(); assert_eq!( @@ -698,7 +732,7 @@ fn test_process_specific_refresh() { concat!("failed 0 check check for ", stringify!($name)), ); } - s.refresh_processes_specifics(ProcessesToUpdate::Some(&[pid]), ProcessRefreshKind::new().$method(UpdateKind::Always)); + s.refresh_processes_specifics(ProcessesToUpdate::Some(&[pid]), false, ProcessRefreshKind::new().$method(UpdateKind::Always)); { let p = s.process(pid).unwrap(); assert_ne!( @@ -707,7 +741,7 @@ fn test_process_specific_refresh() { } // And now we check that re-refreshing nothing won't remove the // information. - s.refresh_processes_specifics(ProcessesToUpdate::Some(&[pid]), ProcessRefreshKind::new()); + s.refresh_processes_specifics(ProcessesToUpdate::Some(&[pid]), false, ProcessRefreshKind::new()); { let p = s.process(pid).unwrap(); assert_ne!( @@ -717,10 +751,18 @@ fn test_process_specific_refresh() { } } - s.refresh_processes_specifics(ProcessesToUpdate::Some(&[pid]), ProcessRefreshKind::new()); + s.refresh_processes_specifics( + ProcessesToUpdate::Some(&[pid]), + false, + ProcessRefreshKind::new(), + ); check_empty(&s, pid); - s.refresh_processes_specifics(ProcessesToUpdate::Some(&[pid]), ProcessRefreshKind::new()); + s.refresh_processes_specifics( + ProcessesToUpdate::Some(&[pid]), + false, + ProcessRefreshKind::new(), + ); check_empty(&s, pid); update_specific_and_check!(memory); @@ -750,7 +792,7 @@ fn test_refresh_pids() { let child_pid = Pid::from_u32(p.id() as _); let pids = &[child_pid, self_pid]; std::thread::sleep(std::time::Duration::from_millis(500)); - s.refresh_processes(ProcessesToUpdate::Some(pids)); + s.refresh_processes(ProcessesToUpdate::Some(pids), false); p.kill().expect("Unable to kill process."); assert_eq!(s.processes().len(), 2); @@ -766,10 +808,10 @@ fn test_process_run_time() { } let mut s = System::new(); let current_pid = sysinfo::get_current_pid().expect("failed to get current pid"); - s.refresh_processes(ProcessesToUpdate::Some(&[current_pid])); + s.refresh_processes(ProcessesToUpdate::Some(&[current_pid]), false); let run_time = s.process(current_pid).expect("no process found").run_time(); std::thread::sleep(std::time::Duration::from_secs(2)); - s.refresh_processes(ProcessesToUpdate::Some(&[current_pid])); + s.refresh_processes(ProcessesToUpdate::Some(&[current_pid]), true); let new_run_time = s.process(current_pid).expect("no process found").run_time(); assert!( new_run_time > run_time, @@ -799,7 +841,7 @@ fn test_parent_change() { let pid = Pid::from_u32(p.id() as _); let mut s = System::new(); - s.refresh_processes(ProcessesToUpdate::All); + s.refresh_processes(ProcessesToUpdate::All, false); assert_eq!( s.process(pid).expect("process was not created").parent(), @@ -816,7 +858,7 @@ fn test_parent_change() { // Waiting for the parent process to stop. p.wait().expect("wait failed"); - s.refresh_processes(ProcessesToUpdate::All); + s.refresh_processes(ProcessesToUpdate::All, true); // Parent should not be around anymore. assert!(s.process(pid).is_none()); @@ -829,7 +871,7 @@ fn test_parent_change() { } // We want to ensure that if `System::refresh_process*` methods are called -// one after the other, it won't impact the CPU usage computation badly. +// one after the other, it won't badly impact the CPU usage computation. #[test] fn test_multiple_single_process_refresh() { if !sysinfo::IS_SUPPORTED_SYSTEM || cfg!(feature = "apple-sandbox") || cfg!(windows) { @@ -853,12 +895,28 @@ fn test_multiple_single_process_refresh() { let mut s = System::new(); let process_refresh_kind = ProcessRefreshKind::new().with_cpu(); - s.refresh_processes_specifics(ProcessesToUpdate::Some(&[pid_a]), process_refresh_kind); - s.refresh_processes_specifics(ProcessesToUpdate::Some(&[pid_b]), process_refresh_kind); + s.refresh_processes_specifics( + ProcessesToUpdate::Some(&[pid_a]), + false, + process_refresh_kind, + ); + s.refresh_processes_specifics( + ProcessesToUpdate::Some(&[pid_b]), + false, + process_refresh_kind, + ); std::thread::sleep(std::time::Duration::from_secs(1)); - s.refresh_processes_specifics(ProcessesToUpdate::Some(&[pid_a]), process_refresh_kind); - s.refresh_processes_specifics(ProcessesToUpdate::Some(&[pid_b]), process_refresh_kind); + s.refresh_processes_specifics( + ProcessesToUpdate::Some(&[pid_a]), + true, + process_refresh_kind, + ); + s.refresh_processes_specifics( + ProcessesToUpdate::Some(&[pid_b]), + true, + process_refresh_kind, + ); let cpu_a = s.process(pid_a).unwrap().cpu_usage(); let cpu_b = s.process(pid_b).unwrap().cpu_usage(); diff --git a/tests/system.rs b/tests/system.rs index 11faf2e4d..dac837dca 100644 --- a/tests/system.rs +++ b/tests/system.rs @@ -28,9 +28,12 @@ fn test_refresh_process() { #[cfg(not(feature = "apple-sandbox"))] if sysinfo::IS_SUPPORTED_SYSTEM { assert_eq!( - sys.refresh_processes(ProcessesToUpdate::Some(&[ - sysinfo::get_current_pid().expect("failed to get current pid") - ])), + sys.refresh_processes( + ProcessesToUpdate::Some(&[ + sysinfo::get_current_pid().expect("failed to get current pid") + ]), + false + ), 1, "process not listed", ); @@ -44,7 +47,7 @@ fn test_refresh_process() { #[test] fn test_get_process() { let mut sys = System::new(); - sys.refresh_processes(ProcessesToUpdate::All); + sys.refresh_processes(ProcessesToUpdate::All, false); let current_pid = match sysinfo::get_current_pid() { Ok(pid) => pid, _ => { @@ -76,7 +79,7 @@ fn check_if_send_and_sync() { impl Bar for T where T: Sync {} let mut sys = System::new(); - sys.refresh_processes(ProcessesToUpdate::All); + sys.refresh_processes(ProcessesToUpdate::All, false); let current_pid = match sysinfo::get_current_pid() { Ok(pid) => pid, _ => { @@ -137,7 +140,11 @@ fn test_consecutive_cpu_usage_update() { let mut sys = System::new_all(); assert!(!sys.cpus().is_empty()); - sys.refresh_processes_specifics(ProcessesToUpdate::All, ProcessRefreshKind::new().with_cpu()); + sys.refresh_processes_specifics( + ProcessesToUpdate::All, + true, + ProcessRefreshKind::new().with_cpu(), + ); let stop = Arc::new(AtomicBool::new(false)); // Spawning a few threads to ensure that it will actually have an impact on the CPU usage. @@ -168,6 +175,7 @@ fn test_consecutive_cpu_usage_update() { for pid in &pids { sys.refresh_processes_specifics( ProcessesToUpdate::Some(&[*pid]), + true, ProcessRefreshKind::new().with_cpu(), ); }