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

Memory leak in gp_camera_get_config #67

Closed
jim-easterbrook opened this issue Nov 1, 2018 · 8 comments
Closed

Memory leak in gp_camera_get_config #67

jim-easterbrook opened this issue Nov 1, 2018 · 8 comments

Comments

@jim-easterbrook
Copy link
Owner

This simple script consumes ever larger amounts of memory.

import time
import gphoto2 as gp
c = gp.Camera()
while True:
    cfg = c.get_config()
    time.sleep(0.01)

I've established that removing the gp_widget_ref call in the Python interface gets rid of the leak, but leaves the Python interface vulnerable to the C object getting deleted prematurely.

@jim-easterbrook
Copy link
Owner Author

I think I've worked out what's happening. Whenever it receives a CameraWidget object (e.g. from gp_widget_get_child) the Python interface calls gp_widget_ref on its root object, to ensure the widget tree doesn't get deleted while the Python object exists. When the Python object is deleted the interface calls gp_widget_unref on the root object.
The problem is with gp_camera_get_config which returns the root object. This newly allocated object already has a ref count of 1, so calling gp_widget_ref increases it to 2. When the Python object is deleted it returns the count to 1 and the C object is not freed.

jim-easterbrook added a commit that referenced this issue Nov 1, 2018
Doing so causes a memory leak, see bug #67.
@jim-easterbrook
Copy link
Owner Author

OK, I think commit b8966a5 should cure this. @AntiSol - do you need a new release immediately? (I've got no other changes planned, so there's no great reason not to, but I don't like to do new releases too often.)

@AntiSol
Copy link

AntiSol commented Nov 1, 2018

If you'd prefer I'm happy to give it a try without a release, that'll let me confirm that it's fixed, as long as I'm not going to have too much trouble with the "Install from GitHub" instructions from the readme - will that overwrite the version installed by pip? or should I remove the pip version first?

@jim-easterbrook
Copy link
Owner Author

Ah, I've just remembered that's a bit of a hassle. You'd need to install SWIG. I'll do a release.

@AntiSol
Copy link

AntiSol commented Nov 1, 2018

ok. thanks :)

@jim-easterbrook
Copy link
Owner Author

Now released as v1.8.5.

@AntiSol
Copy link

AntiSol commented Nov 1, 2018

and confirmed fixed. My hero! Thanks so much!

@jim-easterbrook
Copy link
Owner Author

Thank you for finding and reporting the bug.

jim-easterbrook added a commit that referenced this issue Nov 14, 2018
This is similar to the leak in gp_camera_get_config (bug #67) so I've
implemented a more general solution that should be more robust.
jim-easterbrook added a commit that referenced this issue Nov 15, 2018
Further problems raised by bug #67. Now using more explicit typemap
signatures to select which functions increment the ref count on the root
widget of their result widget.
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

No branches or pull requests

2 participants