Skip to content

Commit

Permalink
Hardening the library
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
Deadolus committed Nov 5, 2020
1 parent 054f64e commit 300dfd1
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 7 deletions.
29 changes: 27 additions & 2 deletions dbus/gattlib.c
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
24 changes: 19 additions & 5 deletions dbus/gattlib_adapter.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -336,16 +342,24 @@ 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;
fprintf(stderr, "Gattlib adapter is: %lu\n" , gattlib_adapter);
fprintf(stderr, "Gattlib adapter device manager is: %lu\n" , gattlib_adapter->device_manager);

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;
}

gboolean stop_scan_func(gpointer data)
{
if(data != NULL)
g_main_loop_quit(data);
return FALSE;
}
2 changes: 2 additions & 0 deletions dbus/gattlib_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 300dfd1

Please sign in to comment.