Single lock for all gattlib library

Author: oliviermartin

common/gattlib_callback_connected_device.c

@@ -50,22 +50,31 @@ static gpointer _gattlib_connected_device_thread(gpointer data) {
 	gattlib_connection_t* connection = data;
 	const gchar *device_mac_address = org_bluez_device1_get_address(connection->backend.device);
 
-	// Mutex to ensure the device is valid and not freed during its use
-	g_mutex_lock(&connection->device->device_mutex);
 	// Mutex to ensure the handler is valid
-	g_rec_mutex_lock(&connection->on_connection.mutex);
+	g_rec_mutex_lock(&m_gattlib_mutex);
+
+	if (!gattlib_connection_is_connected(connection)) {
+		g_rec_mutex_unlock(&m_gattlib_mutex);
+		return NULL;
+	}
 
 	if (!gattlib_has_valid_handler(&connection->on_connection)) {
-		goto EXIT;
+		g_rec_mutex_unlock(&m_gattlib_mutex);
+		return NULL;
 	}
 
+	// Ensure we increment device reference counter to prevent the device/connection is freed during the execution
+	gattlib_device_ref(connection->device);
+
+	// We need to release the lock here to ensure the connection callback that is actually
+	// doing the application sepcific work is not locking the BLE state.
+	g_rec_mutex_unlock(&m_gattlib_mutex);
+
 	connection->on_connection.callback.connection_handler(
 		connection->device->adapter, device_mac_address, connection, 0 /* no error */,
 		connection->on_connection.user_data);
 
-EXIT:
-	g_rec_mutex_unlock(&connection->on_connection.mutex);
-	g_mutex_unlock(&connection->device->device_mutex);
+	gattlib_device_unref(connection->device);
 	return NULL;
 }
 
@@ -75,6 +84,10 @@ static void* _connected_device_thread_args_allocator(va_list args) {
 }
 
 void gattlib_on_connected_device(gattlib_connection_t* connection) {
+	if (!gattlib_device_is_valid(connection->device)) {
+		return;
+	}
+
 	gattlib_handler_dispatch_to_thread(
 		&connection->on_connection,
 #if defined(WITH_PYTHON)

common/gattlib_callback_disconnected_device.c

@@ -31,9 +31,14 @@ void gattlib_disconnected_device_python_callback(gattlib_connection_t* connectio
 #endif
 
 void gattlib_on_disconnected_device(gattlib_connection_t* connection) {
-	if (gattlib_has_valid_handler(&connection->on_disconnection)) {
-		g_rec_mutex_lock(&connection->on_disconnection.mutex);
+	g_rec_mutex_lock(&m_gattlib_mutex);
+
+	if (!gattlib_connection_is_connected(connection)) {
+		g_rec_mutex_unlock(&m_gattlib_mutex);
+		return;
+	}
 
+	if (gattlib_has_valid_handler(&connection->on_disconnection)) {
 #if defined(WITH_PYTHON)
 		// Check if we are using the Python callback, in case of Python argument we keep track of the argument to free them
 		// once we are done with the handler.
@@ -44,16 +49,16 @@ void gattlib_on_disconnected_device(gattlib_connection_t* connection) {
 
 		// For GATT disconnection we do not use thread to ensure the callback is synchronous.
 		connection->on_disconnection.callback.disconnection_handler(connection, connection->on_disconnection.user_data);
-
-		g_rec_mutex_unlock(&connection->on_disconnection.mutex);
 	}
 
-	// Signal the device is now disconnected
-	g_mutex_lock(&connection->device->device_mutex);
-	connection->disconnection_wait.value = true;
-	g_cond_broadcast(&connection->disconnection_wait.condition);
-	g_mutex_unlock(&connection->device->device_mutex);
-
 	// Clean GATTLIB connection on disconnection
 	gattlib_connection_free(connection);
+
+	g_rec_mutex_unlock(&m_gattlib_mutex);
+
+	// Signal the device is now disconnected
+	g_mutex_lock(&m_gattlib_signal.mutex);
+	m_gattlib_signal.signals |= GATTLIB_SIGNAL_DEVICE_DISCONNECTION;
+	g_cond_broadcast(&m_gattlib_signal.condition);
+	g_mutex_unlock(&m_gattlib_signal.mutex);
 }

common/gattlib_callback_discovered_device.c

@@ -55,21 +55,33 @@ struct gattlib_discovered_device_thread_args {
 static gpointer _gattlib_discovered_device_thread(gpointer data) {
 	struct gattlib_discovered_device_thread_args* args = data;
 
-	g_rec_mutex_lock(&args->gattlib_adapter->discovered_device_callback.mutex);
+	g_rec_mutex_lock(&m_gattlib_mutex);
+
+	if (!gattlib_adapter_is_valid(args->gattlib_adapter)) {
+		g_rec_mutex_unlock(&m_gattlib_mutex);
+		goto EXIT;
+	}
 
 	if (!gattlib_has_valid_handler(&args->gattlib_adapter->discovered_device_callback)) {
+		g_rec_mutex_unlock(&m_gattlib_mutex);
 		goto EXIT;
 	}
 
+	// Increase adapter reference counter to ensure the adapter is not freed while
+	// the callback is in use.
+	gattlib_adapter_ref(args->gattlib_adapter);
+
+	g_rec_mutex_unlock(&m_gattlib_mutex);
+
 	args->gattlib_adapter->discovered_device_callback.callback.discovered_device(
 		args->gattlib_adapter,
 		args->mac_address, args->name,
 		args->gattlib_adapter->discovered_device_callback.user_data
 	);
 
-EXIT:
-	g_rec_mutex_unlock(&args->gattlib_adapter->discovered_device_callback.mutex);
+	gattlib_adapter_unref(args->gattlib_adapter);
 
+EXIT:
 	free(args->mac_address);
 	if (args->name != NULL) {
 		free(args->name);
@@ -96,6 +108,10 @@ static void* _discovered_device_thread_args_allocator(va_list args) {
 }
 
 void gattlib_on_discovered_device(gattlib_adapter_t* gattlib_adapter, OrgBluezDevice1* device1) {
+	if (!gattlib_adapter_is_valid(gattlib_adapter)) {
+		return;
+	}
+
 	gattlib_handler_dispatch_to_thread(
 		&gattlib_adapter->discovered_device_callback,
 #if defined(WITH_PYTHON)

common/gattlib_callback_notification_device.c

@@ -56,14 +56,19 @@ void gattlib_notification_device_thread(gpointer data, gpointer user_data) {
 	struct gattlib_notification_device_thread_args* args = data;
 	struct gattlib_handler* handler = user_data;
 
-	g_rec_mutex_lock(&handler->mutex);
+	g_rec_mutex_lock(&m_gattlib_mutex);
+
+	if (!gattlib_connection_is_connected(args->connection)) {
+		g_rec_mutex_unlock(&m_gattlib_mutex);
+		return;
+	}
 
 	handler->callback.notification_handler(
 		args->uuid, args->data, args->data_length,
 		handler->user_data
 	);
 
-	g_rec_mutex_unlock(&handler->mutex);
+	g_rec_mutex_unlock(&m_gattlib_mutex);
 
 	if (args->uuid != NULL) {
 		free(args->uuid);

common/gattlib_common.c

@@ -8,11 +8,21 @@
 
 #include "gattlib_internal.h"
 
+
 int gattlib_register_notification(gattlib_connection_t* connection, gattlib_event_handler_t notification_handler, void* user_data) {
 	GError *error = NULL;
+	int ret = GATTLIB_SUCCESS;
+
+	g_rec_mutex_lock(&m_gattlib_mutex);
 
 	if (connection == NULL) {
-		return GATTLIB_INVALID_PARAMETER;
+		ret = GATTLIB_INVALID_PARAMETER;
+		goto EXIT;
+	}
+
+	if (!gattlib_device_is_valid(connection->device)) {
+		ret = GATTLIB_INVALID_PARAMETER;
+		goto EXIT;
 	}
 
 	connection->notification.callback.notification_handler = notification_handler;
@@ -25,19 +35,33 @@ int gattlib_register_notification(gattlib_connection_t* connection, gattlib_even
 	if (error != NULL) {
 		GATTLIB_LOG(GATTLIB_ERROR, "gattlib_register_notification: Failed to create thread pool: %s", error->message);
 		g_error_free(error);
-		return GATTLIB_ERROR_INTERNAL;
+		ret = GATTLIB_ERROR_INTERNAL;
+		goto EXIT;
 	} else {
 		assert(connection->notification.thread_pool != NULL);
-		return GATTLIB_SUCCESS;
 	}
+
+EXIT:
+	g_rec_mutex_unlock(&m_gattlib_mutex);
+	return ret;
 }
 
 int gattlib_register_indication(gattlib_connection_t* connection, gattlib_event_handler_t indication_handler, void* user_data) {
 	GError *error = NULL;
+	int ret = GATTLIB_SUCCESS;
+
+	g_rec_mutex_lock(&m_gattlib_mutex);
 
 	if (connection == NULL) {
-		return GATTLIB_INVALID_PARAMETER;
+		ret = GATTLIB_INVALID_PARAMETER;
+		goto EXIT;
 	}
+
+	if (!gattlib_device_is_valid(connection->device)) {
+		ret = GATTLIB_INVALID_PARAMETER;
+		goto EXIT;
+	}
+
 	connection->indication.callback.notification_handler = indication_handler;
 	connection->indication.user_data = user_data;
 
@@ -48,19 +72,36 @@ int gattlib_register_indication(gattlib_connection_t* connection, gattlib_event_
 	if (error != NULL) {
 		GATTLIB_LOG(GATTLIB_ERROR, "gattlib_register_indication: Failed to create thread pool: %s", error->message);
 		g_error_free(error);
-		return GATTLIB_ERROR_INTERNAL;
-	} else {
-		return GATTLIB_SUCCESS;
+		ret = GATTLIB_ERROR_INTERNAL;
+		goto EXIT;
 	}
+
+EXIT:
+	g_rec_mutex_unlock(&m_gattlib_mutex);
+	return ret;
 }
 
 int gattlib_register_on_disconnect(gattlib_connection_t *connection, gattlib_disconnection_handler_t handler, void* user_data) {
+	int ret = GATTLIB_SUCCESS;
+
+	g_rec_mutex_lock(&m_gattlib_mutex);
+
 	if (connection == NULL) {
-		return GATTLIB_INVALID_PARAMETER;
+		ret = GATTLIB_INVALID_PARAMETER;
+		goto EXIT;
+	}
+
+	if (!gattlib_device_is_valid(connection->device)) {
+		ret = GATTLIB_INVALID_PARAMETER;
+		goto EXIT;
 	}
+
 	connection->on_disconnection.callback.disconnection_handler = handler;
 	connection->on_disconnection.user_data = user_data;
-	return GATTLIB_SUCCESS;
+
+EXIT:
+	g_rec_mutex_unlock(&m_gattlib_mutex);
+	return ret;
 }
 
 void bt_uuid_to_uuid(bt_uuid_t* bt_uuid, uuid_t* uuid) {
@@ -144,10 +185,8 @@ int gattlib_uuid_cmp(const uuid_t *uuid1, const uuid_t *uuid2) {
 }
 
 void gattlib_handler_free(struct gattlib_handler* handler) {
-	g_rec_mutex_lock(&handler->mutex);
-
 	if (!gattlib_has_valid_handler(handler)) {
-		goto EXIT;
+		return;
 	}
 
 	// Reset callback to stop calling it after we stopped
@@ -172,9 +211,6 @@ void gattlib_handler_free(struct gattlib_handler* handler) {
 		g_thread_pool_free(handler->thread_pool, FALSE /* immediate */, TRUE /* wait */);
 		handler->thread_pool = NULL;
 	}
-
-EXIT:
-	g_rec_mutex_unlock(&handler->mutex);
 }
 
 bool gattlib_has_valid_handler(struct gattlib_handler* handler) {
@@ -185,8 +221,11 @@ void gattlib_handler_dispatch_to_thread(struct gattlib_handler* handler, void (*
 		GThreadFunc thread_func, const char* thread_name, void* (*thread_args_allocator)(va_list args), ...) {
 	GError *error = NULL;
 
+	g_rec_mutex_lock(&m_gattlib_mutex);
+
 	if (!gattlib_has_valid_handler(handler)) {
 		// We do not have (anymore) a callback, nothing to do
+		g_rec_mutex_unlock(&m_gattlib_mutex);
 		return;
 	}
 
@@ -198,6 +237,8 @@ void gattlib_handler_dispatch_to_thread(struct gattlib_handler* handler, void (*
 	}
 #endif
 
+	g_rec_mutex_unlock(&m_gattlib_mutex);
+
 	// We create a thread to ensure the callback is not blocking the mainloop
 	va_list args;
 	va_start(args, thread_args_allocator);
@@ -218,3 +259,10 @@ void gattlib_free_mem(void *ptr) {
 		free(ptr);
 	}
 }
+
+int gattlib_device_ref(gattlib_device_t* device) {
+	g_rec_mutex_lock(&m_gattlib_mutex);
+	device->reference_counter++;
+	g_rec_mutex_unlock(&m_gattlib_mutex);
+	return GATTLIB_SUCCESS;
+}