Skip to content

Commit 300dfd1

Browse files
committed
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 <simon.egli@arendi.ch>
1 parent 054f64e commit 300dfd1

File tree

3 files changed

+48
-7
lines changed

3 files changed

+48
-7
lines changed

dbus/gattlib.c

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ gboolean on_handle_device_property_change(
3939
gpointer user_data)
4040
{
4141
gatt_connection_t* connection = user_data;
42+
if(connection == NULL) return;
4243
gattlib_context_t* conn_context = connection->context;
4344

4445
// Retrieve 'Value' from 'arg_changed_properties'
@@ -52,14 +53,23 @@ gboolean on_handle_device_property_change(
5253
if (strcmp(key, "Connected") == 0) {
5354
if (!g_variant_get_boolean(value)) {
5455
// Disconnection case
56+
if(conn_context->handler_id != 0) {
57+
g_signal_handler_disconnect(conn_context->device,conn_context->handler_id);
58+
conn_context->handler_id = 0;
59+
}
5560
if (gattlib_has_valid_handler(&connection->disconnection)) {
5661
gattlib_call_disconnection_handler(&connection->disconnection);
5762
}
5863
}
5964
} else if (strcmp(key, "ServicesResolved") == 0) {
6065
if (g_variant_get_boolean(value)) {
66+
if(conn_context->handler_id == NULL) {
67+
return false;
68+
}
6169
// Stop the timeout for connection
62-
g_source_remove(conn_context->connection_timeout);
70+
if((conn_context != NULL) && (conn_context->connection_loop != NULL) && (connection != NULL)) {
71+
g_source_remove(conn_context->connection_timeout);
72+
}
6373

6474
// Tell we are now connected
6575
g_main_loop_quit(conn_context->connection_loop);
@@ -175,7 +185,7 @@ gatt_connection_t *gattlib_connect(void* adapter, const char *dst, unsigned long
175185
}
176186

177187
// Register a handle for notification
178-
g_signal_connect(device,
188+
conn_context->handler_id = g_signal_connect(device,
179189
"g-properties-changed",
180190
G_CALLBACK (on_handle_device_property_change),
181191
connection);
@@ -201,15 +211,21 @@ gatt_connection_t *gattlib_connect(void* adapter, const char *dst, unsigned long
201211
// and 'org.bluez.GattCharacteristic1' to be advertised at that moment.
202212
conn_context->connection_loop = g_main_loop_new(NULL, 0);
203213

214+
if(conn_context == NULL) goto FREE_DEVICE;
204215
conn_context->connection_timeout = g_timeout_add_seconds(CONNECT_TIMEOUT, stop_scan_func,
205216
conn_context->connection_loop);
217+
if(conn_context == NULL) goto FREE_DEVICE;
206218
g_main_loop_run(conn_context->connection_loop);
219+
if(conn_context == NULL) goto FREE_DEVICE;
207220
g_main_loop_unref(conn_context->connection_loop);
208221
// Set the attribute to NULL even if not required
209222
conn_context->connection_loop = NULL;
210223

211224
// Get list of objects belonging to Device Manager
212225
device_manager = get_device_manager_from_adapter(conn_context->adapter);
226+
if(device_manager == NULL || (conn_context == NULL) || (conn_context->adapter == NULL)) {
227+
goto FREE_DEVICE;
228+
}
213229
conn_context->dbus_objects = g_dbus_object_manager_get_objects(device_manager);
214230

215231
return connection;
@@ -254,7 +270,16 @@ int gattlib_disconnect(gatt_connection_t* connection) {
254270
g_object_unref(conn_context->device);
255271
g_list_free_full(conn_context->dbus_objects, g_object_unref);
256272
disconnect_all_notifications(conn_context);
273+
if(conn_context->connection_loop != NULL) {
274+
g_main_loop_quit(conn_context->connection_loop);
275+
g_main_loop_unref(conn_context->connection_loop);
276+
}
277+
if(conn_context->handler_id != 0) {
278+
g_signal_handler_disconnect(conn_context->device,conn_context->handler_id);
279+
conn_context->handler_id = 0;
280+
}
257281

282+
free(conn_context);
258283
free(connection->context);
259284
free(connection);
260285
return GATTLIB_SUCCESS;

dbus/gattlib_adapter.c

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -278,8 +278,14 @@ int gattlib_adapter_scan_enable_with_filter(void *adapter, uuid_t **uuid_list, i
278278
// Now, start BLE discovery
279279
org_bluez_adapter1_call_start_discovery_sync(gattlib_adapter->adapter_proxy, NULL, &error);
280280
if (error) {
281-
fprintf(stderr, "Failed to start discovery: %s\n", error->message);
282-
g_error_free(error);
281+
fprintf(stderr, "Failed to start discovery %d: %s\n", error->code, error->message );
282+
//If adapter complains about already started process I have not found any other solution for
283+
//than to power cycle the adapter
284+
if(error->code == 36) {
285+
fprintf(stderr, "Power cycling adapter\n");
286+
org_bluez_adapter1_set_powered(gattlib_adapter->adapter_proxy, false);
287+
org_bluez_adapter1_set_powered(gattlib_adapter->adapter_proxy, true);
288+
}
283289
return GATTLIB_ERROR_DBUS;
284290
}
285291

@@ -336,16 +342,24 @@ int gattlib_adapter_scan_disable(void* adapter) {
336342
int gattlib_adapter_close(void* adapter)
337343
{
338344
struct gattlib_adapter *gattlib_adapter = adapter;
339-
340-
g_object_unref(gattlib_adapter->device_manager);
341-
g_object_unref(gattlib_adapter->adapter_proxy);
345+
if(gattlib_adapter == NULL) return GATTLIB_SUCCESS;
346+
fprintf(stderr, "Gattlib adapter is: %lu\n" , gattlib_adapter);
347+
fprintf(stderr, "Gattlib adapter device manager is: %lu\n" , gattlib_adapter->device_manager);
348+
349+
if (gattlib_adapter->device_manager != NULL) {
350+
g_object_unref(gattlib_adapter->device_manager);
351+
}
352+
if (gattlib_adapter->device_manager != NULL) {
353+
g_object_unref(gattlib_adapter->adapter_proxy);
354+
}
342355
free(gattlib_adapter);
343356

344357
return GATTLIB_SUCCESS;
345358
}
346359

347360
gboolean stop_scan_func(gpointer data)
348361
{
362+
if(data != NULL)
349363
g_main_loop_quit(data);
350364
return FALSE;
351365
}

dbus/gattlib_internal.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,8 @@ typedef struct {
6363

6464
// List of 'OrgBluezGattCharacteristic1*' which has an attached notification
6565
GList *notified_characteristics;
66+
//handler of attached notification
67+
gulong handler_id;
6668
} gattlib_context_t;
6769

6870
struct gattlib_adapter {

0 commit comments

Comments
 (0)