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

Prevent memory corruption when finalizing HIDAPI #130

Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 16 additions & 12 deletions hid.pyx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import sys
import atexit
import weakref

from chid cimport *
from libc.stddef cimport wchar_t, size_t

Expand Down Expand Up @@ -65,21 +66,23 @@ def enumerate(int vendor_id=0, int product_id=0):
hid_free_enumeration(info)
return res

def hidapi_exit():
"""Callback for when the script exits.

This prevents memory leaks in the hidapi C library.
Note that the counterpart, hid_init(), is not called explicitly. It will
be called internally when first needed.
"""
hid_exit()

cdef class device:
"""Device class.

A device instance can be used to read from and write to a HID device.
"""

cdef hid_device *_c_hid
cdef object __weakref__ # enable weak-reference support

def __cinit__(self):
"""Initialize the device instance.

Sets up `self.close()` to be automatically called once the strong count
goes to zero. This is necessary to prevent issues later in the
execution, when finalizing the HIDAPI library itself.
"""
weakref.finalize(self, self.close)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jonasmalacofilho I'm wondering how this works because the doc of weakref.finalize(obj, func, /, *args, **kwargs) say:

Note: It is important to ensure that func, args and kwargs do not own any references to obj, either directly or indirectly, since otherwise obj will never be garbage collected. In particular, func should not be a bound method of obj.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my testing both the device was closed and hid_exit() was called, but it might have been just luck.

I'll take another look...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, it also worked in my testing... maybe the behavior is different for cdef classes


def open(self, int vendor_id=0, int product_id=0, unicode serial_number=None):
"""Open the connection.
Expand Down Expand Up @@ -382,5 +385,6 @@ cdef class device:
return U(<wchar_t*>hid_error(self._c_hid))


# Set a callback to close the HID library as the script exits
atexit.register(hidapi_exit)
# Finalize the HIDAPI library *only* once there are no more references to this
# module, and it is being garbage collected.
weakref.finalize(sys.modules[__name__], hid_exit)