GuillaumeGomez / sysinfo

Cross-platform library to fetch system information
MIT License
1.87k stars 283 forks source link

Possible memory leak in `Disks::refresh_list` #1282

Open keabarnes opened 3 weeks ago

keabarnes commented 3 weeks ago

Describe the bug

System: macOS Sonoma 14.1.2 (23B92) sysinfo version: 0.30.12

When calling Disks::refresh_list() repeatedly in a loop, the memory the application uses grows without bound. Variations shown in the minimum reproducible example below:

To Reproduce

use std::thread::sleep;
use sysinfo::{Disks, MINIMUM_CPU_UPDATE_INTERVAL};

fn main() {
    // Memory leak when using `new_with_refreshed_list`:
    // loop {
    //     let _ = Disks::new_with_refreshed_list();
    //     sleep(MINIMUM_CPU_UPDATE_INTERVAL);
    // }

    // Memory leak when using `refresh_list`:
    // let mut disks = Disks::new_with_refreshed_list();
    // loop {
    //     disks.refresh_list();
    //     disks.refresh();
    //     sleep(MINIMUM_CPU_UPDATE_INTERVAL);
    // }

    // No memorty leak but `available_space` isn't updated when the space on my computer is
    // actively changing:
    let mut disks = Disks::new_with_refreshed_list();
    loop {
        disks.refresh();

        let first_available_space = disks.first().map(|disk| disk.available_space()).unwrap();
        println!("Available space: {first_available_space}");

        sleep(MINIMUM_CPU_UPDATE_INTERVAL);
    }
}
keabarnes commented 3 weeks ago

Using cargo instruments I ran an Allocations template for my program and was able to capture the following memory allocation trace:

image

That queryCache just grows over time

GuillaumeGomez commented 3 weeks ago

It means that CFURLCopyResourcePropertiesForKeys is allocating memory that is never freed. Sounds pretty bad.

Is it on mac M or an older one? If it's on mac M, I can't test it as I don't have one.

keabarnes commented 3 weeks ago

I'm experiencing it on an M*, but I think others have experienced it on Intel. Let me try find an Intel to run the example above on and see if it persists.

In the meantime, I downgraded to 0.29 and the issue persists on that version too.

keabarnes commented 3 weeks ago

I've been able to confirm that this behaviour does still occur on an Intel based Mac

GuillaumeGomez commented 3 weeks ago

Then I'll be able to check what's going on locally. Thanks for the information!

GuillaumeGomez commented 3 weeks ago

I just checked and... I have absolutely no idea where the leak comes from... To be honest, I'm pretty bad at using mac tools to try to check memory issues since they don't have valgrind and their other tools are confusing. So for now, I'll consider it's "ok". Don't hesitate to give it a try though, help to fix this issue would be very welcome!

keabarnes commented 3 weeks ago

Ok, if you leave the issue open, I will try circle back to it and see what I can find. Do you know of some way that I can work around the issue for now? Is there some way that I can instantiate the library in a way that forces it to clear the memory?

GuillaumeGomez commented 3 weeks ago

It seems to be specific to the running process (or maybe the current thread? To be confirmed). So unless you get this information from another process/thread, I don't see how.

keabarnes commented 3 weeks ago

Good suggestion, spawning a new thread does the trick. The underlying code still needs to be fixed, but the code below prevents the memory from growing past one invocation's worth:

use std::thread::sleep;
use sysinfo::{Disks, MINIMUM_CPU_UPDATE_INTERVAL};

fn main() {
    // Memory leak when using `refresh_list`:
    loop {
        let handle = std::thread::spawn(move || {
            let mut disks = Disks::new_with_refreshed_list();
            disks.refresh_list();
            disks.refresh();
            disks.first().map(|disk| {
                let available_space = disk.available_space();
                println!("{available_space}");
            });
            sleep(MINIMUM_CPU_UPDATE_INTERVAL);
        });
        let _ = handle.join();
    }
}
GuillaumeGomez commented 3 weeks ago

It'd be much simpler if we could find out where the leaked memory comes from. If anyone knows a good memory debugger for mac...

keabarnes commented 3 weeks ago

In this comment, I used cargo instruments (cargo install cargo-instruments) which uses the standard macOS tools, running the minimum example with cargo instruments -t Allocations --time-limit 10000.

In the Instruments application, on the left side, you can change from Statistics to Call Trees, which is much more helpful.

image

to

image

Then, you can traverse the tree and see where the memory is being allocated from. In the comment above, you can see in my case the lowest level user-space call was sysinfo::unix::apple::disk::get_disk_properties.

image

I imagine the solution would involve calling release on one of the underlying framework entities, maybe the result stored from CFURLCopyResourcePropertiesForKeys or something like that.

GuillaumeGomez commented 3 weeks ago

Thanks for the detailed explanations! I'll give it another try when I have time.