GuillaumeGomez / sysinfo

Cross-platform library to fetch system information
MIT License
2.01k stars 301 forks source link

Refreshing multiple processes on macos leads to invalid cpu usage #1299

Closed vthib closed 1 month ago

vthib commented 2 months ago

I have a piece of code that looks like this:

let mut sys = sysinfo::System::new();
let process_refresh_kind = ProcessRefreshKind::everything().without_disk_usage().without_environ();
sys.refresh_process_specifics(pid_a, process_refresh_kind);
sys.refresh_process_specifics(pid_b, process_refresh_kind);

loop {
    // Wait 1 minute
    std::thread::sleep(std::time::Duration::from_secs(60));

    // Refresh CPU measurement
    sys.refresh_process_specifics(pid_a, process_refresh_kind);
    sys.refresh_process_specifics(pid_b, process_refresh_kind);

    // Get CPU usage of the two processes
    let cpu_a = sys.process(pid_a).map(|p| p.cpu_usage());
    let cpu_b = sys.process(pid_b).map(|p| p.cpu_usage());
    println!("cpu a: {:?}, cpu b: {:?}", cpu_a, cpu_b);
}

With this code, the cpu for process A seems to be valid, but the one for process B is way too high, it can even get higher than the max value.

Looking at the code, I see that the function compute_cpu_usage uses a time_interval variable. This value seems to be right on the first call to refresh (for pid A), but not on the second call (for pid B). I suppose this value is reset after each call to "refresh_process", leading to the second call computing a CPU load with a time interval that is way too small.

Here is a reproducer:

fn main() {
    let mut sys = sysinfo::System::new();

    let pid_a = sysinfo::Pid::from_u32(std::process::id());
    let pid_b = sysinfo::Pid::from_u32(1);

    let process_refresh_kind = sysinfo::ProcessRefreshKind::everything()
        .without_disk_usage()
        .without_environ();
    sys.refresh_process_specifics(pid_a, process_refresh_kind);
    sys.refresh_process_specifics(pid_b, process_refresh_kind);

    loop {
        std::thread::sleep(std::time::Duration::from_secs(10));

        sys.refresh_process_specifics(pid_a, process_refresh_kind);
        sys.refresh_process_specifics(pid_b, process_refresh_kind);

        let cpu_a = sys.process(pid_a).map(|p| p.cpu_usage());
        let cpu_b = sys.process(pid_b).map(|p| p.cpu_usage());
        println!("cpu a: {:?}, cpu b: {:?}", cpu_a, cpu_b);
    }
}

Reversing the two PIDs reverses the one that is reported with a very high CPU usage, showing the bug.

This was reproduced on 0.30.12 on macos x64.

GuillaumeGomez commented 2 months ago

For this kind of operations, you likely want to use refresh_pids_specifics. Although, in this case it should still work.

vthib commented 2 months ago

Indeed, I didn't know about this API, and it fixes the issue. I'm leaving this issue open since there is still a bug afaict, but i'm no longer blocked by it, I let you decide what to do with it. Thanks!

GuillaumeGomez commented 2 months ago

It definitely needs to be fixed. Gonna try to take a look in the next days.