From 42602e6465675e4ecb2ec9d3ebadc0e604fea0ce Mon Sep 17 00:00:00 2001 From: Emil Gydesen Date: Mon, 9 Sep 2024 17:16:33 +0200 Subject: [PATCH] Bluetooth: TBS: Replace busy bool with atomic Replace the busy boolean flag with an atomic value. This prevents any race conditions with the implementation. The discovery procedure is also now properly guarded with it so that in case that any procedure is currently in progress for the specific connection, then a new discovery procedure cannot happen until all other procedures are finished. Signed-off-by: Emil Gydesen --- subsys/bluetooth/audio/tbs_client.c | 63 +++++++++++++++++++++++++-- subsys/bluetooth/audio/tbs_internal.h | 10 ++++- 2 files changed, 68 insertions(+), 5 deletions(-) diff --git a/subsys/bluetooth/audio/tbs_client.c b/subsys/bluetooth/audio/tbs_client.c index e222cd8e52c..b48ca91f22b 100644 --- a/subsys/bluetooth/audio/tbs_client.c +++ b/subsys/bluetooth/audio/tbs_client.c @@ -769,7 +769,7 @@ static void initialize_net_buf_read_buffer(struct bt_tbs_instance *inst) static void tbs_client_gatt_read_complete(struct bt_tbs_instance *inst) { (void)memset(&inst->read_params, 0, sizeof(inst->read_params)); - inst->busy = false; + atomic_clear_bit(inst->flags, BT_TBS_CLIENT_FLAG_BUSY); } static int tbs_client_gatt_read(struct bt_conn *conn, struct bt_tbs_instance *inst, uint16_t handle, @@ -777,7 +777,9 @@ static int tbs_client_gatt_read(struct bt_conn *conn, struct bt_tbs_instance *in { int err; - if (inst->busy) { + if (atomic_test_and_set_bit(inst->flags, BT_TBS_CLIENT_FLAG_BUSY)) { + LOG_DBG("Instance is busy"); + return -EBUSY; } @@ -787,7 +789,6 @@ static int tbs_client_gatt_read(struct bt_conn *conn, struct bt_tbs_instance *in inst->read_params.handle_count = 1U; inst->read_params.single.handle = handle; inst->read_params.single.offset = 0U; - inst->busy = true; err = bt_gatt_read(conn, &inst->read_params); if (err != 0) { @@ -827,6 +828,15 @@ static void tbs_client_discover_complete(struct bt_conn *conn, int err) /* Clear the current instance in discovery */ srv_inst->current_inst = NULL; +#if defined(CONFIG_BT_TBS_CLIENT_GTBS) + atomic_clear_bit(srv_inst->gtbs_inst.flags, BT_TBS_CLIENT_FLAG_BUSY); +#endif /* CONFIG_BT_TBS_CLIENT_GTBS */ +#if defined(CONFIG_BT_TBS_CLIENT_TBS) + for (size_t i = 0U; i < ARRAY_SIZE(srv_inst->tbs_insts); i++) { + atomic_clear_bit(srv_inst->tbs_insts[i].flags, BT_TBS_CLIENT_FLAG_BUSY); + } +#endif /* CONFIG_BT_TBS_CLIENT_TBS */ + SYS_SLIST_FOR_EACH_CONTAINER_SAFE(&tbs_client_cbs, listener, next, _node) { if (listener->discover != NULL) { listener->discover(conn, err, inst_cnt(srv_inst), gtbs_found(srv_inst)); @@ -2381,6 +2391,48 @@ int bt_tbs_client_read_friendly_name(struct bt_conn *conn, uint8_t inst_index) } #endif /* defined(CONFIG_BT_TBS_CLIENT_CALL_FRIENDLY_NAME) */ +static bool check_and_set_all_busy(struct bt_tbs_server_inst *srv_inst) +{ + bool all_idle = true; + +#if defined(CONFIG_BT_TBS_CLIENT_GTBS) + if (atomic_test_and_set_bit(srv_inst->gtbs_inst.flags, BT_TBS_CLIENT_FLAG_BUSY)) { + LOG_DBG("GTBS is busy"); + + return false; + } +#endif /* CONFIG_BT_TBS_CLIENT_GTBS */ + +#if defined(CONFIG_BT_TBS_CLIENT_TBS) + size_t num_free; + + for (num_free = 0U; num_free < ARRAY_SIZE(srv_inst->tbs_insts); num_free++) { + struct bt_tbs_instance *tbs_inst = &srv_inst->tbs_insts[num_free]; + + if (atomic_test_and_set_bit(tbs_inst->flags, BT_TBS_CLIENT_FLAG_BUSY)) { + LOG_DBG("inst[%zu] (%p) is busy", num_free, tbs_inst); + all_idle = false; + + break; + } + } +#endif /* CONFIG_BT_TBS_CLIENT_TBS */ + + /* If any is busy, revert any busy states we've set */ + if (!all_idle) { +#if defined(CONFIG_BT_TBS_CLIENT_GTBS) + atomic_clear_bit(srv_inst->gtbs_inst.flags, BT_TBS_CLIENT_FLAG_BUSY); +#endif /* CONFIG_BT_TBS_CLIENT_GTBS */ +#if defined(CONFIG_BT_TBS_CLIENT_TBS) + for (uint8_t i = 0U; i < num_free; i++) { + atomic_clear_bit(srv_inst->tbs_insts[i].flags, BT_TBS_CLIENT_FLAG_BUSY); + } +#endif /* CONFIG_BT_TBS_CLIENT_TBS */ + } + + return all_idle; +} + int bt_tbs_client_discover(struct bt_conn *conn) { uint8_t conn_index; @@ -2393,7 +2445,10 @@ int bt_tbs_client_discover(struct bt_conn *conn) conn_index = bt_conn_index(conn); srv_inst = &srv_insts[conn_index]; - if (srv_inst->current_inst) { + /* Before we do discovery we ensure that all TBS instances are currently not busy as to not + * interfere with any procedures in progress + */ + if (!check_and_set_all_busy(srv_inst)) { return -EBUSY; } diff --git a/subsys/bluetooth/audio/tbs_internal.h b/subsys/bluetooth/audio/tbs_internal.h index 9b15195c355..68b4dbb0e70 100644 --- a/subsys/bluetooth/audio/tbs_internal.h +++ b/subsys/bluetooth/audio/tbs_internal.h @@ -18,6 +18,7 @@ #include #include #include +#include #include #define BT_TBS_MAX_UCI_SIZE 6 @@ -312,6 +313,12 @@ struct bt_tbs_in_uri { * sizeof(struct bt_tbs_client_call_state))) #endif /* defined(CONFIG_BT_TBS_CLIENT_BEARER_LIST_CURRENT_CALLS) */ +enum bt_tbs_client_flag { + BT_TBS_CLIENT_FLAG_BUSY, + + BT_TBS_CLIENT_FLAG_NUM_FLAGS, /* keep as last */ +}; + struct bt_tbs_instance { struct bt_tbs_client_call_state calls[CONFIG_BT_TBS_CLIENT_MAX_CALLS]; @@ -336,7 +343,6 @@ struct bt_tbs_instance { #endif /* defined(CONFIG_BT_TBS_CLIENT_OPTIONAL_OPCODES) */ uint16_t termination_reason_handle; - bool busy; #if defined(CONFIG_BT_TBS_CLIENT_CCID) uint8_t ccid; #endif /* defined(CONFIG_BT_TBS_CLIENT_CCID) */ @@ -384,5 +390,7 @@ struct bt_tbs_instance { struct bt_gatt_read_params read_params; uint8_t read_buf[BT_TBS_CLIENT_INST_READ_BUF_SIZE]; struct net_buf_simple net_buf; + + ATOMIC_DEFINE(flags, BT_TBS_CLIENT_FLAG_NUM_FLAGS); }; #endif /* CONFIG_BT_TBS_CLIENT */