From b2c4094cb641825b0d43400537cb3b66792a2f15 Mon Sep 17 00:00:00 2001 From: Olivier Martin Date: Wed, 13 Mar 2024 13:49:13 +0100 Subject: [PATCH] gattlib-py: Fix some memory leaks --- common/gattlib_common.c | 7 +++++++ common/gattlib_eddystone.c | 11 ++++++++-- gattlib-py/gattlib/__init__.py | 4 ++++ gattlib-py/gattlib/adapter.py | 3 +++ gattlib-py/gattlib/device.py | 37 ++++++++++++++++++++++++++++------ 5 files changed, 54 insertions(+), 8 deletions(-) diff --git a/common/gattlib_common.c b/common/gattlib_common.c index eed1a775..824e5d73 100644 --- a/common/gattlib_common.c +++ b/common/gattlib_common.c @@ -191,3 +191,10 @@ void gattlib_handler_dispatch_to_thread(struct gattlib_handler* handler, void (* return; } } + +// Helper function to free memory from Python frontend +void gattlib_free_mem(void *ptr) { + if (ptr != NULL) { + free(ptr); + } +} diff --git a/common/gattlib_eddystone.c b/common/gattlib_eddystone.c index 4e13e219..294f6e52 100644 --- a/common/gattlib_eddystone.c +++ b/common/gattlib_eddystone.c @@ -28,10 +28,10 @@ struct on_eddystone_discovered_device_arg { static void on_eddystone_discovered_device(void *adapter, const char* addr, const char* name, void *user_data) { struct on_eddystone_discovered_device_arg *callback_data = user_data; - gattlib_advertisement_data_t *advertisement_data; + gattlib_advertisement_data_t *advertisement_data = NULL; size_t advertisement_data_count; uint16_t manufacturer_id; - uint8_t *manufacturer_data; + uint8_t *manufacturer_data = NULL; size_t manufacturer_data_size; int ret; @@ -46,6 +46,13 @@ static void on_eddystone_discovered_device(void *adapter, const char* addr, cons advertisement_data, advertisement_data_count, manufacturer_id, manufacturer_data, manufacturer_data_size, callback_data->user_data); + + if (advertisement_data != NULL) { + free(advertisement_data); + } + if (manufacturer_data != NULL) { + free(manufacturer_data); + } } int gattlib_adapter_scan_eddystone(void *adapter, int16_t rssi_threshold, uint32_t eddystone_types, diff --git a/gattlib-py/gattlib/__init__.py b/gattlib-py/gattlib/__init__.py index c7eb2e4e..783735c4 100644 --- a/gattlib-py/gattlib/__init__.py +++ b/gattlib-py/gattlib/__init__.py @@ -217,3 +217,7 @@ class GattlibAdvertisementData(Structure): # int gattlib_mainloop_python(PyObject *handler, PyObject *user_data) gattlib_mainloop = gattlib.gattlib_mainloop_python gattlib_mainloop.argtypes = [py_object, py_object] + +# void gattlib_free_mem(void *ptr]) +gattlib_free_mem = gattlib.gattlib_free_mem +gattlib_free_mem.argtypes = [c_void_p] diff --git a/gattlib-py/gattlib/adapter.py b/gattlib-py/gattlib/adapter.py index 90871817..ebbd0367 100644 --- a/gattlib-py/gattlib/adapter.py +++ b/gattlib-py/gattlib/adapter.py @@ -268,4 +268,7 @@ def gattlib_get_advertisement_data_from_mac(self, mac_address): for i in range(_manufacturer_data_len.value): manufacturer_data[i] = c_bytearray.contents[i] & 0xFF + gattlib_free_mem(_advertisement_data) + gattlib_free_mem(_manufacturer_data) + return advertisement_data, _manufacturer_id.value, manufacturer_data diff --git a/gattlib-py/gattlib/device.py b/gattlib-py/gattlib/device.py index df1b99a2..a1a98d9e 100644 --- a/gattlib-py/gattlib/device.py +++ b/gattlib-py/gattlib/device.py @@ -40,7 +40,10 @@ def __init__(self, adapter: Adapter, addr: str, name: str = None): self._addr = addr self._name = name self._connection = None + # We use a lock because on disconnection, we will set self._connection to None self._connection_lock = threading.Lock() + # We use a lock on disconnection to ensure the memory is safely freed + self._disconnection_lock = threading.Lock() self.on_connection_callback = None self.on_connection_error_callback = None @@ -51,6 +54,10 @@ def __init__(self, adapter: Adapter, addr: str, name: str = None): # Dictionnary for GATT characteristic callback self._gatt_characteristic_callbacks = {} + # Memory that could be allocated by native gattlib + self._services_ptr = None + self._characteristics_ptr = None + @property def mac_address(self) -> str: """Return Device MAC Address""" @@ -109,9 +116,24 @@ def register_on_disconnect(self, callback, user_data=None): self.disconnection_callback = callback def on_disconnection(user_data): + self._disconnection_lock.acquire() + if self.disconnection_callback: self.disconnection_callback() + # On disconnection, we do not need the list of GATT services and GATT characteristics + if self._services_ptr: + gattlib_free_mem(self._services_ptr) + self._services_ptr = None + if self._characteristics_ptr: + gattlib_free_mem(self._characteristics_ptr) + self._characteristics_ptr = None + + # Reset the connection handler + self._connection = None + + self._disconnection_lock.release() + gattlib_register_on_disconnect(self.connection, gattlib_disconnected_device_python_callback, gattlib_python_callback_args(on_disconnection, user_data)) @@ -130,14 +152,14 @@ def discover(self): # # Discover GATT Services # - _services = POINTER(GattlibPrimaryService)() + self._services_ptr = POINTER(GattlibPrimaryService)() _services_count = c_int(0) - ret = gattlib_discover_primary(self.connection, byref(_services), byref(_services_count)) + ret = gattlib_discover_primary(self.connection, byref(self._services_ptr), byref(_services_count)) handle_return(ret) self._services = {} for i in range(0, _services_count.value): - service = GattService(self, _services[i]) + service = GattService(self, self._services_ptr[i]) self._services[service.short_uuid] = service logger.debug("Service UUID:0x%x" % service.short_uuid) @@ -145,14 +167,14 @@ def discover(self): # # Discover GATT Characteristics # - _characteristics = POINTER(GattlibCharacteristic)() + self._characteristics_ptr = POINTER(GattlibCharacteristic)() _characteristics_count = c_int(0) - ret = gattlib_discover_char(self.connection, byref(_characteristics), byref(_characteristics_count)) + ret = gattlib_discover_char(self.connection, byref(self._characteristics_ptr), byref(_characteristics_count)) handle_return(ret) self._characteristics = {} for i in range(0, _characteristics_count.value): - characteristic = GattCharacteristic(self, _characteristics[i]) + characteristic = GattCharacteristic(self, self._characteristics_ptr[i]) self._characteristics[characteristic.short_uuid] = characteristic logger.debug("Characteristic UUID:0x%x" % characteristic.short_uuid) @@ -201,6 +223,9 @@ def get_advertisement_data(self): for i in range(_manufacturer_data_len.value): manufacturer_data[i] = c_bytearray.contents[i] & 0xFF + gattlib_free_mem(_advertisement_data) + gattlib_free_mem(_manufacturer_data) + return advertisement_data, _manufacturer_id.value, manufacturer_data @property