diff --git a/include/zephyr/bluetooth/conn.h b/include/zephyr/bluetooth/conn.h index 6a4b7c238c898..2f9b796e1ec14 100644 --- a/include/zephyr/bluetooth/conn.h +++ b/include/zephyr/bluetooth/conn.h @@ -1877,9 +1877,8 @@ struct bt_conn_cb { * start either a connectable advertiser or create a new connection * this might fail because there are no free connection objects * available. - * To avoid this issue it is recommended to either start connectable - * advertise or create a new connection using @ref k_work_submit or - * increase @kconfig{CONFIG_BT_MAX_CONN}. + * To avoid this issue, it's recommended to rely instead on @ref bt_conn_cb.recycled + * which notifies the application when a connection object has actually been freed. * * @param conn Connection object. * @param reason BT_HCI_ERR_* reason for the disconnection. @@ -1891,12 +1890,13 @@ struct bt_conn_cb { * This callback notifies the application that it might be able to * allocate a connection object. No guarantee, first come, first serve. * - * Use this to e.g. re-start connectable advertising or scanning. + * The maximum number of simultaneous connections is configured + * by @kconfig{CONFIG_BT_MAX_CONN}. * - * Treat this callback as an ISR, as it originates from - * @ref bt_conn_unref which is used by the BT stack. Making - * Bluetooth API calls in this context is error-prone and strongly - * discouraged. + * This is the event to listen for to start a new connection or connectable advertiser, + * both when the intention is to start it after the system is completely + * finished with an earlier connection, and when the application wants to start + * a connection for any reason but failed and is waiting for the right time to retry. */ void (*recycled)(void); diff --git a/subsys/bluetooth/host/conn.c b/subsys/bluetooth/host/conn.c index f3abf67d4fb6d..c52ecc42fc1c6 100644 --- a/subsys/bluetooth/host/conn.c +++ b/subsys/bluetooth/host/conn.c @@ -1500,38 +1500,57 @@ void bt_conn_unref(struct bt_conn *conn) enum bt_conn_type conn_type; uint8_t conn_role; uint16_t conn_handle; + /* Used only if CONFIG_ASSERT and CONFIG_BT_CONN_TX. */ + __maybe_unused bool conn_tx_is_pending; __ASSERT(conn, "Invalid connection reference"); - /* Storing parameters of interest so we don't access the object - * after decrementing its ref-count + /* If we're removing the last reference, the connection object will be + * considered freed and possibly re-allocated by the Bluetooth Host stack + * as soon as we decrement the counter. + * To prevent inconsistencies when accessing the connection's state, + * we store its properties of interest before decrementing the ref-count, + * then unset the local pointer. */ conn_type = conn->type; conn_role = conn->role; conn_handle = conn->handle; - +#if CONFIG_BT_CONN_TX && CONFIG_ASSERT + conn_tx_is_pending = k_work_is_pending(&conn->tx_complete_work); +#endif old = atomic_dec(&conn->ref); - /* Prevent from accessing connection object */ - deallocated = (atomic_get(&old) == 1); - IF_ENABLED(CONFIG_BT_CONN_TX, - (__ASSERT(!(deallocated && k_work_is_pending(&conn->tx_complete_work)), - "tx_complete_work is pending when conn is deallocated"))); conn = NULL; LOG_DBG("handle %u ref %ld -> %ld", conn_handle, old, (old - 1)); __ASSERT(old > 0, "Conn reference counter is 0"); - /* Slot has been freed and can be taken. No guarantees are made on requests - * to claim connection object as only the first claim will be served. - */ - if (deallocated) { - k_sem_give(&pending_recycled_events); - k_work_submit(&recycled_work); + /* Whether we removed the last reference. */ + deallocated = (old == 1); + if (!deallocated) { + return; } + IF_ENABLED(CONFIG_BT_CONN_TX, + (__ASSERT(!conn_tx_is_pending, + "tx_complete_work is pending when conn is deallocated");)) + + /* Notify listeners that a slot has been freed and can be taken. + * No guarantees are made on requests to claim connection object + * as only the first claim will be served. + */ + k_sem_give(&pending_recycled_events); + k_work_submit(&recycled_work); + + /* Use the freed slot to automatically resume LE peripheral advertising. + * + * This behavior is deprecated: + * - 8cfad44: Bluetooth: Deprecate adv auto-resume + * - #72567: Bluetooth: Advertising resume functionality is broken + * - Migration guide to Zephyr v4.0.0, Automatic advertiser resumption is deprecated + */ if (IS_ENABLED(CONFIG_BT_PERIPHERAL) && conn_type == BT_CONN_TYPE_LE && - conn_role == BT_CONN_ROLE_PERIPHERAL && deallocated) { + conn_role == BT_CONN_ROLE_PERIPHERAL) { bt_le_adv_resume(); } } @@ -2272,6 +2291,10 @@ static void deferred_work(struct k_work *work) err); } #endif + } else { + /* Neither the application nor the configuration + * set LE connection parameters. + */ } atomic_set_bit(conn->flags, BT_CONN_PERIPHERAL_PARAM_UPDATE); @@ -2374,6 +2397,8 @@ struct bt_conn *bt_conn_add_sco(const bt_addr_t *peer, int link_type) } else if (link_type == BT_HCI_ESCO) { sco_conn->sco.pkt_type = (bt_dev.br.esco_pkt_type & ~EDR_ESCO_PKT_MASK); + } else { + /* Ignoring unexpected link type BT_HCI_ACL. */ } return sco_conn;