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 memleaks related to un-freed mmal objs #6

Merged
merged 2 commits into from
Oct 2, 2021

Conversation

Stephen-Seo
Copy link
Contributor

@Stephen-Seo Stephen-Seo commented Sep 27, 2021

I've been using rascam in a long-lived process that periodically creates
and drops a SimpleCamera (that has an encapsulated SeriousCamera)
periodically or on demand. The process mainly hosts an actix-web
HttpServer and periodically saves an image from the raspberry pi camera,
or serves a live feed from it through the front-end.

Eventually, creating SimpleCamera fails with the following error
message:

mmal: mmal_vc_component_create: failed to create component 'vc.null_sink' (1:ENOMEM)
mmal: mmal_component_create_core: could not create component 'vc.null_sink' (1)
Failed to activate SimpleCamera: MMAL error: Unable to create null sink for preview Status: ENOMEM

It seems that rascam has a memory leak in GPU memory. Upon examination,
I've discovered some pointers to mmal objects created with mmal_*_create
that were never freed with mmal_*_destroy.

This commit addresses this issue by cleaning up mmal objects which
should prevent (or at least reduce) GPU memory leaks.

Note that I've been checking the GPU memory of a Raspberry Pi 3B with
the following commands:

vcgencmd get_mem malloc
vcgencmd get_mem reloc

After applying this commit, the periodic usage of SimpleCamera appears
to not leak the GPU memory.

Signed-off-by: Stephen Seo seo.disparate@gmail.com

I've been using rascam in a long-lived process that periodically creates
and drops a SimpleCamera (that has an encapsulated SeriousCamera)
periodically or on demand. The process mainly hosts an actix-web
HttpServer and periodically saves an image from the raspberry pi camera,
or serves a live feed from it through the front-end.

Eventually, creating SimpleCamera fails with the following error
message:

    mmal: mmal_vc_component_create: failed to create component 'vc.null_sink' (1:ENOMEM)
    mmal: mmal_component_create_core: could not create component 'vc.null_sink' (1)
    Failed to activate SimpleCamera: MMAL error: Unable to create null sink for preview Status: ENOMEM

It seems that rascam has a memory leak in GPU memory. Upon examination,
I've discovered some pointers to mmal objects created with mmal_*_create
that were never freed with mmal_*_destroy.

This commit addresses this issue by cleaning up mmal objects which
should prevent (or at least reduce) GPU memory leaks.

Note that I've been checking the GPU memory of a Raspberry Pi 3B with
the following commands:

    vcgencmd get_mem malloc
    vcgencmd get_mem reloc

After applying this commit, the periodic usage of SimpleCamera appears
to not leak the GPU memory.

Signed-off-by: Stephen Seo <seo.disparate@gmail.com>
@Stephen-Seo
Copy link
Contributor Author

Stephen-Seo commented Sep 28, 2021

I've found that this test program will panic within 100 iterations without the PR applied, but will successfully run 100 iterations when it is applied:

use rascam::SimpleCamera;
use std::thread;
use std::time::Duration;

fn main() {
    let info = rascam::info().unwrap();
    let sleep_duration = Duration::from_millis(100);

    for i in 0..100 {
        println!("Iteration {}", i + 1);
        let mut camera = SimpleCamera::new(info.cameras[0].clone())
            .expect("Should be able to create SimpleCamera");
        camera.activate().expect("Should be able to activate SimpleCamera");

        thread::sleep(sleep_duration);
    }
}

This is with 128M allocated to the Raspberry Pi 3B GPU.

@pedrosland
Copy link
Owner

Hi @Stephen-Seo, thank you very much for your PR.
My Pi is being used for something else so I won't test this myself. @UnicornsOnLSD suggested part of this over in #5 so I feel good about this change.

I've re-linked TravisCI. If you don't mind, could you amend and force push so we can get the green light? Thanks 🙂

`git commit --amend` was invoked on this commit to be force pushed so
that TravisCI can do its thing.
@Stephen-Seo
Copy link
Contributor Author

I've done the git commit --amend and force push, but I don't see anything about TravisCI on GitHub. Are you sure it's working ok, or did I miss it?

@pedrosland
Copy link
Owner

Thanks Stephen.
I triggered a build from Travis and it built as well as can be expected. The integration is apparently set up and settings for PRs are enabled but I agree, nothing is building 😿
Ok. Let's yolo it for now and I will try and fix it later and a release for crates.io

@pedrosland pedrosland merged commit 8366372 into pedrosland:master Oct 2, 2021
@Stephen-Seo Stephen-Seo deleted the fix_gpu_memory_leak branch October 5, 2021 06:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants