jim-easterbrook / python-gphoto2

Python interface to libgphoto2
GNU Lesser General Public License v3.0
362 stars 59 forks source link

Repeated checks for camera causes memory leak #38

Closed flyte closed 6 years ago

flyte commented 6 years ago

I'm attempting to write a program which acts a lot like --capture-tethered but doesn't quit when the camera is removed. I'm doing something analogous to the following, but it's eventually consuming all of my memory:

leaktest.py

import sys

import gphoto2 as gp

def write(msg):
    sys.stdout.write(msg)
    sys.stdout.flush()

def get_camera(context):
    camera = None
    write("Waiting for a camera...")
    while True:
        try:
            camera = gp.check_result(gp.gp_camera_new())
            camera.init(context)
            print("DONE")
            return camera
        except gp.GPhoto2Error:
            write(".")
        finally:
            # Just in case the reassignment somehow doesn't dereference `camera`.
            if camera is not None:
                del camera

def wait_for_image(camera, context, timeout=1000):
    status = None
    write("Waiting for an image...")
    while status != gp.GP_EVENT_FILE_ADDED:
        status, output = camera.wait_for_event(timeout, context)
        write(".")
    print("DONE")
    return output

context = gp.gp_context_new()
while True:
    camera = get_camera(context)
    try:
        while True:
            img_info = wait_for_image(camera, context)
            # Do things with img_info etc.
            print("You took a photo!")
    except gp.GPhoto2Error:
        print("\nCamera disconnected!")
    finally:
        try:
            camera.exit(context)
            print("Cleanly exited camera.")
        except gp.GPhoto2Error:
            # Well, we tried.
            print("Unable to exit camera cleanly.")
        del camera

PS. The real version has some more sensible sleep() calls etc, but this should demonstrate the point.

If you run python leaktest.py and in another terminal, watch -d "ps aux | grep leaktest | grep -v grep" without a camera attached, then you should see the memory usage creep up continuously.

Am I doing this wrong? If I keep the existing camera object around and try to init() it again, it says something along the lines of Bad Parameters.

Cheers

jim-easterbrook commented 6 years ago

Sorry but I don't have time to look into this now, or for the rest of the weekend. Can you wait a day or two? PS The list-cameras.py example shows another way to detect a connected camera.

flyte commented 6 years ago

Yep, sure. I was hoping I was doing something obviously wrong though :)

jim-easterbrook commented 6 years ago

Memory leaks in SWIG generated Python interfaces can be a bit hard to track down, especially with a library that has its own reference counting stuff. Is this happening with Python2 or Python3 or both?

flyte commented 6 years ago

It's happening in Python 2 and 3. Here's a far simpler program to reproduce it (run without any cameras plugged in):

import gphoto2 as gp

context = gp.gp_context_new()
while True:
    camera = gp.check_result(gp.gp_camera_new())
    gp.gp_camera_init(camera, context)

It seems to be caused by gp.gp_camera_init(camera, context) as it doesn't leak without that line.

jim-easterbrook commented 6 years ago

When I run your delightfully short script I get a lot of messages swig/python detected a memory leak of type 'Camera *', no destructor found. if I'm using the PYTHON_GPHOTO2_NO_BUILTIN=1 build. With the normal build I don't get the error messages but still have the leak. I probably won't have time to sort this out today, but I'm confident of tracking it down tomorrow.

jim-easterbrook commented 6 years ago

Ignore that - the messages were because I wasn't correctly installing the PYTHON_GPHOTO2_NO_BUILTIN=1 version. When installed correctly the -builtin option makes no difference to the memory leak and generates no error messages.

jim-easterbrook commented 6 years ago

I've tried adding some gp_camera_unref calls to the Python interface to gp_camera_init but it makes no difference. I suspect the leak may be in libgphoto2 itself. I found this discussion which suggests there was a problem in version 2.4. http://gphoto-software.10949.n7.nabble.com/Memory-Leaks-with-libgphoto2-4-5-td8318.html

flyte commented 6 years ago

Ah, well that's interesting. Sorry if I've wasted your time! I suppose the way forward here is to document a workaround in the examples. I'll see if I can come up with something, but as far as I'm aware, whatever you do with a new camera object, it calls gp_camera_init first. I'll try a few things out this morning.

jim-easterbrook commented 6 years ago

No time wasted - I've learnt a bit more about libgphoto2. It appears the leak is caused if you call camera.init without having first called camera.set_port_info to select the camera. You could try something like this:

context = gp.Context()
camera_list = []
for name, addr in context.camera_autodetect():
    camera_list.append((name, addr))
if camera_list:
    camera = gp.Camera()
    port_info_list = gp.PortInfoList()
    port_info_list.load()
    idx = port_info_list.lookup_path(camera_list[0][1])
    camera.set_port_info(port_info_list[idx])
    camera.init(self.context)

I've not tried this exact code (I adapted it from another project where I allow the user to choose a camera) but it should give you the idea.

flyte commented 6 years ago

So far I've narrowed the leak (if there's only one?) down to gp_port_info_list_load, as can be seen with the following:

import gphoto2 as gp

while True:
    pil = gp.check_result(gp.gp_port_info_list_new())
    gp.gp_port_info_list_load(pil)  # Leaks

and indeed just context.camera_autodetect() itself leaks, presumably because it calls gp_port_info_list_load:

import gphoto2 as gp

context = gp.gp_context_new()
while True:
    context.camera_autodetect()  # Leaks

Going further down the rabbit hole :)

flyte commented 6 years ago

I'm reaching the limit of my ability to track this down now. I'm not sure there's a way we can get around it without the issue being fixed upstream. There's a bug report already at https://github.com/gphoto/libgphoto2/issues/165.

I guess I'll just have to run my code in a bash while loop and exit when a camera is not found/disconnects :(

jim-easterbrook commented 6 years ago

I agree, it's not something I can do anything about in the Python interface. Please reopen this bug if you disagree.

AntiSol commented 5 years ago

Hi Jim, me again. I'm seeing a similar issue to this one.

memleak.py:

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

while running this, if you do something like

while true; do ps -eo pid,args,rss | grep "python [Mm]emleak.py" ; sleep 1; done

You'll see rapidly increasing memory usage. Note that you probably don't want to run that python code for very long unless you like seeing your system start swapping like crazy.

I think this is probably an issue with the library that I'm not going to be able to workaround without debugging C (which is beyond my abilities), I think it might be related to https://github.com/gphoto/libgphoto2/issues/165 and https://github.com/gphoto/libgphoto2/issues/208. Would you agree? or is this a different issue you might be able to help me track down?

the device I'm working with has limited memory, so if I can't find a fix or workaround for this it pretty much kills my project. I'd appreciate any suggestions.

jim-easterbrook commented 5 years ago

It's possible this is a bug in the Python interface, as it's not to do with the camera autodetect stuff this bug is about. The whole config tree system is a bit of a nightmare, with nodes allocated in C that may or may not get properly freed when the Python object is deleted. I'll investigate further.

jim-easterbrook commented 5 years ago

I've established that this is a bug, and have opened #67 for it.

AntiSol commented 5 years ago

thanks, I appreciate it.