search

lthiery / SPI-Py

Hardware SPI as a C Extension for Python
286 stars 150 forks source link

Memory leak resolved and support for 2 open devices at once. #22

Closed naleefer closed 5 years ago

naleefer commented 5 years ago

The original code did not allow for both SPI file descriptors to be open at the same time, even from separate processes. This was due to the settings being defined as global variables for the modules, and the python interpreter not importing a module more than once.

In order to use both chip selects with the original code, it was necessary to do something like: spi.openSPI(device="/dev/spidev0.0")
spi.transfer(...)
spi.closeSPI()

spi.openSPI(device="/dev/spidev0.1") spi.transfer(...) spi.closeSPI()

This originally seemed OK, but led to continually increasing memory use, and eventual process termination after all memory was consumed.

The offending code seems to have been this kind of call in openSPI(): PyDict_SetItem(retDict, PyBytes_FromString("mode"), PyLong_FromLong((long)mode));

Both the PyBytes_FromString("mode") and PyLong_FromLong((long)mode) calls allocate memory on the heap that is never freed. Repeated calls to openSPI then led to continually increasing memory consumption.

The fix involves moving those variables out into separate declarations, for instance PyObject* key_mode = PyBytes_FromString("mode"); followed by Py_XDECREF(key_mode); after creation of the dictionary.

In addition to this fix, I modified the behavior of spi.transfer() to accept the dictionary returned from spi.openSPI(), allowing both SPI file descriptors to be opened and used at the same time.

The README.md file has been expanded to document appropriate usage, and two example scripts added, one to demonstrate a normal use case and one to verify that no memory leak persists.

This modification of the interface will require changes to any code that depends on the repository as-is, but these changes are minor.

lthiery commented 5 years ago

Great work! Would you like to become a maintainer for the project? I haven't had time to really touch this stuff in years and this is a significant PR!

naleefer commented 5 years ago

Hey, thanks! Yeah, you can put me on as a maintainer for the project and I’ll do my best to keep it in good shape :) Thanks for the putting the project together in the first place, it’s really a nice and simple interface to the spi device.

On Feb 20, 2019, at 10:06, Louis Thiery notifications@github.com wrote:

Great work! Would you like to become a maintainer for the project? I haven't had time to really touch this stuff in years and this is a significant PR!

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/lthiery/SPI-Py/pull/22#issuecomment-465690657, or mute the thread https://github.com/notifications/unsubscribe-auth/AHLfQYGDggeXDFc1MhgJGxy1wZGPVQrMks5vPY6QgaJpZM4a_5VW.