From ffd62547501087eebc96b2221149766c71500064 Mon Sep 17 00:00:00 2001 From: Simon Egli Date: Thu, 5 Nov 2020 13:18:06 +0100 Subject: [PATCH] Hardening the library I had multiple segsev crashes in different locations all over the library. Mostly they seem to stem from instances where the library does just assume that some pointer is still valid, although the object was already freed. With these changes the library now runs stable for me. These changes are not very tidy, as I stumbled through the library and fixed stuff one by one. Contributing my changes back in the hope that the library will get better - please pick and choose and verify my changes. Signed-off-by: Simon Egli --- dbus/gattlib.c | 29 +++++++++++++++++++++++++++-- dbus/gattlib_adapter.c | 22 +++++++++++++++++----- dbus/gattlib_internal.h | 2 ++ 3 files changed, 46 insertions(+), 7 deletions(-) diff --git a/dbus/gattlib.c b/dbus/gattlib.c index c53b02ff..08ab0cca 100644 --- a/dbus/gattlib.c +++ b/dbus/gattlib.c @@ -39,6 +39,7 @@ gboolean on_handle_device_property_change( gpointer user_data) { gatt_connection_t* connection = user_data; + if(connection == NULL) return; gattlib_context_t* conn_context = connection->context; // Retrieve 'Value' from 'arg_changed_properties' @@ -52,14 +53,23 @@ gboolean on_handle_device_property_change( if (strcmp(key, "Connected") == 0) { if (!g_variant_get_boolean(value)) { // Disconnection case + if(conn_context->handler_id != 0) { + g_signal_handler_disconnect(conn_context->device,conn_context->handler_id); + conn_context->handler_id = 0; + } if (gattlib_has_valid_handler(&connection->disconnection)) { gattlib_call_disconnection_handler(&connection->disconnection); } } } else if (strcmp(key, "ServicesResolved") == 0) { if (g_variant_get_boolean(value)) { + if(conn_context->handler_id == NULL) { + return false; + } // Stop the timeout for connection - g_source_remove(conn_context->connection_timeout); + if((conn_context != NULL) && (conn_context->connection_loop != NULL) && (connection != NULL)) { + g_source_remove(conn_context->connection_timeout); + } // Tell we are now connected g_main_loop_quit(conn_context->connection_loop); @@ -175,7 +185,7 @@ gatt_connection_t *gattlib_connect(void* adapter, const char *dst, unsigned long } // Register a handle for notification - g_signal_connect(device, + conn_context->handler_id = g_signal_connect(device, "g-properties-changed", G_CALLBACK (on_handle_device_property_change), connection); @@ -201,15 +211,21 @@ gatt_connection_t *gattlib_connect(void* adapter, const char *dst, unsigned long // and 'org.bluez.GattCharacteristic1' to be advertised at that moment. conn_context->connection_loop = g_main_loop_new(NULL, 0); + if(conn_context == NULL) goto FREE_DEVICE; conn_context->connection_timeout = g_timeout_add_seconds(CONNECT_TIMEOUT, stop_scan_func, conn_context->connection_loop); + if(conn_context == NULL) goto FREE_DEVICE; g_main_loop_run(conn_context->connection_loop); + if(conn_context == NULL) goto FREE_DEVICE; g_main_loop_unref(conn_context->connection_loop); // Set the attribute to NULL even if not required conn_context->connection_loop = NULL; // Get list of objects belonging to Device Manager device_manager = get_device_manager_from_adapter(conn_context->adapter); + if(device_manager == NULL || (conn_context == NULL) || (conn_context->adapter == NULL)) { + goto FREE_DEVICE; + } conn_context->dbus_objects = g_dbus_object_manager_get_objects(device_manager); return connection; @@ -254,7 +270,16 @@ int gattlib_disconnect(gatt_connection_t* connection) { g_object_unref(conn_context->device); g_list_free_full(conn_context->dbus_objects, g_object_unref); disconnect_all_notifications(conn_context); + if(conn_context->connection_loop != NULL) { + g_main_loop_quit(conn_context->connection_loop); + g_main_loop_unref(conn_context->connection_loop); + } + if(conn_context->handler_id != 0) { + g_signal_handler_disconnect(conn_context->device,conn_context->handler_id); + conn_context->handler_id = 0; + } + free(conn_context); free(connection->context); free(connection); return GATTLIB_SUCCESS; diff --git a/dbus/gattlib_adapter.c b/dbus/gattlib_adapter.c index f7363f08..ac9a60df 100644 --- a/dbus/gattlib_adapter.c +++ b/dbus/gattlib_adapter.c @@ -278,8 +278,14 @@ int gattlib_adapter_scan_enable_with_filter(void *adapter, uuid_t **uuid_list, i // Now, start BLE discovery org_bluez_adapter1_call_start_discovery_sync(gattlib_adapter->adapter_proxy, NULL, &error); if (error) { - fprintf(stderr, "Failed to start discovery: %s\n", error->message); - g_error_free(error); + fprintf(stderr, "Failed to start discovery %d: %s\n", error->code, error->message ); + //If adapter complains about already started process I have not found any other solution for + //than to power cycle the adapter + if(error->code == 36) { + fprintf(stderr, "Power cycling adapter\n"); + org_bluez_adapter1_set_powered(gattlib_adapter->adapter_proxy, false); + org_bluez_adapter1_set_powered(gattlib_adapter->adapter_proxy, true); + } return GATTLIB_ERROR_DBUS; } @@ -336,9 +342,14 @@ int gattlib_adapter_scan_disable(void* adapter) { int gattlib_adapter_close(void* adapter) { struct gattlib_adapter *gattlib_adapter = adapter; - - g_object_unref(gattlib_adapter->device_manager); - g_object_unref(gattlib_adapter->adapter_proxy); + if(gattlib_adapter == NULL) return GATTLIB_SUCCESS; + + if (gattlib_adapter->device_manager != NULL) { + g_object_unref(gattlib_adapter->device_manager); + } + if (gattlib_adapter->device_manager != NULL) { + g_object_unref(gattlib_adapter->adapter_proxy); + } free(gattlib_adapter); return GATTLIB_SUCCESS; @@ -346,6 +357,7 @@ int gattlib_adapter_close(void* adapter) gboolean stop_scan_func(gpointer data) { + if(data != NULL) g_main_loop_quit(data); return FALSE; } diff --git a/dbus/gattlib_internal.h b/dbus/gattlib_internal.h index 55c8bd9c..3023a42f 100644 --- a/dbus/gattlib_internal.h +++ b/dbus/gattlib_internal.h @@ -63,6 +63,8 @@ typedef struct { // List of 'OrgBluezGattCharacteristic1*' which has an attached notification GList *notified_characteristics; + //handler of attached notification + gulong handler_id; } gattlib_context_t; struct gattlib_adapter {