search

adafruit / Adafruit_DotStar_Pi

DotStar module for Python on Raspberry Pi
GNU General Public License v3.0
60 stars 30 forks source link

commenting out superfluous INCREF that causes memory leak #18

Closed casoich closed 6 years ago

casoich commented 6 years ago

The problem is the Py_INCREF(), it increases the reference count of result from 1 to 2. Then you return it to Python. The problem is that when result (the or whatever Python variable is named) is garbage collected, this decrements the refcount by 1 so refcount is not == 0 so the variable is not removed. Every time you call this routine you get 4 (or 8) bytes of memory allocated and not released.

This pull request fixes the issue documented here: https://github.com/adafruit/Adafruit_DotStar_Pi/issues/15

PaintYourDragon commented 6 years ago

Thanks for spotting this!

Python modules are not my native tongue. Reading up on this a little, it looks like maybe a Py_INCREF is needed only when returning a Python object (or Py_None), but is NOT needed if returning a regular C type (e.g. integer, short, etc.), sound right? If that's the case, several spots in the code where I'll need to make the same fix (maybe just hasn't come up because they're not called as often).

casoich commented 6 years ago

Yea, I only fixed it on the method I was using, but I spotted it in other places. Also, you're only losing 4 bytes at a time, so it takes a while to eat up a gig of memory (I was calling it 280 times, 60 times a second though...) It took me probably two weeks to track down. In the meantime it led me to develop a routine to write a bitmap font to my DotStar pixel matrix, so that was something! You can check out the project here if you like.

Thanks for accepting the pull request!