Fix memory leakage in python bindings #4290

Merged
merged 1 commit into from Mar 14, 2014

Projects

None yet

5 participants

@dgburr

The python bindings generated by the groovy code generator contain a reference counting bug which results in a memory leak whenever a std::vector is converted to a python list. The following member functions are affected:

  • xbmcvfs.listdir
  • xbmc.Player.getAvailableSubtitleStreams
  • xbmc.Player.getAvailableAudioStreams
  • xbmcgui.Control.getPosition
  • xbmcgui.Dialog.browse
  • xbmcgui.Dialog.browseMultiple

The reference counting bug occurs because PyList_Append() increments the reference count of the item, so the caller needs to call Py_DECREF() because it no longer owns the object.

This bug is especially problematic when running the Watchdog Service Addon since it may result in frequent calls to xbmcvfs.listdir, and each call can leak many kilobytes of data when listing directories containing many files.

@dgburr dgburr Fix memory leakage in python bindings
The python bindings generated by the groovy code generator contain a reference counting bug which results in a memory leak whenever a std::vector is converted to a python list.  The following member functions are affected:

* xbmcvfs.listdir
* xbmc.Player.getAvailableSubtitleStreams
* xbmc.Player.getAvailableAudioStreams
* xbmcgui.Control.getPosition
* xbmcgui.Dialog.browse
* xbmcgui.Dialog.browseMultiple

The reference counting bug occurs because PyList_Append() increments the reference count of the item, so the caller needs to call Py_DECREF() because it no longer owns the object.

This bug is especially problematic when running the Watchdog Service Addon since it may result in frequent calls to xbmcvfs.listdir, and each call can leak many kilobytes of data when listing directories containing many files.
f164b7c
@jmarshallnz
Team Kodi member

Looks correct to me - nice find.

@jimfcarroll any comments?

jenkins build this please

@dgburr dgburr referenced this pull request in tamland/xbmc-addon-watchdog Feb 28, 2014
Merged

Add a setting for network polling interval #4

@MartijnKaijser MartijnKaijser added this to the Pending for inclusion milestone Mar 2, 2014
@MartijnKaijser
Team Kodi member

@jimfcarroll
nugde

@TheCheif

Tried the newest nightly, xbmc still want's to grab more and more ram. I use the boblight addon, I think it's the same problem there? When I start a video xbmc takes like 11% RAM and after a while, it takes like 50% and then it invokes the oomkiller and xbmc gets killed. Im using xbmc on an raspberry pi. This only happens, when the boblight addon ist activated, so it must have something to do with it.

@jmarshallnz jmarshallnz merged commit 33407f8 into xbmc:master Mar 14, 2014

1 check passed

Details default Merged build #293 succeeded in 1 hr 3 min
@jmarshallnz jmarshallnz added the Gotham label Mar 14, 2014
@jmarshallnz jmarshallnz removed the Gotham label Mar 14, 2014
@jimfcarroll
Team Kodi member

Wow. I'm not sure how I missed this. Sorry. Even though it's merged I'll review it.

@TheCheif

But it only happens on my raspberry pi, on my android devices, everything is okay. So don't know if it is worth the effort looking into this :)

@jimfcarroll
Team Kodi member

Boblight is one of the few users of the RenderCapture python interface. I think the bug you uncovered may be more widespread than the list typemap. I'll dig more.

Thanks for this find. The more I look at it the more I think you got it. I need to double check default ownership of some newly created python objects to be sure.

@jimfcarroll
Team Kodi member

I looked over the rest of the conversions and they seem okay. The PyTuple equivalent methods "steal" a reference so there's no need to decrement the ref count.

I also looked at the RenderCapture functionality and cannot see where it might be leaking. Take a look and let me know what you think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment