From 38ba174ce379ab903dab6764e477d969d4da67a2 Mon Sep 17 00:00:00 2001 From: mikew-ed Date: Mon, 17 Jul 2017 08:46:29 -0700 Subject: [PATCH] iteration / thread race condition fixes --- bluez/gattlib_connect.c | 41 +++++++++++++++++++++++++++++++------- bluez/gattlib_discover.c | 7 ++++--- bluez/gattlib_internal.h | 7 +++++++ bluez/gattlib_read_write.c | 6 ++++-- 4 files changed, 49 insertions(+), 12 deletions(-) diff --git a/bluez/gattlib_connect.c b/bluez/gattlib_connect.c index 46f13f32..c8ff582a 100644 --- a/bluez/gattlib_connect.c +++ b/bluez/gattlib_connect.c @@ -165,6 +165,7 @@ static void io_connect_cb(GIOChannel *io, GError *err, gpointer user_data) { } } +#ifdef USE_THREAD static void *connection_thread(void* arg) { struct gattlib_thread_t* loop_thread = arg; @@ -175,6 +176,7 @@ static void *connection_thread(void* arg) { g_main_loop_unref(loop_thread->loop); assert(0); } +#endif static gatt_connection_t *initialize_gattlib_connection(const gchar *src, const gchar *dst, uint8_t dest_type, BtIOSecLevel sec_level, int psm, int mtu, @@ -186,6 +188,7 @@ static gatt_connection_t *initialize_gattlib_connection(const gchar *src, const /* Check if the GattLib thread has been started */ if (g_gattlib_thread.ref == 0) { +#ifdef USE_THREAD /* Start it */ /* Create a thread that will handle Bluetooth events */ @@ -199,11 +202,15 @@ static gatt_connection_t *initialize_gattlib_connection(const gchar *src, const while (!g_gattlib_thread.loop || !g_main_loop_is_running (g_gattlib_thread.loop)) { usleep(1000); } - } else { - /* Increase the reference to know how many GATT connection use the loop */ - g_gattlib_thread.ref++; +#else + g_gattlib_thread.loop_context = g_main_context_new(); + g_gattlib_thread.loop = g_main_loop_new(g_gattlib_thread.loop_context, TRUE); +#endif } + /* Increase the reference to know how many GATT connection use the loop */ + g_gattlib_thread.ref++; + /* Remote device */ if (dst == NULL) { fprintf(stderr, "Remote Bluetooth address required\n"); @@ -308,6 +315,22 @@ static BtIOSecLevel get_bt_io_sec_level(gattlib_bt_sec_level_t sec_level) { } } + + +void gattlib_iteration(void){ +#ifdef USE_THREAD + if( pthread_self() == g_gattlib_thread.thread) { + g_main_context_iteration(g_gattlib_thread.loop_context, FALSE); + } + else { + pthread_yield(); + } +#else + g_main_context_iteration(g_gattlib_thread.loop_context, TRUE); +#endif +} + + gatt_connection_t *gattlib_connect_async(const char *src, const char *dst, uint8_t dest_type, gattlib_bt_sec_level_t sec_level, int psm, int mtu, gatt_connect_cb_t connect_cb) @@ -335,11 +358,12 @@ static gboolean connection_timeout(gpointer user_data) { * @param psm Specify the PSM for GATT/ATT over BR/EDR * @param mtu Specify the MTU size */ + gatt_connection_t *gattlib_connect(const char *src, const char *dst, uint8_t dest_type, gattlib_bt_sec_level_t sec_level, int psm, int mtu) { BtIOSecLevel bt_io_sec_level = get_bt_io_sec_level(sec_level); - io_connect_arg_t io_connect_arg; + io_connect_arg_t io_connect_arg = {}; GSource* timeout; gatt_connection_t *conn = initialize_gattlib_connection(src, dst, dest_type, bt_io_sec_level, @@ -358,7 +382,7 @@ gatt_connection_t *gattlib_connect(const char *src, const char *dst, // Wait for the connection to be done while ((io_connect_arg.connected == FALSE) && (io_connect_arg.timeout == FALSE)) { - g_main_context_iteration(g_gattlib_thread.loop_context, FALSE); + gattlib_iteration(); } // Disconnect the timeout source @@ -379,6 +403,7 @@ gatt_connection_t *gattlib_connect(const char *src, const char *dst, int gattlib_disconnect(gatt_connection_t* connection) { gattlib_context_t* conn_context = connection->context; + #if BLUEZ_VERSION_MAJOR == 4 // Stop the I/O Channel GIOStatus status = g_io_channel_shutdown(conn_context->io, FALSE, NULL); @@ -387,15 +412,16 @@ int gattlib_disconnect(gatt_connection_t* connection) { #endif g_attrib_unref(conn_context->attrib); - free(conn_context->characteristics); free(connection->context); free(connection); - +#if 0 // todo fix, but for now just leave thread running //TODO: Add a mutex around this code to avoid a race condition /* Decrease the reference counter of the loop */ g_gattlib_thread.ref--; /* Check if we are the last one */ + + if (g_gattlib_thread.ref == 0) { g_main_loop_quit(g_gattlib_thread.loop); g_main_loop_unref(g_gattlib_thread.loop); @@ -404,6 +430,7 @@ int gattlib_disconnect(gatt_connection_t* connection) { // Detach the thread pthread_detach(g_gattlib_thread.thread); } +#endif return 0; } diff --git a/bluez/gattlib_discover.c b/bluez/gattlib_discover.c index 1b45e51e..29a0c484 100644 --- a/bluez/gattlib_discover.c +++ b/bluez/gattlib_discover.c @@ -86,7 +86,7 @@ int gattlib_discover_primary(gatt_connection_t* connection, gattlib_primary_serv // Wait for completion while(user_data.discovered == FALSE) { - g_main_context_iteration(g_gattlib_thread.loop_context, FALSE); + gattlib_iteration(); } *services = user_data.services; @@ -150,9 +150,10 @@ int gattlib_discover_char_range(gatt_connection_t* connection, int start, int en // Wait for completion while(user_data.discovered == FALSE) { - g_main_context_iteration(g_gattlib_thread.loop_context, FALSE); + gattlib_iteration(); } + *characteristics = user_data.characteristics; *characteristics_count = user_data.characteristics_count; @@ -264,7 +265,7 @@ int gattlib_discover_desc_range(gatt_connection_t* connection, int start, int en // Wait for completion while(descriptor_data.discovered == FALSE) { - g_main_context_iteration(g_gattlib_thread.loop_context, FALSE); + gattlib_iteration(); } *descriptors = descriptor_data.descriptors; diff --git a/bluez/gattlib_internal.h b/bluez/gattlib_internal.h index 3300dbea..391b4e36 100644 --- a/bluez/gattlib_internal.h +++ b/bluez/gattlib_internal.h @@ -24,6 +24,12 @@ #ifndef __GATTLIB_INTERNAL_H__ #define __GATTLIB_INTERNAL_H__ + +// uncomment the the following line to have an glib main event (g_main_loop_run) +// loop created in its own thread for gattlib, otherwise operations will +// do gattlib_iterate when required. +//#define USE_THREAD + #include #define BLUEZ_VERSIONS(major, minor) (((major) << 8) | (minor)) @@ -62,6 +68,7 @@ extern struct gattlib_thread_t g_gattlib_thread; GSource* gattlib_watch_connection_full(GIOChannel* io, GIOCondition condition, GIOFunc func, gpointer user_data, GDestroyNotify notify); GSource* gattlib_timeout_add_seconds(guint interval, GSourceFunc function, gpointer data); +void gattlib_iteration(void); void uuid_to_bt_uuid(uuid_t* uuid, bt_uuid_t* bt_uuid); void bt_uuid_to_uuid(bt_uuid_t* bt_uuid, uuid_t* uuid); diff --git a/bluez/gattlib_read_write.c b/bluez/gattlib_read_write.c index 5239e548..1b7c9b13 100644 --- a/bluez/gattlib_read_write.c +++ b/bluez/gattlib_read_write.c @@ -22,6 +22,8 @@ */ #include +#include +#include #include "gattlib_internal.h" @@ -120,7 +122,7 @@ int gattlib_read_char_by_uuid(gatt_connection_t* connection, uuid_t* uuid, // Wait for completion of the event while(gattlib_result->completed == FALSE) { - g_main_context_iteration(g_gattlib_thread.loop_context, FALSE); + gattlib_iteration(); } *buffer_len = gattlib_result->buffer_len; @@ -178,7 +180,7 @@ int gattlib_write_char_by_handle(gatt_connection_t* connection, uint16_t handle, // Wait for completion of the event while(write_completed == FALSE) { - g_main_context_iteration(g_gattlib_thread.loop_context, FALSE); + gattlib_iteration(); } return 0;