From c0d5827c3ec7c1ea08fe1b1deef806fa1c54cccd Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 4 Mar 2025 23:54:03 +0100 Subject: [PATCH 1/4] Fix tasks update for linux processes --- src/unix/linux/process.rs | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/src/unix/linux/process.rs b/src/unix/linux/process.rs index 73f2bd97d..ced241a2e 100644 --- a/src/unix/linux/process.rs +++ b/src/unix/linux/process.rs @@ -508,7 +508,7 @@ fn update_existing_process( uptime: u64, info: &SystemInfo, refresh_kind: ProcessRefreshKind, -) -> Result<(Option, Pid), ()> { +) -> Result, ()> { let entry = &mut proc.inner; let data = if let Some(mut f) = entry.stat_file.take() { match get_all_data_from_file(&mut f, 1024) { @@ -546,7 +546,7 @@ fn update_existing_process( ); refresh_user_group_ids(entry, &mut proc_path, refresh_kind); - return Ok((None, entry.pid)); + return Ok(None); } // If we're here, it means that the PID still exists but it's a different process. let p = retrieve_all_new_process_info( @@ -560,7 +560,7 @@ fn update_existing_process( ); *proc = p; // Since this PID is already in the HashMap, no need to add it again. - Ok((None, proc.inner.pid)) + Ok(None) } pub(crate) fn _get_process_data( @@ -571,7 +571,7 @@ pub(crate) fn _get_process_data( uptime: u64, info: &SystemInfo, refresh_kind: ProcessRefreshKind, -) -> Result<(Option, Pid), ()> { +) -> Result, ()> { if let Some(ref mut entry) = proc_list.get_mut(&pid) { return update_existing_process(entry, parent_pid, uptime, info, refresh_kind); } @@ -579,10 +579,10 @@ pub(crate) fn _get_process_data( let data = _get_stat_data(path, &mut stat_file)?; let parts = parse_stat_file(&data).ok_or(())?; - let mut p = + let mut new_process = retrieve_all_new_process_info(pid, parent_pid, &parts, path, info, refresh_kind, uptime); - p.inner.stat_file = stat_file; - Ok((Some(p), pid)) + new_process.inner.stat_file = stat_file; + Ok(Some(new_process)) } fn old_get_memory(entry: &mut ProcessInner, str_parts: &[&str], info: &SystemInfo) { @@ -796,9 +796,10 @@ pub(crate) fn refresh_procs( .flatten() .filter(|e| filter_callback(e, filter)) .filter_map(|e| { - let (mut p, _) = _get_process_data( + let proc_list = proc_list.get(); + let mut new_process = _get_process_data( e.path.as_path(), - proc_list.get(), + proc_list, e.pid, e.parent_pid, uptime, @@ -807,10 +808,12 @@ pub(crate) fn refresh_procs( ) .ok()?; nb_updated.fetch_add(1, Ordering::Relaxed); - if let Some(ref mut p) = p { + if let Some(ref mut p) = new_process { + p.inner.tasks = e.tasks; + } else if let Some(p) = proc_list.get_mut(&e.pid) { p.inner.tasks = e.tasks; } - p + new_process }) .collect::>() }; From d405b62617691326d98470a4b96d4cd548faad27 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 5 Mar 2025 00:06:24 +0100 Subject: [PATCH 2/4] Add regression test for tasks update --- tests/process.rs | 59 +++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 53 insertions(+), 6 deletions(-) diff --git a/tests/process.rs b/tests/process.rs index 8af922bd1..c18e49882 100644 --- a/tests/process.rs +++ b/tests/process.rs @@ -116,7 +116,7 @@ fn test_environ() { s.refresh_processes_specifics( ProcessesToUpdate::Some(&[pid]), false, - sysinfo::ProcessRefreshKind::everything(), + ProcessRefreshKind::everything(), ); p.kill().expect("Unable to kill process."); @@ -374,7 +374,7 @@ fn test_refresh_process_doesnt_remove() { // Checks that the process is listed as it should. let mut s = System::new_with_specifics( - RefreshKind::nothing().with_processes(sysinfo::ProcessRefreshKind::nothing()), + RefreshKind::nothing().with_processes(ProcessRefreshKind::nothing()), ); s.refresh_processes(ProcessesToUpdate::All, false); @@ -589,7 +589,7 @@ fn test_process_iterator_lifetimes() { } let s = System::new_with_specifics( - sysinfo::RefreshKind::nothing().with_processes(sysinfo::ProcessRefreshKind::nothing()), + RefreshKind::nothing().with_processes(ProcessRefreshKind::nothing()), ); let process: Option<&sysinfo::Process>; @@ -671,8 +671,6 @@ fn test_process_creds() { // This test ensures that only the requested information is retrieved. #[test] fn test_process_specific_refresh() { - use sysinfo::{DiskUsage, ProcessRefreshKind}; - if !sysinfo::IS_SUPPORTED_SYSTEM || cfg!(feature = "apple-sandbox") { return; } @@ -693,7 +691,7 @@ fn test_process_specific_refresh() { assert_eq!(p.memory(), 0); assert_eq!(p.virtual_memory(), 0); // These two won't be checked, too much lazyness in testing them... - assert_eq!(p.disk_usage(), DiskUsage::default()); + assert_eq!(p.disk_usage(), sysinfo::DiskUsage::default()); assert_eq!(p.cpu_usage(), 0.); } @@ -1006,3 +1004,52 @@ fn test_exists() { s.refresh_processes_specifics(ProcessesToUpdate::Some(&[pid]), false, process_refresh_kind); assert!(!s.process(pid).unwrap().exists()); } + +#[cfg(target_os = "linux")] +#[test] +fn test_tasks() { + use std::collections::HashSet; + + fn get_tasks(system: &System) -> HashSet { + let pid_to_process = system.processes(); + let mut task_pids: HashSet = HashSet::new(); + for process in pid_to_process.values() { + if let Some(tasks) = process.tasks() { + task_pids.extend(tasks); + } + } + task_pids + } + + let scheduler = std::thread::spawn(|| { + let mut system = System::new_with_specifics(RefreshKind::nothing()); + let mut nb_failures = 0; + + for _ in 0..5 { + system.refresh_processes_specifics( + ProcessesToUpdate::All, + true, + ProcessRefreshKind::nothing(), + ); + + let mut system_new = System::new_with_specifics(RefreshKind::nothing()); + system_new.refresh_processes_specifics( + ProcessesToUpdate::All, + true, + ProcessRefreshKind::nothing(), + ); + + let nb_tasks = get_tasks(&system).len(); + let new_nb_tasks = get_tasks(&system_new).len(); + if nb_tasks != new_nb_tasks { + nb_failures += 1; + } + println!("{nb_tasks} == {new_nb_tasks}"); + std::thread::sleep(std::time::Duration::from_millis(500)); + } + if nb_failures > 2 { + panic!("number of tasks doesn't match"); + } + }); + scheduler.join().expect("Scheduler panicked"); +} From a194b13c7c83bdc22069cdc22d38deddad0abb40 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 5 Mar 2025 11:36:11 +0100 Subject: [PATCH 3/4] Pass `tasks` as arguments instead of re-getting the HashMap process --- src/unix/linux/process.rs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/unix/linux/process.rs b/src/unix/linux/process.rs index ced241a2e..a824c60ad 100644 --- a/src/unix/linux/process.rs +++ b/src/unix/linux/process.rs @@ -508,6 +508,7 @@ fn update_existing_process( uptime: u64, info: &SystemInfo, refresh_kind: ProcessRefreshKind, + tasks: Option>, ) -> Result, ()> { let entry = &mut proc.inner; let data = if let Some(mut f) = entry.stat_file.take() { @@ -526,6 +527,8 @@ fn update_existing_process( } else { _get_stat_data(&entry.proc_path, &mut entry.stat_file)? }; + entry.tasks = tasks; + let parts = parse_stat_file(&data).ok_or(())?; let start_time_without_boot_time = compute_start_time_without_boot_time(&parts, info); @@ -563,6 +566,7 @@ fn update_existing_process( Ok(None) } +#[allow(clippy::too_many_arguments)] pub(crate) fn _get_process_data( path: &Path, proc_list: &mut HashMap, @@ -571,9 +575,10 @@ pub(crate) fn _get_process_data( uptime: u64, info: &SystemInfo, refresh_kind: ProcessRefreshKind, + tasks: Option>, ) -> Result, ()> { if let Some(ref mut entry) = proc_list.get_mut(&pid) { - return update_existing_process(entry, parent_pid, uptime, info, refresh_kind); + return update_existing_process(entry, parent_pid, uptime, info, refresh_kind, tasks); } let mut stat_file = None; let data = _get_stat_data(path, &mut stat_file)?; @@ -582,6 +587,7 @@ pub(crate) fn _get_process_data( let mut new_process = retrieve_all_new_process_info(pid, parent_pid, &parts, path, info, refresh_kind, uptime); new_process.inner.stat_file = stat_file; + new_process.inner.tasks = tasks; Ok(Some(new_process)) } @@ -797,7 +803,7 @@ pub(crate) fn refresh_procs( .filter(|e| filter_callback(e, filter)) .filter_map(|e| { let proc_list = proc_list.get(); - let mut new_process = _get_process_data( + let new_process = _get_process_data( e.path.as_path(), proc_list, e.pid, @@ -805,14 +811,10 @@ pub(crate) fn refresh_procs( uptime, info, refresh_kind, + e.tasks, ) .ok()?; nb_updated.fetch_add(1, Ordering::Relaxed); - if let Some(ref mut p) = new_process { - p.inner.tasks = e.tasks; - } else if let Some(p) = proc_list.get_mut(&e.pid) { - p.inner.tasks = e.tasks; - } new_process }) .collect::>() From a69bf23e943078ad76b893367145d73e5d4ec8e9 Mon Sep 17 00:00:00 2001 From: Steven Xu Date: Wed, 5 Mar 2025 22:28:12 +1100 Subject: [PATCH 4/4] test: make tasks test reproducible --- tests/process.rs | 48 +++++++++++++++++++----------------------------- 1 file changed, 19 insertions(+), 29 deletions(-) diff --git a/tests/process.rs b/tests/process.rs index c18e49882..6ca5d87da 100644 --- a/tests/process.rs +++ b/tests/process.rs @@ -1021,35 +1021,25 @@ fn test_tasks() { task_pids } - let scheduler = std::thread::spawn(|| { - let mut system = System::new_with_specifics(RefreshKind::nothing()); - let mut nb_failures = 0; - - for _ in 0..5 { - system.refresh_processes_specifics( - ProcessesToUpdate::All, - true, - ProcessRefreshKind::nothing(), - ); - - let mut system_new = System::new_with_specifics(RefreshKind::nothing()); - system_new.refresh_processes_specifics( - ProcessesToUpdate::All, - true, - ProcessRefreshKind::nothing(), - ); - - let nb_tasks = get_tasks(&system).len(); - let new_nb_tasks = get_tasks(&system_new).len(); - if nb_tasks != new_nb_tasks { - nb_failures += 1; - } - println!("{nb_tasks} == {new_nb_tasks}"); - std::thread::sleep(std::time::Duration::from_millis(500)); - } - if nb_failures > 2 { - panic!("number of tasks doesn't match"); - } + let mut system = System::new_with_specifics(RefreshKind::nothing()); + system.refresh_processes_specifics(ProcessesToUpdate::All, true, ProcessRefreshKind::nothing()); + + // Spawn a thread to increase the task count + let scheduler = std::thread::spawn(move || { + system.refresh_processes_specifics( + ProcessesToUpdate::All, + true, + ProcessRefreshKind::nothing(), + ); + + let mut system_new = System::new_with_specifics(RefreshKind::nothing()); + system_new.refresh_processes_specifics( + ProcessesToUpdate::All, + true, + ProcessRefreshKind::nothing(), + ); + + assert_eq!(get_tasks(&system), get_tasks(&system_new)); }); scheduler.join().expect("Scheduler panicked"); }