Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix tasks update for linux processes #1473

Merged
merged 4 commits into from
Mar 5, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 19 additions & 14 deletions src/unix/linux/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -508,7 +508,8 @@ fn update_existing_process(
uptime: u64,
info: &SystemInfo,
refresh_kind: ProcessRefreshKind,
) -> Result<(Option<Process>, Pid), ()> {
tasks: Option<HashSet<Pid>>,
) -> Result<Option<Process>, ()> {
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) {
Expand All @@ -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);

Expand All @@ -546,7 +549,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(
Expand All @@ -560,9 +563,10 @@ 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)
}

#[allow(clippy::too_many_arguments)]
pub(crate) fn _get_process_data(
path: &Path,
proc_list: &mut HashMap<Pid, Process>,
Expand All @@ -571,18 +575,20 @@ pub(crate) fn _get_process_data(
uptime: u64,
info: &SystemInfo,
refresh_kind: ProcessRefreshKind,
) -> Result<(Option<Process>, Pid), ()> {
tasks: Option<HashSet<Pid>>,
) -> Result<Option<Process>, ()> {
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)?;
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;
new_process.inner.tasks = tasks;
Ok(Some(new_process))
}

fn old_get_memory(entry: &mut ProcessInner, str_parts: &[&str], info: &SystemInfo) {
Expand Down Expand Up @@ -796,21 +802,20 @@ 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 new_process = _get_process_data(
e.path.as_path(),
proc_list.get(),
proc_list,
e.pid,
e.parent_pid,
uptime,
info,
refresh_kind,
e.tasks,
)
.ok()?;
nb_updated.fetch_add(1, Ordering::Relaxed);
if let Some(ref mut p) = p {
p.inner.tasks = e.tasks;
}
p
new_process
})
.collect::<Vec<_>>()
};
Expand Down
49 changes: 43 additions & 6 deletions tests/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.");

Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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>;
Expand Down Expand Up @@ -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;
}
Expand All @@ -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.);
}

Expand Down Expand Up @@ -1006,3 +1004,42 @@ 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<Pid> {
let pid_to_process = system.processes();
let mut task_pids: HashSet<Pid> = HashSet::new();
for process in pid_to_process.values() {
if let Some(tasks) = process.tasks() {
task_pids.extend(tasks);
}
}
task_pids
}

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");
}
Loading