From 20757b79335e139b35f2472bb203a0c17eefd97c Mon Sep 17 00:00:00 2001 From: liqigan Date: Thu, 11 Jun 2026 19:55:15 +0800 Subject: [PATCH 1/7] change(bt/bluedroid): Refactored HID host datapath --- .../bt/host/bluedroid/bta/hh/bta_hh_act.c | 72 ++- .../bt/host/bluedroid/bta/hh/bta_hh_le.c | 17 +- .../bluedroid/bta/hh/include/bta_hh_int.h | 6 +- .../bluedroid/bta/include/bta/bta_hh_co.h | 42 +- .../bluedroid/btc/profile/std/hid/bta_hh_co.c | 225 ++++++++-- .../bluedroid/btc/profile/std/hid/btc_hh.c | 416 ++++++++++++++++-- .../btc/profile/std/include/btc_hh.h | 31 +- 7 files changed, 672 insertions(+), 137 deletions(-) diff --git a/components/bt/host/bluedroid/bta/hh/bta_hh_act.c b/components/bt/host/bluedroid/bta/hh/bta_hh_act.c index 8a7725fa300..efe6a99ebd9 100644 --- a/components/bt/host/bluedroid/bta/hh/bta_hh_act.c +++ b/components/bt/host/bluedroid/bta/hh/bta_hh_act.c @@ -40,7 +40,6 @@ ** Constants *****************************************************************************/ - /***************************************************************************** ** Local Function prototypes *****************************************************************************/ @@ -96,6 +95,7 @@ void bta_hh_api_enable(tBTA_HH_DATA *p_data) for (xx = 0; xx < BTA_HH_MAX_KNOWN; xx ++) { bta_hh_cb.cb_index[xx] = BTA_HH_IDX_INVALID; } + } #if (BTA_HH_LE_INCLUDED == TRUE) @@ -538,8 +538,23 @@ void bta_hh_open_cmpl_act(tBTA_HH_DEV_CB *p_cb, tBTA_HH_DATA *p_data) bta_hh_cb.cnt_num ++; /* initialize device driver */ - bta_hh_co_open(dev_handle, p_cb->sub_class, - p_cb->attr_mask, p_cb->app_id); + if (!bta_hh_co_open(dev_handle, p_cb->sub_class, + p_cb->attr_mask, p_cb->app_id)) { + conn.status = BTA_HH_ERR_NO_RES; + p_cb->opened = FALSE; +#if (BTA_HH_LE_INCLUDED == TRUE) + if (!p_cb->is_le_device) +#endif + { + /* Balance the async close path which unconditionally calls bta_sys_conn_close(). */ + bta_sys_conn_open(BTA_ID_HH, p_cb->app_id, p_cb->addr); + } + HID_HostCloseDev(dev_handle); + (* bta_hh_cb.p_cback)(BTA_HH_OPEN_EVT, (tBTA_HH *)&conn); + p_cb->incoming_conn = FALSE; + p_cb->incoming_hid_handle = BTA_HH_INVALID_HANDLE; + return; + } #if (BTA_HH_LE_INCLUDED == TRUE) conn.status = p_cb->status; @@ -552,6 +567,7 @@ void bta_hh_open_cmpl_act(tBTA_HH_DEV_CB *p_cb, tBTA_HH_DATA *p_data) /* inform role manager */ bta_sys_conn_open( BTA_ID_HH , p_cb->app_id, p_cb->addr); } + p_cb->opened = TRUE; /* set protocol mode when not default report mode */ if ( p_cb->mode != BTA_HH_PROTO_RPT_MODE #if (BTA_HH_LE_INCLUDED == TRUE) @@ -631,12 +647,15 @@ void bta_hh_open_act(tBTA_HH_DEV_CB *p_cb, tBTA_HH_DATA *p_data) void bta_hh_data_act(tBTA_HH_DEV_CB *p_cb, tBTA_HH_DATA *p_data) { BT_HDR *pdata = p_data->hid_cback.p_data; - UINT8 *p_rpt = (UINT8 *)(pdata + 1) + pdata->offset; - bta_hh_co_data((UINT8)p_data->hid_cback.hdr.layer_specific, p_rpt, pdata->len, - p_cb->mode, p_cb->sub_class, p_cb->dscp_info.ctry_code, p_cb->addr, p_cb->app_id); + if (pdata == NULL) { + return; + } - utl_freebuf((void **)&pdata); + bta_hh_co_data_hdr((UINT8)p_data->hid_cback.hdr.layer_specific, pdata, + p_cb->mode, p_cb->sub_class, p_cb->dscp_info.ctry_code, + p_cb->addr, p_cb->app_id); + p_data->hid_cback.p_data = NULL; } @@ -834,6 +853,7 @@ void bta_hh_open_failure(tBTA_HH_DEV_CB *p_cb, tBTA_HH_DATA *p_data) /* Report OPEN fail event */ (*bta_hh_cb.p_cback)(BTA_HH_OPEN_EVT, (tBTA_HH *)&conn_dat); + p_cb->opened = FALSE; #if BTA_HH_DEBUG bta_hh_trace_dev_db(); @@ -885,6 +905,7 @@ void bta_hh_close_act (tBTA_HH_DEV_CB *p_cb, tBTA_HH_DATA *p_data) /* Report OPEN fail event */ (*bta_hh_cb.p_cback)(BTA_HH_OPEN_EVT, (tBTA_HH *)&conn_dat); + p_cb->opened = FALSE; #if BTA_HH_DEBUG bta_hh_trace_dev_db(); @@ -893,24 +914,27 @@ void bta_hh_close_act (tBTA_HH_DEV_CB *p_cb, tBTA_HH_DATA *p_data) } /* otherwise report CLOSE/VC_UNPLUG event */ else { - /* finaliza device driver */ - bta_hh_co_close(p_cb->hid_handle, p_cb->app_id); /* inform role manager */ bta_sys_conn_close( BTA_ID_HH , p_cb->app_id, p_cb->addr); /* update total conn number */ bta_hh_cb.cnt_num --; - if (disc_dat.status) { - disc_dat.status = BTA_HH_ERR; - } + if (p_cb->opened) { + /* finalize device driver only for successfully opened devices */ + bta_hh_co_close(p_cb->hid_handle, p_cb->app_id); + if (disc_dat.status) { + disc_dat.status = BTA_HH_ERR; + } - (*bta_hh_cb.p_cback)(event, (tBTA_HH *)&disc_dat); + (*bta_hh_cb.p_cback)(event, (tBTA_HH *)&disc_dat); - /* if virtually unplug, remove device */ - if (p_cb->vp ) { - HID_HostRemoveDev( p_cb->hid_handle); - bta_hh_clean_up_kdev(p_cb); + /* if virtually unplug, remove device */ + if (p_cb->vp ) { + HID_HostRemoveDev( p_cb->hid_handle); + bta_hh_clean_up_kdev(p_cb); + } } + p_cb->opened = FALSE; #if BTA_HH_DEBUG bta_hh_trace_dev_db(); @@ -1190,9 +1214,17 @@ static void bta_hh_cback (UINT8 dev_handle, BD_ADDR addr, UINT8 event, case HID_HDEV_EVT_CLOSE: sm_event = BTA_HH_INT_CLOSE_EVT; break; - case HID_HDEV_EVT_INTR_DATA: - sm_event = BTA_HH_INT_DATA_EVT; - break; + case HID_HDEV_EVT_INTR_DATA: { + UINT8 index = bta_hh_dev_handle_to_cb_idx(dev_handle); + tBTA_HH_DEV_CB *p_cb = (index != BTA_HH_IDX_INVALID) ? &bta_hh_cb.kdev[index] : NULL; + if (p_cb != NULL && p_cb->state == BTA_HH_CONN_ST) { + bta_hh_co_data_hdr(dev_handle, pdata, p_cb->mode, p_cb->sub_class, p_cb->dscp_info.ctry_code, p_cb->addr, + p_cb->app_id); + } else { + utl_freebuf((void **)&pdata); + } + return; + } case HID_HDEV_EVT_HANDSHAKE: sm_event = BTA_HH_INT_HANDSK_EVT; break; diff --git a/components/bt/host/bluedroid/bta/hh/bta_hh_le.c b/components/bt/host/bluedroid/bta/hh/bta_hh_le.c index 9bb7801a412..69e57cf3189 100644 --- a/components/bt/host/bluedroid/bta/hh/bta_hh_le.c +++ b/components/bt/host/bluedroid/bta/hh/bta_hh_le.c @@ -2266,17 +2266,12 @@ void bta_hh_le_input_rpt_notify(tBTA_GATTC_NOTIFY *p_data) p_buf = p_data->value; } - bta_hh_co_data((UINT8)p_dev_cb->hid_handle, - p_buf, - p_data->len, - p_dev_cb->mode, - 0 , /* no sub class*/ - p_dev_cb->dscp_info.ctry_code, - p_dev_cb->addr, - app_id); - - if (p_buf != p_data->value) { - osi_free(p_buf); + if (p_buf == p_data->value) { + bta_hh_le_co_data((UINT8)p_dev_cb->hid_handle, p_buf, p_data->len, p_dev_cb->mode, 0, /* no sub class*/ + p_dev_cb->dscp_info.ctry_code, p_dev_cb->addr, app_id); + } else { + bta_hh_le_co_data_owned((UINT8)p_dev_cb->hid_handle, p_buf, p_data->len, p_dev_cb->mode, 0, /* no sub class*/ + p_dev_cb->dscp_info.ctry_code, p_dev_cb->addr, app_id); } } diff --git a/components/bt/host/bluedroid/bta/hh/include/bta_hh_int.h b/components/bt/host/bluedroid/bta/hh/include/bta_hh_int.h index 0efb2fcab49..56f99e8420d 100644 --- a/components/bt/host/bluedroid/bta/hh/include/bta_hh_int.h +++ b/components/bt/host/bluedroid/bta/hh/include/bta_hh_int.h @@ -28,6 +28,8 @@ #include "bta/bta_sys.h" #include "bta/utl.h" #include "bta/bta_hh_api.h" +#include "osi/pkt_queue.h" +#include "osi/thread.h" //#if BTA_HH_LE_INCLUDED == TRUE #include "bta/bta_gatt_api.h" @@ -82,7 +84,7 @@ typedef UINT16 tBTA_HH_INT_EVT; /* HID host internal events */ #define BTA_HH_FST_TRANS_CB_EVT BTA_HH_GET_RPT_EVT #define BTA_HH_FST_BTE_TRANS_EVT HID_TRANS_GET_REPORT -/* sub event code used for device maintainence API call */ +/* sub event code used for device maintenance API call */ #define BTA_HH_ADD_DEV 0 #define BTA_HH_REMOVE_DEV 1 @@ -201,7 +203,7 @@ typedef struct { #define BTA_HH_LE_PROTO_MODE_BIT 0x01 #define BTA_HH_LE_CP_BIT 0x02 - UINT8 option_char; /* control point char exisit or not */ + UINT8 option_char; /* control point char exist or not */ BOOLEAN expl_incl_srvc; UINT8 incl_srvc_inst; /* assuming only one included service : battery service */ diff --git a/components/bt/host/bluedroid/bta/include/bta/bta_hh_co.h b/components/bt/host/bluedroid/bta/include/bta/bta_hh_co.h index f0fef370c83..05343c2a97c 100644 --- a/components/bt/host/bluedroid/bta/include/bta/bta_hh_co.h +++ b/components/bt/host/bluedroid/bta/include/bta/bta_hh_co.h @@ -25,6 +25,10 @@ #define BTA_HH_CO_H #include "bta/bta_hh_api.h" +#include "osi/pkt_queue.h" +#include "stack/bt_types.h" + +#if defined(BTA_HH_INCLUDED) && (BTA_HH_INCLUDED == TRUE) typedef struct { UINT16 rpt_uuid; @@ -34,21 +38,48 @@ typedef struct { UINT8 prop; } tBTA_HH_RPT_CACHE_ENTRY; +typedef enum { + BTA_HH_DATA_BUF_RAW = 0, + BTA_HH_DATA_BUF_BT_HDR, +} tBTA_HH_DATA_BUF_TYPE; + +typedef struct { + UINT8 dev_handle; + UINT8 buf_type; // see tBTA_HH_DATA_BUF_TYPE + tBTA_HH_PROTO_MODE proto_mode; + UINT16 len; + void *p_buf; +} tBTA_HH_DATA_PKT; + +extern void bta_hh_co_data_pkt_free(tBTA_HH_DATA_PKT *pkt); +extern void bta_hh_co_data_linked_pkt_free(pkt_linked_item_t *item); +extern UINT8 *bta_hh_co_data_pkt_get_payload(tBTA_HH_DATA_PKT *pkt); + /******************************************************************************* ** ** Function bta_hh_co_data ** ** Description This callout function is executed by HH when data is received -** in interupt channel. +** in interrupt channel. ** ** ** Returns void. ** *******************************************************************************/ -extern void bta_hh_co_data(UINT8 dev_handle, UINT8 *p_rpt, UINT16 len, + +extern void bta_hh_co_data_hdr(UINT8 dev_handle, BT_HDR *p_hdr, + tBTA_HH_PROTO_MODE mode, UINT8 sub_class, + UINT8 ctry_code, BD_ADDR peer_addr, UINT8 app_id); +#if (BLE_INCLUDED == TRUE && BTA_HH_LE_INCLUDED == TRUE) +extern void bta_hh_le_co_data(UINT8 dev_handle, UINT8 *p_rpt, UINT16 len, tBTA_HH_PROTO_MODE mode, UINT8 sub_class, UINT8 ctry_code, BD_ADDR peer_addr, UINT8 app_id); +extern void bta_hh_le_co_data_owned(UINT8 dev_handle, UINT8 *p_buf, UINT16 len, + tBTA_HH_PROTO_MODE mode, UINT8 sub_class, + UINT8 ctry_code, BD_ADDR peer_addr, UINT8 app_id); +#endif /* (BLE_INCLUDED == TRUE && BTA_HH_LE_INCLUDED == TRUE) */ + /******************************************************************************* ** ** Function bta_hh_co_open @@ -57,11 +88,11 @@ extern void bta_hh_co_data(UINT8 dev_handle, UINT8 *p_rpt, UINT16 len, ** opened, and application may do some device specific ** initialization. ** -** Returns void. +** Returns TRUE if platform specific initialization succeeds. ** *******************************************************************************/ -extern void bta_hh_co_open(UINT8 dev_handle, UINT8 sub_class, - UINT16 attr_mask, UINT8 app_id); +extern BOOLEAN bta_hh_co_open(UINT8 dev_handle, UINT8 sub_class, + UINT16 attr_mask, UINT8 app_id); /******************************************************************************* ** @@ -129,4 +160,5 @@ extern tBTA_HH_RPT_CACHE_ENTRY *bta_hh_le_co_cache_load (BD_ADDR remote_bda, extern void bta_hh_le_co_reset_rpt_cache (BD_ADDR remote_bda, UINT8 app_id); #endif /* #if (BLE_INCLUDED == TRUE && BTA_HH_LE_INCLUDED == TRUE) */ +#endif /* defined(BTA_HH_INCLUDED) && (BTA_HH_INCLUDED == TRUE) */ #endif /* BTA_HH_CO_H */ diff --git a/components/bt/host/bluedroid/btc/profile/std/hid/bta_hh_co.c b/components/bt/host/bluedroid/btc/profile/std/hid/bta_hh_co.c index 63daecafa64..0885a26b447 100644 --- a/components/bt/host/bluedroid/btc/profile/std/hid/bta_hh_co.c +++ b/components/bt/host/bluedroid/btc/profile/std/hid/bta_hh_co.c @@ -1,8 +1,49 @@ #include #include "btc_hh.h" +#include "bta/utl.h" #include "osi/allocator.h" #if HID_HOST_INCLUDED == TRUE +void bta_hh_co_data_pkt_free(tBTA_HH_DATA_PKT *pkt) +{ + if (pkt == NULL || pkt->p_buf == NULL) { + return; + } + + if (pkt->buf_type == BTA_HH_DATA_BUF_BT_HDR) { + utl_freebuf((void **)&pkt->p_buf); + } else { + osi_free(pkt->p_buf); + } + pkt->p_buf = NULL; +} + +void bta_hh_co_data_linked_pkt_free(pkt_linked_item_t *item) +{ + if (item == NULL) { + return; + } + + bta_hh_co_data_pkt_free((tBTA_HH_DATA_PKT *)item->data); + osi_free(item); +} + +UINT8 *bta_hh_co_data_pkt_get_payload(tBTA_HH_DATA_PKT *pkt) +{ + BT_HDR *hdr; + + if (pkt == NULL || pkt->p_buf == NULL || pkt->len == 0) { + return NULL; + } + + if (pkt->buf_type == BTA_HH_DATA_BUF_BT_HDR) { + hdr = (BT_HDR *)pkt->p_buf; + return hdr->data + hdr->offset; + } + + return (UINT8 *)pkt->p_buf; +} + /******************************************************************************* * * Function bta_hh_co_open @@ -10,16 +51,18 @@ * Description When connection is opened, this call-out function is executed * by HH to do platform specific initialization. * - * Returns void. + * Returns TRUE if platform specific initialization succeeds. ******************************************************************************/ -void bta_hh_co_open(UINT8 dev_handle, UINT8 sub_class, tBTA_HH_ATTR_MASK attr_mask, UINT8 app_id) +BOOLEAN bta_hh_co_open(UINT8 dev_handle, UINT8 sub_class, tBTA_HH_ATTR_MASK attr_mask, UINT8 app_id) { + BOOLEAN is_new_device = TRUE; + uint8_t old_dev_status = ESP_HIDH_CONN_STATE_UNKNOWN; UINT32 i; btc_hh_device_t *p_dev = NULL; if (dev_handle == BTA_HH_INVALID_HANDLE) { APPL_TRACE_WARNING("%s: Oops, dev_handle (%d) is invalid...", __func__, dev_handle); - return; + return FALSE; } for (i = 0; i < BTC_HH_MAX_HID; i++) { @@ -30,6 +73,8 @@ void bta_hh_co_open(UINT8 dev_handle, UINT8 sub_class, tBTA_HH_ATTR_MASK attr_ma "dev_handle=0x%2x, attr_mask=0x%04x, sub_class=0x%02x, app_id=%d", __func__, p_dev->dev_status, dev_handle, p_dev->attr_mask, p_dev->sub_class, p_dev->app_id); + is_new_device = FALSE; + old_dev_status = p_dev->dev_status; break; } p_dev = NULL; @@ -54,11 +99,26 @@ void bta_hh_co_open(UINT8 dev_handle, UINT8 sub_class, tBTA_HH_ATTR_MASK attr_ma if (p_dev == NULL) { APPL_TRACE_ERROR("%s: Error: too many HID devices are connected", __func__); - return; + return FALSE; } p_dev->dev_status = ESP_HIDH_CONN_STATE_CONNECTED; + if (!btc_hh_data_path_init(dev_handle)) { + APPL_TRACE_ERROR("%s: failed to init HID host datapath, dev_handle=%u", __func__, dev_handle); + if (is_new_device) { + p_dev->dev_status = ESP_HIDH_CONN_STATE_UNKNOWN; + p_dev->dev_handle = BTA_HH_INVALID_HANDLE; + if (btc_hh_cb.device_num) { + btc_hh_cb.device_num--; + } + } else { + p_dev->dev_status = old_dev_status; + } + return FALSE; + } + APPL_TRACE_DEBUG("%s: Return device status %d", __func__, p_dev->dev_status); + return TRUE; } /******************************************************************************* @@ -93,64 +153,133 @@ void bta_hh_co_close(UINT8 dev_handle, UINT8 app_id) break; } } + + // data path will be cleaned up on CLOSE or UNPLUG event in btc context } /******************************************************************************* * - * Function bta_hh_co_data + * Function bta_hh_co_data_hdr * - * Description This function is executed by BTA when HID host receive a - * data report on interrupt channel. - * - * Parameters dev_handle - device handle - * *p_rpt - pointer to the report data - * len - length of report data - * mode - Hid host Protocol Mode - * sub_clas - Device Subclass - * app_id - application id + * Description Transfer BT_HDR ownership to BTC queue (BR/EDR zero-copy). * * Returns void ******************************************************************************/ -void bta_hh_co_data(UINT8 dev_handle, UINT8 *p_rpt, UINT16 len, tBTA_HH_PROTO_MODE mode, UINT8 sub_class, UINT8 ctry_code, - BD_ADDR peer_addr, UINT8 app_id) +void bta_hh_co_data_hdr(UINT8 dev_handle, BT_HDR *p_hdr, tBTA_HH_PROTO_MODE mode, UINT8 sub_class, UINT8 ctry_code, + BD_ADDR peer_addr, UINT8 app_id) { - btc_msg_t msg; - tBTA_HH p_data; - BT_HDR *p_buf = NULL; - bt_status_t status; - tBTA_HH_STATUS ret = BTA_HH_OK; - - msg.sig = BTC_SIG_API_CB; - msg.pid = BTC_PID_HH; - msg.act = BTA_HH_DATA_IND_EVT; + pkt_linked_item_t *linked_pkt = NULL; + tBTA_HH_DATA_PKT *pkt; APPL_TRACE_DEBUG("%s: dev_handle = %d, subclass = 0x%02X, mode = %d, " "ctry_code = %d, app_id = %d", __func__, dev_handle, sub_class, mode, ctry_code, app_id); - do { - if ((p_rpt == NULL) || (len == 0)) { - ret = BTA_HH_ERR; - break; - } - if ((p_buf = osi_malloc(sizeof(BT_HDR) + len)) == NULL) { - APPL_TRACE_ERROR("%s malloc failed!", __func__); - ret = BTA_HH_ERR_NO_RES; - break; - } - p_buf->offset = 0; - p_buf->len = len; - p_buf->event = 0; - p_buf->layer_specific = dev_handle; - memcpy(p_buf->data, p_rpt, len); - } while (0); + if (p_hdr == NULL || p_hdr->len == 0) { + utl_freebuf((void **)&p_hdr); + return; + } - p_data.int_data.status = ret; - p_data.int_data.handle = dev_handle; - p_data.int_data.p_data = p_buf; - p_data.int_data.proto_mode = mode; - status = btc_transfer_context(&msg, &p_data, sizeof(tBTA_HH), NULL, NULL); - assert(status == BT_STATUS_SUCCESS); + linked_pkt = btc_hh_data_dequeue_reusable_linked_pkt(dev_handle); + if (linked_pkt != NULL) { + bta_hh_co_data_pkt_free((tBTA_HH_DATA_PKT *)linked_pkt->data); + } else { + linked_pkt = (pkt_linked_item_t *)osi_malloc(BT_PKT_LINKED_HDR_SIZE + + sizeof(tBTA_HH_DATA_PKT)); + if (linked_pkt == NULL) { + btc_hh_on_pkt_dropped(dev_handle); + utl_freebuf((void **)&p_hdr); + return; + } + } + + pkt = (tBTA_HH_DATA_PKT *)linked_pkt->data; + pkt->dev_handle = dev_handle; + pkt->len = p_hdr->len; + pkt->proto_mode = mode; + pkt->buf_type = BTA_HH_DATA_BUF_BT_HDR; + pkt->p_buf = p_hdr; + + if (!btc_hh_data_enqueue_linked_pkt(linked_pkt)) { + btc_hh_on_pkt_dropped(dev_handle); + bta_hh_co_data_linked_pkt_free(linked_pkt); + } +} +#endif /* HID_HOST_INCLUDED == TRUE */ + +#if (BLE_INCLUDED == TRUE && BTA_HH_LE_INCLUDED == TRUE) +/******************************************************************************* + * + * Function bta_hh_le_co_data_owned + * + * Description Transfer raw buffer ownership to BTC queue (BLE zero-copy). + * + * Returns void + ******************************************************************************/ +void bta_hh_le_co_data_owned(UINT8 dev_handle, UINT8 *p_buf, UINT16 len, tBTA_HH_PROTO_MODE mode, UINT8 sub_class, + UINT8 ctry_code, BD_ADDR peer_addr, UINT8 app_id) +{ + tBTA_HH_DATA_PKT pkt = { + .dev_handle = dev_handle, + .len = len, + .proto_mode = mode, + .buf_type = BTA_HH_DATA_BUF_RAW, + .p_buf = p_buf, + }; + + APPL_TRACE_DEBUG("%s: dev_handle = %d, subclass = 0x%02X, mode = %d, " + "ctry_code = %d, app_id = %d", + __func__, dev_handle, sub_class, mode, ctry_code, app_id); + + if ((p_buf == NULL) || (len == 0)) { + osi_free(p_buf); + return; + } + + if (!btc_hh_data_enqueue_pkt(&pkt)) { + btc_hh_on_pkt_dropped(dev_handle); + osi_free(p_buf); + } } -#endif /* HID_HOST_INCLUDED == TRUE */ +/******************************************************************************* + * + * Function bta_hh_le_co_data + * + * Description Copy ephemeral report data into BTC queue (BLE GATT notify). + * + * Returns void + ******************************************************************************/ +void bta_hh_le_co_data(UINT8 dev_handle, UINT8 *p_rpt, UINT16 len, tBTA_HH_PROTO_MODE mode, UINT8 sub_class, + UINT8 ctry_code, BD_ADDR peer_addr, UINT8 app_id) +{ + UINT8 *p_buf; + tBTA_HH_DATA_PKT pkt; + + APPL_TRACE_DEBUG("%s: dev_handle = %d, subclass = 0x%02X, mode = %d, " + "ctry_code = %d, app_id = %d", + __func__, dev_handle, sub_class, mode, ctry_code, app_id); + + if ((p_rpt == NULL) || (len == 0)) { + return; + } + + p_buf = (UINT8 *)osi_malloc(len); + if (p_buf == NULL) { + btc_hh_on_pkt_dropped(dev_handle); + return; + } + memcpy(p_buf, p_rpt, len); + + pkt.dev_handle = dev_handle; + pkt.len = len; + pkt.proto_mode = mode; + pkt.buf_type = BTA_HH_DATA_BUF_RAW; + pkt.p_buf = p_buf; + + if (!btc_hh_data_enqueue_pkt(&pkt)) { + btc_hh_on_pkt_dropped(dev_handle); + osi_free(p_buf); + } +} +#endif /* (BLE_INCLUDED == TRUE && BTA_HH_LE_INCLUDED == TRUE) */ diff --git a/components/bt/host/bluedroid/btc/profile/std/hid/btc_hh.c b/components/bt/host/bluedroid/btc/profile/std/hid/btc_hh.c index cda6db3dff2..e73594210ae 100644 --- a/components/bt/host/bluedroid/btc/profile/std/hid/btc_hh.c +++ b/components/bt/host/bluedroid/btc/profile/std/hid/btc_hh.c @@ -34,9 +34,13 @@ #include "device/bdaddr.h" #include "btc/btc_storage.h" #include "osi/allocator.h" +#include "osi/pkt_queue.h" +#include "stack/bt_types.h" +#include "stack/gatt_api.h" #include "bta/utl.h" #include "bta/bta_hh_api.h" #include "stack/l2c_api.h" +#include "esp_log.h" // #include "bta_dm_int.h" #if HID_HOST_INCLUDED == TRUE @@ -64,7 +68,25 @@ static bdstr_t bdstr; #define is_hidh_init() (btc_hh_cb.status > BTC_HH_DISABLED) #define BTC_TIMEOUT_VUP_MS (3 * 1000) +#define BTC_HH_DATA_QUEUE_IDX (1) +/** + * Low-latency profile for gamepad-like traffic: + * keep queue short to avoid stale input backlog and keep batch moderate. + */ +#define BTC_HH_DATA_QUEUE_LEN_MAX (20) // for high report rate, the queue will cause 1.25ms * BTC_HH_DATA_QUEUE_LEN_MAX latency +#define BTC_HH_DATA_BATCH_SIZE_BASE (8) +#define BTC_HH_DATA_BATCH_SIZE_HIGH (10) +#define BTC_HH_DATA_BATCH_SIZE_MAX (12) + static void btc_hh_cb_arg_deep_free(btc_msg_t *msg); +static void btc_hh_data_pkt_handler(void *arg); +static size_t btc_hh_get_batch_size(size_t queue_len); +static bool btc_hh_data_path_init_inner(btc_hh_device_t *p_dev); +static void btc_hh_data_path_deinit_inner(btc_hh_device_t *p_dev); +static bool btc_hh_device_drop_cnt_init(btc_hh_device_t *p_dev); +static void btc_hh_device_drop_cnt_deinit(btc_hh_device_t *p_dev); +static void btc_hh_device_drop_cnt_set(btc_hh_device_t *p_dev, uint32_t cnt); +static void btc_hh_device_drop_cnt_inc(btc_hh_device_t *p_dev); static inline void btc_hh_cb_to_app(esp_hidh_cb_event_t event, esp_hidh_cb_param_t *param) { @@ -146,6 +168,68 @@ btc_hh_device_t *btc_hh_find_connected_dev_by_handle(uint8_t handle) return NULL; } +static bool btc_hh_device_drop_cnt_init(btc_hh_device_t *p_dev) +{ + if (p_dev == NULL) { + return false; + } + + return (osi_mutex_new(&p_dev->lock) == 0); +} + +static void btc_hh_device_drop_cnt_deinit(btc_hh_device_t *p_dev) +{ + if (p_dev == NULL) { + return; + } + + osi_mutex_free(&p_dev->lock); +} + +static void btc_hh_device_drop_cnt_set(btc_hh_device_t *p_dev, uint32_t cnt) +{ + if (p_dev == NULL) { + return; + } + + osi_mutex_lock(&p_dev->lock, OSI_MUTEX_MAX_TIMEOUT); + p_dev->drop_pkt_cnt = cnt; + osi_mutex_unlock(&p_dev->lock); +} + +static void btc_hh_device_drop_cnt_inc(btc_hh_device_t *p_dev) +{ + if (p_dev == NULL) { + return; + } + + osi_mutex_lock(&p_dev->lock, OSI_MUTEX_MAX_TIMEOUT); + p_dev->drop_pkt_cnt++; + osi_mutex_unlock(&p_dev->lock); +} + +void btc_hh_on_pkt_dropped(uint8_t dev_handle) +{ + btc_hh_device_t *p_dev = btc_hh_find_connected_dev_by_handle(dev_handle); + + if (p_dev == NULL) { + return; + } + + btc_hh_device_drop_cnt_inc(p_dev); +} + +void btc_hh_reset_drop_pkt_cnt(uint8_t dev_handle) +{ + btc_hh_device_t *p_dev = btc_hh_find_connected_dev_by_handle(dev_handle); + + if (p_dev == NULL) { + return; + } + + btc_hh_device_drop_cnt_set(p_dev, 0); +} + /******************************************************************************* * * Function btc_hh_find_dev_by_bda @@ -337,9 +421,10 @@ void btc_hh_remove_device(BD_ADDR bd_addr) * with up-layer */ //[boblane] // HAL_CBACK(bt_hh_callbacks, connection_state_cb, &(p_dev->bd_addr), BTHH_CONN_STATE_DISCONNECTED); + btc_hh_device_drop_cnt_set(p_dev, 0); p_dev->dev_status = ESP_HIDH_CONN_STATE_UNKNOWN; p_dev->dev_handle = BTA_HH_INVALID_HANDLE; - p_dev->ready_for_data = false; + btc_hh_data_path_deinit_inner(p_dev); if (btc_hh_cb.device_num > 0) { btc_hh_cb.device_num--; @@ -367,23 +452,6 @@ static void bte_hh_arg_deep_copy(btc_msg_t *msg, void *p_dst, void *p_src) } break; } - case BTA_HH_DATA_IND_EVT: { - BT_HDR *src_hdr = p_src_data->int_data.p_data; - p_dst_data->int_data.p_data = NULL; - if (src_hdr) { - p_dst_data->int_data.p_data = osi_malloc(sizeof(BT_HDR) + src_hdr->len); - if (p_dst_data->int_data.p_data == NULL) { - BTC_TRACE_ERROR("%s malloc int_data.p_data failed!", __func__); - p_dst_data->int_data.status = ESP_HIDH_ERR_NO_RES; - break; - } - BT_HDR *dst_hdr = p_dst_data->int_data.p_data; - memcpy(dst_hdr, src_hdr, sizeof(BT_HDR)); - memcpy(dst_hdr->data, src_hdr->data + src_hdr->offset, src_hdr->len); - dst_hdr->offset = 0; - } - break; - } default: break; } @@ -442,9 +510,6 @@ static void bte_hh_evt(tBTA_HH_EVT event, tBTA_HH *p_data) case BTA_HH_DATA_EVT: param_len = sizeof(tBTA_HH_API_SENDDATA); break; - case BTA_HH_DATA_IND_EVT: - param_len = sizeof(tBTA_HH_INTDATA); - break; case BTA_HH_API_ERR_EVT: param_len = 0; break; @@ -472,6 +537,7 @@ static void btc_hh_init(void) { BTC_TRACE_API("%s", __func__); esp_hidh_status_t ret = ESP_HIDH_OK; + uint8_t i; do { if (is_hidh_init()) { BTC_TRACE_ERROR("%s HH has been initiated, shall uninit first!", __func__); @@ -480,8 +546,19 @@ static void btc_hh_init(void) } memset(&btc_hh_cb, 0, sizeof(btc_hh_cb)); - for (uint8_t i = 0; i < BTC_HH_MAX_HID; i++) { + for (i = 0; i < BTC_HH_MAX_HID; i++) { btc_hh_cb.devices[i].dev_status = ESP_HIDH_CONN_STATE_UNKNOWN; + if (!btc_hh_device_drop_cnt_init(&btc_hh_cb.devices[i])) { + BTC_TRACE_ERROR("%s: device lock init failed", __func__); + ret = ESP_HIDH_ERR_NO_RES; + break; + } + } + if (ret != ESP_HIDH_OK) { + while (i-- > 0) { + btc_hh_device_drop_cnt_deinit(&btc_hh_cb.devices[i]); + } + break; } BTA_HhEnable(BTA_SEC_AUTHENTICATE | BTA_SEC_ENCRYPT, bte_hh_evt); } while (0); @@ -1220,14 +1297,263 @@ static void btc_hh_cb_arg_deep_free(btc_msg_t *msg) case BTA_HH_GET_RPT_EVT: utl_freebuf((void **)&arg->hs_data.rsp_data.p_rpt_data); break; - case BTA_HH_DATA_IND_EVT: - utl_freebuf((void **)&arg->int_data.p_data); - break; default: break; } } +static void btc_hh_data_path_deinit_inner(btc_hh_device_t *p_dev) +{ + struct pkt_queue *queue; + struct osi_event *event; + + if (p_dev == NULL) { + return; + } + + osi_mutex_lock(&p_dev->lock, OSI_MUTEX_MAX_TIMEOUT); + queue = p_dev->data_queue; + event = p_dev->data_ready; + p_dev->data_queue = NULL; + p_dev->data_ready = NULL; + osi_mutex_unlock(&p_dev->lock); + + if (event != NULL) { + osi_event_delete(event); + } + + if (queue != NULL) { + pkt_queue_destroy(queue, bta_hh_co_data_linked_pkt_free); + } +} + +bool btc_hh_data_enqueue_linked_pkt(pkt_linked_item_t *linked_pkt) +{ + tBTA_HH_DATA_PKT *pkt = linked_pkt != NULL ? (tBTA_HH_DATA_PKT *)linked_pkt->data : NULL; + btc_hh_device_t *p_dev = pkt != NULL ? btc_hh_find_connected_dev_by_handle(pkt->dev_handle) : NULL; + struct pkt_queue *data_queue; + struct osi_event *data_ready; + pkt_linked_item_t *old = NULL; + bool enqueue_ok; + + if (p_dev == NULL || pkt == NULL || pkt->p_buf == NULL || pkt->len == 0) { + return false; + } + + osi_mutex_lock(&p_dev->lock, OSI_MUTEX_MAX_TIMEOUT); + data_queue = p_dev->data_queue; + data_ready = p_dev->data_ready; + if (data_queue == NULL) { + osi_mutex_unlock(&p_dev->lock); + return false; + } + + if (pkt_queue_length(data_queue) >= BTC_HH_DATA_QUEUE_LEN_MAX) { + old = pkt_queue_dequeue(data_queue); + if (old != NULL) { + p_dev->drop_pkt_cnt++; + } + } + + enqueue_ok = pkt_queue_enqueue(data_queue, linked_pkt); + if (enqueue_ok && data_ready != NULL) { + osi_thread_post_event(data_ready, 0); + } + osi_mutex_unlock(&p_dev->lock); + + if (old != NULL) { + bta_hh_co_data_linked_pkt_free(old); + } + + return enqueue_ok; +} + +pkt_linked_item_t *btc_hh_data_dequeue_reusable_linked_pkt(uint8_t dev_handle) +{ + btc_hh_device_t *p_dev = btc_hh_find_connected_dev_by_handle(dev_handle); + struct pkt_queue *data_queue; + pkt_linked_item_t *old; + + if (p_dev == NULL) { + return NULL; + } + + osi_mutex_lock(&p_dev->lock, OSI_MUTEX_MAX_TIMEOUT); + data_queue = p_dev->data_queue; + if (data_queue == NULL || pkt_queue_length(data_queue) < BTC_HH_DATA_QUEUE_LEN_MAX) { + osi_mutex_unlock(&p_dev->lock); + return NULL; + } + + old = pkt_queue_dequeue(data_queue); + if (old != NULL) { + p_dev->drop_pkt_cnt++; + } + osi_mutex_unlock(&p_dev->lock); + + return old; +} + +bool btc_hh_data_enqueue_pkt(const tBTA_HH_DATA_PKT *pkt_meta) +{ + pkt_linked_item_t *linked_pkt; + tBTA_HH_DATA_PKT *pkt; + + if (pkt_meta == NULL || pkt_meta->p_buf == NULL || pkt_meta->len == 0) { + return false; + } + + linked_pkt = (pkt_linked_item_t *)osi_malloc(BT_PKT_LINKED_HDR_SIZE + sizeof(tBTA_HH_DATA_PKT)); + if (linked_pkt == NULL) { + return false; + } + + pkt = (tBTA_HH_DATA_PKT *)linked_pkt->data; + *pkt = *pkt_meta; + + if (!btc_hh_data_enqueue_linked_pkt(linked_pkt)) { + osi_free(linked_pkt); + return false; + } + + return true; +} + +static bool btc_hh_data_path_init_inner(btc_hh_device_t *p_dev) +{ + bool result = false; + struct pkt_queue *data_queue = NULL; + struct osi_event *data_ready = NULL; + + do { + if (p_dev == NULL) { + break; + } + + osi_mutex_lock(&p_dev->lock, OSI_MUTEX_MAX_TIMEOUT); + if (p_dev->data_queue != NULL && p_dev->data_ready != NULL) { + osi_mutex_unlock(&p_dev->lock); + result = true; + break; + } + osi_mutex_unlock(&p_dev->lock); + + btc_hh_data_path_deinit_inner(p_dev); + + data_queue = pkt_queue_create(); + if (data_queue == NULL) { + BTC_TRACE_ERROR("%s: pkt_queue_create failed", __func__); + break; + } + + data_ready = osi_event_create(btc_hh_data_pkt_handler, p_dev); + if (data_ready == NULL) { + BTC_TRACE_ERROR("%s: osi_event_create failed", __func__); + break; + } + + if (!osi_event_bind(data_ready, btc_get_current_thread(), BTC_HH_DATA_QUEUE_IDX)) { + BTC_TRACE_ERROR("%s: osi_event_bind failed", __func__); + break; + } + + osi_mutex_lock(&p_dev->lock, OSI_MUTEX_MAX_TIMEOUT); + p_dev->data_queue = data_queue; + p_dev->data_ready = data_ready; + p_dev->drop_pkt_cnt = 0; + osi_mutex_unlock(&p_dev->lock); + data_queue = NULL; + data_ready = NULL; + result = true; + } while (0); + + if (!result) { + btc_hh_data_path_deinit_inner(p_dev); + if (data_ready != NULL) { + osi_event_delete(data_ready); + } + if (data_queue != NULL) { + pkt_queue_destroy(data_queue, bta_hh_co_data_linked_pkt_free); + } + } + + return result; +} + +bool btc_hh_data_path_init(uint8_t dev_handle) +{ + btc_hh_device_t *p_dev = btc_hh_find_connected_dev_by_handle(dev_handle); + return btc_hh_data_path_init_inner(p_dev); +} + +static size_t btc_hh_get_batch_size(size_t queue_len) +{ + size_t batch_size = BTC_HH_DATA_BATCH_SIZE_BASE; + + if (queue_len >= BTC_HH_DATA_BATCH_SIZE_MAX) { + batch_size = BTC_HH_DATA_BATCH_SIZE_MAX; + } else if (queue_len >= BTC_HH_DATA_BATCH_SIZE_HIGH) { + batch_size = BTC_HH_DATA_BATCH_SIZE_HIGH; + } + + return batch_size; +} + +static void btc_hh_data_pkt_handler(void *arg) +{ + tBTA_HH_DATA_PKT *pkt; + pkt_linked_item_t *linked_pkt; + btc_hh_device_t *p_dev = (btc_hh_device_t *)arg; + size_t pkts_to_process; + size_t batch_limit; + uint32_t drop_pkt_cnt; + + if (p_dev == NULL || p_dev->data_queue == NULL) { + return; + } + + drop_pkt_cnt = p_dev->drop_pkt_cnt; + if (drop_pkt_cnt) { + osi_mutex_lock(&p_dev->lock, OSI_MUTEX_MAX_TIMEOUT); + drop_pkt_cnt = p_dev->drop_pkt_cnt; + p_dev->drop_pkt_cnt = 0; + osi_mutex_unlock(&p_dev->lock); + + BTC_TRACE_WARNING("hh hdl:%d drop %u pkts", p_dev->dev_handle, (unsigned int)drop_pkt_cnt); + } + + pkts_to_process = pkt_queue_length(p_dev->data_queue); + batch_limit = btc_hh_get_batch_size(pkts_to_process); + if (pkts_to_process > batch_limit) { + pkts_to_process = batch_limit; + } + + for (size_t i = 0; i < pkts_to_process; i++) { + esp_hidh_cb_param_t param = {0}; + + linked_pkt = pkt_queue_dequeue(p_dev->data_queue); + if (linked_pkt == NULL) { + break; + } + + pkt = (tBTA_HH_DATA_PKT *)linked_pkt->data; + BTC_TRACE_DEBUG("handle = %d", pkt->dev_handle); + if (pkt->len > 0) { + param.data_ind.len = pkt->len; + param.data_ind.data = bta_hh_co_data_pkt_get_payload(pkt); + } + param.data_ind.handle = pkt->dev_handle; + param.data_ind.status = ESP_HIDH_OK; + param.data_ind.proto_mode = proto_mode_change_to_upper_layer(pkt->proto_mode); + btc_hh_cb_to_app(ESP_HIDH_DATA_IND_EVT, ¶m); + bta_hh_co_data_linked_pkt_free(linked_pkt); + } + + if (p_dev->data_ready != NULL && !pkt_queue_is_empty(p_dev->data_queue)) { + osi_thread_post_event(p_dev->data_ready, 0); + } +} + bool btc_hh_copy_hid_info(tBTA_HH_DEV_DSCP_INFO *dest, tBTA_HH_DEV_DSCP_INFO *src) { dest->descriptor.dl_len = 0; @@ -1287,12 +1613,20 @@ void btc_hh_cb_handler(btc_msg_t *msg) btc_hh_cb.status = BTC_HH_ENABLED; BTC_TRACE_DEBUG("Loading added devices"); /* Add hid descriptors for already bonded hid devices*/ - // btc_storage_load_bonded_hid_info(); + btc_storage_load_bonded_hid_info(); + param.init.status = ESP_HIDH_OK; } else { + for (i = 0; i < BTC_HH_MAX_HID; i++) { + btc_hh_device_drop_cnt_deinit(&btc_hh_cb.devices[i]); + } + memset(&btc_hh_cb, 0, sizeof(btc_hh_cb)); + for (i = 0; i < BTC_HH_MAX_HID; i++) { + btc_hh_cb.devices[i].dev_status = ESP_HIDH_CONN_STATE_UNKNOWN; + } btc_hh_cb.status = BTC_HH_DISABLED; BTC_TRACE_ERROR("Error, HH enabling failed, status = %d", p_data->status); + param.init.status = p_data->status; } - param.init.status = p_data->status; btc_hh_cb_to_app(ESP_HIDH_INIT_EVT, ¶m); break; case BTA_HH_DISABLE_EVT: @@ -1302,9 +1636,12 @@ void btc_hh_cb_handler(btc_msg_t *msg) // btif_hd_service_registration(); btc_hh_cb.service_dereg_active = FALSE; } + if (p_data->status == BTA_HH_OK) { // Clear the control block for (i = 0; i < BTC_HH_MAX_HID; i++) { + btc_hh_data_path_deinit_inner(&btc_hh_cb.devices[i]); + btc_hh_device_drop_cnt_deinit(&btc_hh_cb.devices[i]); if (btc_hh_cb.devices[i].vup_timer) { osi_alarm_free(btc_hh_cb.devices[i].vup_timer); } @@ -1343,9 +1680,8 @@ void btc_hh_cb_handler(btc_msg_t *msg) // if (check_cod(&p_data->conn.bda, COD_HID_KEYBOARD) || check_cod(&p_data->conn.bda, COD_HID_COMBO)) // BTA_HhSetIdle(p_data->conn.handle, 0); btc_hh_cb.p_curr_dev = btc_hh_find_connected_dev_by_handle(p_data->conn.handle); - BTA_HhGetDscpInfo(p_data->conn.handle); p_dev->dev_status = ESP_HIDH_CONN_STATE_CONNECTED; - + BTA_HhGetDscpInfo(p_data->conn.handle); param.open.status = ESP_HIDH_OK; param.open.conn_status = ESP_HIDH_CONN_STATE_CONNECTED; } @@ -1354,6 +1690,7 @@ void btc_hh_cb_handler(btc_msg_t *msg) if (p_dev != NULL) { btc_hh_stop_vup_timer(p_dev->bd_addr); p_dev->dev_status = ESP_HIDH_CONN_STATE_DISCONNECTED; + btc_hh_data_path_deinit_inner(p_dev); } btc_hh_cb.status = (BTC_HH_STATUS)BTC_HH_DEV_DISCONNECTED; @@ -1419,10 +1756,13 @@ void btc_hh_cb_handler(btc_msg_t *msg) case BTA_HH_CLOSE_EVT: BTC_TRACE_DEBUG("status = %d, handle = %d", p_data->dev_status.status, p_data->dev_status.handle); + btc_hh_cb.status = (BTC_HH_STATUS)BTC_HH_DEV_DISCONNECTED; p_dev = btc_hh_find_connected_dev_by_handle(p_data->dev_status.handle); if (p_dev != NULL) { BTC_TRACE_DEBUG("uhid local_vup=%d", p_dev->local_vup); btc_hh_stop_vup_timer(p_dev->bd_addr); + p_dev->dev_status = ESP_HIDH_CONN_STATE_DISCONNECTED; + btc_hh_data_path_deinit_inner(p_dev); /* If this is a locally initiated VUP, remove the bond as ACL got * disconnected while VUP being processed. */ @@ -1432,11 +1772,7 @@ void btc_hh_cb_handler(btc_msg_t *msg) BTA_DmRemoveDevice(p_dev->bd_addr, BT_TRANSPORT_BR_EDR); #endif btc_hh_remove_device(p_dev->bd_addr); - } else { - p_dev->dev_status = ESP_HIDH_CONN_STATE_DISCONNECTED; } - - btc_hh_cb.status = (BTC_HH_STATUS)BTC_HH_DEV_DISCONNECTED; param.close.status = p_data->dev_status.status; } else { BTC_TRACE_ERROR("Error: cannot find device with handle %d", p_data->dev_status.handle); @@ -1449,12 +1785,13 @@ void btc_hh_cb_handler(btc_msg_t *msg) case BTA_HH_VC_UNPLUG_EVT: BTC_TRACE_DEBUG("status = %d, handle = %d", p_data->dev_status.status, p_data->dev_status.handle); - p_dev = btc_hh_find_connected_dev_by_handle(p_data->dev_status.handle); btc_hh_cb.status = (BTC_HH_STATUS)BTC_HH_DEV_DISCONNECTED; + p_dev = btc_hh_find_connected_dev_by_handle(p_data->dev_status.handle); if (p_dev != NULL) { /* Stop the VUP timer */ btc_hh_stop_vup_timer(p_dev->bd_addr); p_dev->dev_status = ESP_HIDH_CONN_STATE_DISCONNECTED; + btc_hh_data_path_deinit_inner(p_dev); BTC_TRACE_DEBUG("%s---Sending connection state change", __func__); param.close.status = ESP_HIDH_OK; param.close.handle = p_data->dev_status.handle; @@ -1577,17 +1914,6 @@ void btc_hh_cb_handler(btc_msg_t *msg) memcpy(param.rmv_dev.bd_addr, p_data->dev_info.bda, BD_ADDR_LEN); btc_hh_cb_to_app(ESP_HIDH_RMV_DEV_EVT, ¶m); break; - case BTA_HH_DATA_IND_EVT: - BTC_TRACE_DEBUG("status = %d, handle = %d", p_data->int_data.status, p_data->int_data.handle); - if (p_data->int_data.status == BTA_HH_OK && p_data->int_data.p_data) { - param.data_ind.len = p_data->int_data.p_data->len; - param.data_ind.data = p_data->int_data.p_data->data + p_data->int_data.p_data->offset; - } - param.data_ind.handle = p_data->int_data.handle; - param.data_ind.status = p_data->int_data.status; - param.data_ind.proto_mode = proto_mode_change_to_upper_layer(p_data->int_data.proto_mode); - btc_hh_cb_to_app(ESP_HIDH_DATA_IND_EVT, ¶m); - break; case BTA_HH_API_ERR_EVT: break; default: diff --git a/components/bt/host/bluedroid/btc/profile/std/include/btc_hh.h b/components/bt/host/bluedroid/btc/profile/std/include/btc_hh.h index 827cd095c75..3117cc7a30f 100644 --- a/components/bt/host/bluedroid/btc/profile/std/include/btc_hh.h +++ b/components/bt/host/bluedroid/btc/profile/std/include/btc_hh.h @@ -22,11 +22,18 @@ #include #include "bta/bta_hh_api.h" +#include "bta/bta_hh_co.h" +#include "stack/bt_types.h" #include "btc/btc_task.h" #include "osi/alarm.h" +#include "osi/pkt_queue.h" +#include "osi/thread.h" +#include "osi/mutex.h" #include "esp_hidh_api.h" -#define BTC_HH_MAX_HID 8 +#if (defined BTC_HH_INCLUDED && BTC_HH_INCLUDED == TRUE) + +#define BTC_HH_MAX_HID BTA_HH_MAX_DEVICE #define BTC_HH_MAX_ADDED_DEV 32 #define BTC_HH_MAX_KEYSTATES 3 @@ -67,15 +74,18 @@ typedef enum { } BTC_HH_STATUS; typedef struct { - esp_hidh_connection_state_t dev_status; + uint8_t dev_status; // see esp_hidh_connection_state_t uint8_t dev_handle; - BD_ADDR bd_addr; - uint16_t attr_mask; uint8_t sub_class; uint8_t app_id; - bool ready_for_data; - osi_alarm_t *vup_timer; bool local_vup; // Indicated locally initiated VUP + BD_ADDR bd_addr; + uint16_t attr_mask; + uint32_t drop_pkt_cnt; + osi_mutex_t lock; + struct pkt_queue *data_queue; + struct osi_event *data_ready; + osi_alarm_t *vup_timer; } btc_hh_device_t; /* Control block to maintain properties of devices */ @@ -187,4 +197,13 @@ bool btc_hh_add_added_dev(BD_ADDR bd_addr, uint16_t attr_mask); void btc_hh_get_profile_status(esp_hidh_profile_status_t *param); +btc_hh_device_t *btc_hh_find_connected_dev_by_handle(uint8_t handle); +bool btc_hh_data_enqueue_pkt(const tBTA_HH_DATA_PKT *pkt_meta); +bool btc_hh_data_enqueue_linked_pkt(pkt_linked_item_t *linked_pkt); +pkt_linked_item_t *btc_hh_data_dequeue_reusable_linked_pkt(uint8_t dev_handle); +void btc_hh_on_pkt_dropped(uint8_t dev_handle); +void btc_hh_reset_drop_pkt_cnt(uint8_t dev_handle); +bool btc_hh_data_path_init(uint8_t dev_handle); +#endif /* (defined BTC_HH_INCLUDED && BTC_HH_INCLUDED == TRUE) */ + #endif /* BTC_HH_H */ From 6c27057d67efca95e38cc3bdf6e74b8e0058a9fe Mon Sep 17 00:00:00 2001 From: liqigan Date: Fri, 26 Jun 2026 14:55:04 +0800 Subject: [PATCH 2/7] change(bt/bluedroid): Refactored HCI ACL datapath --- components/bt/host/bluedroid/hci/hci_layer.c | 19 +- .../bt/host/bluedroid/stack/btu/btu_init.c | 20 +- .../bt/host/bluedroid/stack/btu/btu_task.c | 174 ++++++++++++++++++ .../host/bluedroid/stack/include/stack/btu.h | 9 + 4 files changed, 214 insertions(+), 8 deletions(-) diff --git a/components/bt/host/bluedroid/hci/hci_layer.c b/components/bt/host/bluedroid/hci/hci_layer.c index a2265a90eb3..cb6bf9b6a4d 100644 --- a/components/bt/host/bluedroid/hci/hci_layer.c +++ b/components/bt/host/bluedroid/hci/hci_layer.c @@ -581,8 +581,23 @@ intercepted: static void dispatch_reassembled(BT_HDR *packet) { // Events should already have been dispatched before this point - //Tell Up-layer received packet. - if (btu_task_post(SIG_BTU_HCI_MSG, packet, OSI_THREAD_MAX_TIMEOUT) == false) { + // Tell Up-layer received packet. + do { + if ((packet->event & BT_EVT_MASK) == BT_EVT_TO_BTU_HCI_ACL) { + if (btu_hci_acl_data_post(packet)) { + packet = NULL; + } + // TODO: Use controller to host flow control + break; + } + + if (btu_task_post(SIG_BTU_HCI_MSG, packet, OSI_THREAD_MAX_TIMEOUT)) { + packet = NULL; + break; + } + } while (0); + + if (packet != NULL) { osi_free(packet); } } diff --git a/components/bt/host/bluedroid/stack/btu/btu_init.c b/components/bt/host/bluedroid/stack/btu/btu_init.c index 1bcad2f7ce5..1d3b70a0e23 100644 --- a/components/bt/host/bluedroid/stack/btu/btu_init.c +++ b/components/bt/host/bluedroid/stack/btu/btu_init.c @@ -206,6 +206,10 @@ bool BTU_StartUp(void) goto error_exit; } + if (!btu_acl_queue_init()) { + goto error_exit; + } + if (btu_task_post(SIG_BTU_START_UP, NULL, OSI_THREAD_MAX_TIMEOUT) == false) { goto error_exit; } @@ -229,10 +233,19 @@ error_exit:; ******************************************************************************/ void BTU_ShutDown(void) { + btu_acl_queue_close(); + + btu_task_shut_down(); + + if (btu_thread) { + osi_thread_free(btu_thread); + btu_thread = NULL; + } + + btu_acl_queue_deinit(); #if BTU_DYNAMIC_MEMORY FREE_AND_RESET(btu_cb_ptr); #endif - btu_task_shut_down(); hash_map_free(btu_general_alarm_hash_map); osi_mutex_free(&btu_general_alarm_lock); @@ -243,11 +256,6 @@ void BTU_ShutDown(void) hash_map_free(btu_l2cap_alarm_hash_map); osi_mutex_free(&btu_l2cap_alarm_lock); - if (btu_thread) { - osi_thread_free(btu_thread); - btu_thread = NULL; - } - btu_general_alarm_hash_map = NULL; btu_oneshot_alarm_hash_map = NULL; btu_l2cap_alarm_hash_map = NULL; diff --git a/components/bt/host/bluedroid/stack/btu/btu_task.c b/components/bt/host/bluedroid/stack/btu/btu_task.c index 56c7983d044..4f929f6838a 100644 --- a/components/bt/host/bluedroid/stack/btu/btu_task.c +++ b/components/bt/host/bluedroid/stack/btu/btu_task.c @@ -29,6 +29,7 @@ #include "btm_int.h" #include "stack/btu.h" #include "osi/hash_map.h" +#include "osi/pkt_queue.h" #include "stack/hcimsgs.h" #include "l2c_int.h" #include "osi/osi.h" @@ -122,6 +123,25 @@ typedef void (tUSER_TIMEOUT_FUNC) (TIMER_LIST_ENT *p_tle); static void btu_l2cap_alarm_process(void *param); static void btu_general_alarm_process(void *param); static void btu_hci_msg_process(void *param); +static void btu_hci_acl_data_handler(void *param); +static bool btu_hci_acl_data_ready(uint32_t timeout); +static void btu_acl_pkt_linked_free(pkt_linked_item_t *linked_pkt); + +static void btu_acl_pkt_linked_free(pkt_linked_item_t *linked_pkt) +{ + do { + if (linked_pkt == NULL) { + break; + } + + BT_HDR *packet = NULL; + memcpy(&packet, linked_pkt->data, sizeof(packet)); + if (packet != NULL) { + osi_free(packet); + } + osi_free(linked_pkt); + } while (0); +} #if (defined(BTA_INCLUDED) && BTA_INCLUDED == TRUE) static void btu_bta_alarm_process(void *param); @@ -197,6 +217,57 @@ static void btu_hci_msg_process(void *param) } +static bool btu_hci_acl_data_ready(uint32_t timeout) +{ + bool status = false; + + do { + if (btu_cb.acl_closing || btu_cb.acl_data_ready == NULL) { + break; + } + + status = osi_thread_post_event(btu_cb.acl_data_ready, timeout); + } while (0); + + return status; +} + +static void btu_hci_acl_data_handler(void *param) +{ + UNUSED(param); + struct pkt_queue *acl_pkt_queue = btu_cb.acl_pkt_queue; + if (acl_pkt_queue == NULL || btu_cb.acl_closing) { + return; + } + + size_t pkts_to_process = pkt_queue_length(acl_pkt_queue); + if (pkts_to_process > BTU_ACL_QUEUE_BATCH_SIZE) { + pkts_to_process = BTU_ACL_QUEUE_BATCH_SIZE; + } + + for (size_t i = 0; i < pkts_to_process; i++) { + pkt_linked_item_t *linked_pkt = pkt_queue_dequeue(acl_pkt_queue); + if (linked_pkt == NULL) { + break; + } + + BT_HDR *packet = NULL; + memcpy(&packet, linked_pkt->data, sizeof(packet)); + if (packet == NULL) { + osi_free(linked_pkt); + continue; + } + l2c_rcv_acl_data(packet); + osi_free(linked_pkt); + } + + size_t pending = pkt_queue_length(acl_pkt_queue); + if (pending != 0) { + // Re-post from BTU thread itself must stay non-blocking to avoid deadlock on a full queue. + btu_hci_acl_data_ready(0); + } +} + #if (defined(BTA_INCLUDED) && BTA_INCLUDED == TRUE) static void btu_bta_alarm_process(void *param) { @@ -263,6 +334,109 @@ bool btu_task_post(uint32_t sig, void *param, uint32_t timeout) return status; } +bool btu_acl_queue_init(void) +{ + bool status = false; + + btu_cb.acl_closing = FALSE; + + do { + btu_cb.acl_pkt_queue = pkt_queue_create(); + if (btu_cb.acl_pkt_queue == NULL) { + break; + } + + btu_cb.acl_data_ready = osi_event_create(btu_hci_acl_data_handler, NULL); + if (btu_cb.acl_data_ready == NULL) { + break; + } + + if (!osi_event_bind(btu_cb.acl_data_ready, btu_thread, 0)) { + break; + } + status = true; + } while (0); + + if (status == false) { + if (btu_cb.acl_data_ready != NULL) { + osi_event_delete(btu_cb.acl_data_ready); + btu_cb.acl_data_ready = NULL; + } + + if (btu_cb.acl_pkt_queue != NULL) { + pkt_queue_destroy(btu_cb.acl_pkt_queue, NULL); + btu_cb.acl_pkt_queue = NULL; + } + + btu_cb.acl_closing = TRUE; + } + + return status; +} + +void btu_acl_queue_close(void) +{ +#if BTU_DYNAMIC_MEMORY == TRUE + if (!btu_cb_ptr) { + return; + } +#endif /* BTU_DYNAMIC_MEMORY == FALSE */ + + btu_cb.acl_closing = TRUE; + + if (btu_cb.acl_data_ready != NULL) { + osi_event_delete(btu_cb.acl_data_ready); + btu_cb.acl_data_ready = NULL; + } +} + +void btu_acl_queue_deinit(void) +{ +#if BTU_DYNAMIC_MEMORY == TRUE + if (!btu_cb_ptr) { + return; + } +#endif /* BTU_DYNAMIC_MEMORY == FALSE */ + + btu_cb.acl_closing = TRUE; + + if (btu_cb.acl_pkt_queue != NULL) { + pkt_queue_destroy(btu_cb.acl_pkt_queue, btu_acl_pkt_linked_free); + btu_cb.acl_pkt_queue = NULL; + } +} + +bool btu_hci_acl_data_post(BT_HDR *packet) +{ + bool status = false; + + do { + if (packet == NULL || btu_cb.acl_closing || btu_cb.acl_pkt_queue == NULL) { + break; + } + + size_t acl_q_len = pkt_queue_length(btu_cb.acl_pkt_queue); + if (acl_q_len >= BTU_ACL_QUEUE_HIGH_WATERMARK) { + HCI_TRACE_WARNING("ACL queue high watermark (len=%u)", (unsigned)acl_q_len); + break; + } + + pkt_linked_item_t *linked_pkt = (pkt_linked_item_t *)osi_malloc(BT_PKT_LINKED_HDR_SIZE + sizeof(packet)); + if (linked_pkt == NULL) { + HCI_TRACE_WARNING("ACL queue malloc pkt failed"); + break; + } + + memcpy(linked_pkt->data, &packet, sizeof(packet)); + pkt_queue_enqueue(btu_cb.acl_pkt_queue, linked_pkt); + + btu_hci_acl_data_ready(OSI_THREAD_MAX_TIMEOUT); + status = true; + } while (0); + + return status; +} + void btu_task_start_up(void *param) { UNUSED(param); diff --git a/components/bt/host/bluedroid/stack/include/stack/btu.h b/components/bt/host/bluedroid/stack/include/stack/btu.h index f973cccb68c..7a49619e050 100644 --- a/components/bt/host/bluedroid/stack/include/stack/btu.h +++ b/components/bt/host/bluedroid/stack/include/stack/btu.h @@ -206,6 +206,8 @@ typedef struct { #define BTU_MAX_REG_TIMER (2) /* max # timer callbacks which may register */ #define BTU_MAX_REG_EVENT (6) /* max # event callbacks which may register */ #define BTU_DEFAULT_DATA_SIZE (0x2a0) +#define BTU_ACL_QUEUE_BATCH_SIZE (12) +#define BTU_ACL_QUEUE_HIGH_WATERMARK (100) #if (BLE_INCLUDED == TRUE) #define BTU_DEFAULT_BLE_DATA_SIZE (27) @@ -234,6 +236,9 @@ typedef struct { typedef struct { tBTU_TIMER_REG timer_reg[BTU_MAX_REG_TIMER]; tBTU_EVENT_REG event_reg[BTU_MAX_REG_EVENT]; + struct pkt_queue *acl_pkt_queue; + struct osi_event *acl_data_ready; + BOOLEAN acl_closing; BOOLEAN reset_complete; /* TRUE after first ack from device received */ UINT8 trace_level; /* Trace level for HCI layer */ @@ -303,6 +308,10 @@ void btu_task_shut_down(void); UINT16 BTU_BleAclPktSize(void); bool btu_task_post(uint32_t sig, void *param, uint32_t timeout); +bool btu_acl_queue_init(void); +void btu_acl_queue_close(void); +void btu_acl_queue_deinit(void); +bool btu_hci_acl_data_post(BT_HDR *packet); int get_btu_work_queue_size(void); From 1dcaf241291c75d94372905343ba3137d8c4c35d Mon Sep 17 00:00:00 2001 From: liqigan Date: Fri, 12 Jun 2026 11:24:41 +0800 Subject: [PATCH 3/7] fix(bt/bluedroid): Fixed HID host reconnection bug and enabled load HID devices Closes https://github.com/espressif/esp-idf/issues/18335 --- .../bluedroid/btc/profile/std/hid/btc_hh.c | 3 +- components/esp_hid/src/bt_hidh.c | 232 +++++++++++++----- .../bluetooth/esp_hid_host/main/esp_hid_gap.c | 2 +- 3 files changed, 171 insertions(+), 66 deletions(-) diff --git a/components/bt/host/bluedroid/btc/profile/std/hid/btc_hh.c b/components/bt/host/bluedroid/btc/profile/std/hid/btc_hh.c index e73594210ae..dc0960f355c 100644 --- a/components/bt/host/bluedroid/btc/profile/std/hid/btc_hh.c +++ b/components/bt/host/bluedroid/btc/profile/std/hid/btc_hh.c @@ -1629,7 +1629,7 @@ void btc_hh_cb_handler(btc_msg_t *msg) } btc_hh_cb_to_app(ESP_HIDH_INIT_EVT, ¶m); break; - case BTA_HH_DISABLE_EVT: + case BTA_HH_DISABLE_EVT: { btc_hh_cb.status = BTC_HH_DISABLED; if (btc_hh_cb.service_dereg_active) { BTIF_TRACE_DEBUG("BTA_HH_DISABLE_EVT: enabling HID Device service"); @@ -1656,6 +1656,7 @@ void btc_hh_cb_handler(btc_msg_t *msg) param.deinit.status = p_data->status; btc_hh_cb_to_app(ESP_HIDH_DEINIT_EVT, ¶m); break; + } case BTA_HH_OPEN_EVT: BTC_TRACE_DEBUG("handle=%d, status =%d", p_data->conn.handle, p_data->conn.status); memset(btc_hh_cb.pending_conn_address, 0, BD_ADDR_LEN); diff --git a/components/esp_hid/src/bt_hidh.c b/components/esp_hid/src/bt_hidh.c index 6b33beee5e9..ee946506275 100644 --- a/components/esp_hid/src/bt_hidh.c +++ b/components/esp_hid/src/bt_hidh.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2017-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2017-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -14,19 +14,26 @@ #include "freertos/FreeRTOS.h" #include "freertos/task.h" #include "freertos/semphr.h" -#include "osi/fixed_queue.h" +#include "osi/list.h" #include "string.h" #include "esp_hidh_api.h" static const char *TAG = "BT_HIDH"; // element of connection queue +typedef enum { + CONN_FLOW_WAIT_DSCP = 0, // OPEN ok, waiting for GET_DSCP + CONN_FLOW_WAIT_ADD_DEV, // GET_DSCP ok and !added, waiting for ADD_DEV +} conn_flow_state_t; + typedef struct { esp_hidh_dev_t* dev; + uint8_t handle; + uint8_t state; } conn_item_t; typedef struct { - fixed_queue_t *connection_queue; /* Queue of connection */ + list_t *connection_queue; /* List of pending connections (accessed only in esp_hh_cb / BTC task context) */ esp_event_loop_handle_t event_loop_handle; } hidh_local_param_t; @@ -86,7 +93,7 @@ static char *get_trans_type_str(esp_hid_trans_type_t trans_type) case ESP_HID_TRANS_MAX: return "TRANS_MAX"; default: - return "UNKOWN"; + return "UNKNOWN"; } } @@ -107,14 +114,6 @@ static esp_err_t bt_hidh_get_status(esp_hidh_status_t status) return ret; } -static void utl_freebuf(void **p) -{ - if (*p != NULL) { - free(*p); - *p = NULL; - } -} - static void transaction_timeout_handler(void *arg) { esp_hidh_dev_t *dev = (esp_hidh_dev_t *)arg; @@ -168,7 +167,8 @@ static inline bool is_trans_done(esp_hidh_dev_t *dev) static void free_local_param(void) { if (hidh_local_param.connection_queue) { - fixed_queue_free(hidh_local_param.connection_queue, free); + list_free(hidh_local_param.connection_queue); + hidh_local_param.connection_queue = NULL; } } @@ -190,6 +190,44 @@ static void open_failed_cb(esp_hidh_dev_t *dev, esp_hidh_status_t status, esp_hi portMAX_DELAY); } +// Find the pending connection entry matching |handle|. The connection flow is driven +// entirely from esp_hh_cb (single BTC task context), so no extra locking is required. +static conn_item_t *find_conn_item_by_handle(uint8_t handle) +{ + if (hidh_local_param.connection_queue == NULL) { + return NULL; + } + for (const list_node_t *node = list_begin(hidh_local_param.connection_queue); + node != list_end(hidh_local_param.connection_queue); node = list_next(node)) { + conn_item_t *conn_item = (conn_item_t *)list_node(node); + if (conn_item != NULL && conn_item->handle == handle) { + return conn_item; + } + } + return NULL; +} + +// Remove a specific entry from the connection list. list_remove() invokes the free +// callback registered in list_new(), releasing the conn_item itself. +static void remove_conn_item(conn_item_t *conn_item) +{ + if (conn_item != NULL && hidh_local_param.connection_queue != NULL) { + list_remove(hidh_local_param.connection_queue, conn_item); + } +} + +static const char *conn_flow_state_str(conn_flow_state_t state) +{ + switch (state) { + case CONN_FLOW_WAIT_DSCP: + return "WAIT_DSCP"; + case CONN_FLOW_WAIT_ADD_DEV: + return "WAIT_ADD_DEV"; + default: + return "UNKNOWN"; + } +} + static void esp_hh_cb(esp_hidh_cb_event_t event, esp_hidh_cb_param_t *param) { conn_item_t *conn_item = NULL; @@ -265,8 +303,16 @@ static void esp_hh_cb(esp_hidh_cb_event_t event, esp_hidh_cb_param_t *param) break; } conn_item->dev = dev; - bool ret = fixed_queue_enqueue(hidh_local_param.connection_queue, conn_item, FIXED_QUEUE_MAX_TIMEOUT); - assert(ret == true); + conn_item->handle = param->open.handle; + conn_item->state = CONN_FLOW_WAIT_DSCP; + bool ret = list_append(hidh_local_param.connection_queue, conn_item); + if (!ret) { + ESP_LOGE(TAG, "conn_item enqueue failed!"); + free(conn_item); + conn_item = NULL; + param->open.status = ESP_HIDH_ERR_NO_RES; + break; + } } while (0); if (param->open.status != ESP_HIDH_OK) { @@ -282,38 +328,57 @@ static void esp_hh_cb(esp_hidh_cb_event_t event, esp_hidh_cb_param_t *param) break; } case ESP_HIDH_GET_DSCP_EVT: { + bool post_open = false; + bool report_open_failed = false; + esp_hidh_status_t open_failed_status = ESP_HIDH_OK; + do { ESP_LOGV(TAG, "DESCRIPTOR: PID: 0x%04x, VID: 0x%04x, VERSION: 0x%04x, REPORT_LEN: %u", param->dscp.product_id, param->dscp.vendor_id, param->dscp.version, param->dscp.dl_len); - if ((conn_item = (conn_item_t *)fixed_queue_dequeue(hidh_local_param.connection_queue, - FIXED_QUEUE_MAX_TIMEOUT)) == NULL) { - ESP_LOGE(TAG, "No pending connect device!"); - param->dscp.status = ESP_HIDH_NO_CONNECTION; + conn_item = find_conn_item_by_handle(param->dscp.handle); + if (!conn_item) { + ESP_LOGE(TAG, "No pending connect device for handle %d!", param->dscp.handle); break; } + dev = conn_item->dev; - utl_freebuf((void **)&conn_item); // in case the dev has been freed if (!esp_hidh_dev_exists(dev)) { ESP_LOGE(TAG, "Device Not Found"); dev = NULL; param->dscp.status = ESP_HIDH_NO_CONNECTION; + report_open_failed = true; + open_failed_status = param->dscp.status; break; } + + if (conn_item->state != CONN_FLOW_WAIT_DSCP) { + ESP_LOGW(TAG, "Unexpected flow state for GET_DSCP: %s", conn_flow_state_str(conn_item->state)); + param->dscp.status = ESP_HIDH_NO_CONNECTION; + report_open_failed = true; + open_failed_status = param->dscp.status; + break; + } + // check if connected esp_hidh_dev_lock(dev); if (!dev->connected) { esp_hidh_dev_unlock(dev); ESP_LOGE(TAG, "Connection has been released!"); param->dscp.status = ESP_HIDH_NO_CONNECTION; + report_open_failed = true; + open_failed_status = param->dscp.status; break; } // check if get descriptor failed if (param->dscp.status != ESP_HIDH_OK) { esp_hidh_dev_unlock(dev); ESP_LOGE(TAG, "GET_DSCP ERROR: %s", s_esp_hh_status_names[param->dscp.status]); + report_open_failed = true; + open_failed_status = param->dscp.status; break; } + dev->added = param->dscp.added; dev->config.product_id = param->dscp.product_id; dev->config.vendor_id = param->dscp.vendor_id; @@ -377,25 +442,40 @@ static void esp_hh_cb(esp_hidh_cb_event_t event, esp_hidh_cb_param_t *param) } } esp_hidh_dev_unlock(dev); - } while (0); - if (param->dscp.status != ESP_HIDH_OK) { - open_failed_cb(dev, param->dscp.status, &p, event_data_size); - } + if (param->dscp.status == ESP_HIDH_OK && !param->dscp.added) { + // New device path: lower layer has called AddDev, next stage waits ADD_DEV. + conn_item->state = CONN_FLOW_WAIT_ADD_DEV; + } + } while (0); if (dev != NULL) { esp_hidh_dev_lock(dev); dev->status = param->dscp.status; - // if has been added by lower layer, tell up layer if (dev->status == ESP_HIDH_OK && dev->connected && dev->added) { - p.open.status = bt_hidh_get_status(ESP_HIDH_OK); - p.open.dev = dev; - esp_hidh_dev_unlock(dev); - esp_event_post_to(hidh_local_param.event_loop_handle, ESP_HIDH_EVENTS, ESP_HIDH_OPEN_EVENT, &p, - event_data_size, portMAX_DELAY); - } else { - esp_hidh_dev_unlock(dev); + // Device is already added, no ADD_DEV_EVT will follow. Report OPEN here. + post_open = true; + } else if (dev->status != ESP_HIDH_OK) { + report_open_failed = true; + open_failed_status = dev->status; + } else if (dev->added && !dev->connected) { + report_open_failed = true; + open_failed_status = ESP_HIDH_NO_CONNECTION; } + esp_hidh_dev_unlock(dev); + } + + if (param->dscp.status != ESP_HIDH_OK || param->dscp.added) { + remove_conn_item(conn_item); + } + + if (report_open_failed) { + open_failed_cb(dev, open_failed_status, &p, event_data_size); + } else if (post_open) { + p.open.status = bt_hidh_get_status(ESP_HIDH_OK); + p.open.dev = dev; + esp_event_post_to(hidh_local_param.event_loop_handle, ESP_HIDH_EVENTS, ESP_HIDH_OPEN_EVENT, &p, + event_data_size, portMAX_DELAY); } break; } @@ -403,34 +483,51 @@ static void esp_hh_cb(esp_hidh_cb_event_t event, esp_hidh_cb_param_t *param) ESP_LOGV(TAG, "ADD_DEV: BDA: " ESP_BD_ADDR_STR ", handle: %d, status: %s", ESP_BD_ADDR_HEX(param->add_dev.bd_addr), param->add_dev.handle, s_esp_hh_status_names[param->add_dev.status]); - do { - dev = esp_hidh_dev_get_by_handle(param->add_dev.handle); - if (dev == NULL) { - ESP_LOGE(TAG, "Device Not Found"); - param->add_dev.status = ESP_HIDH_NO_CONNECTION; - break; - } - esp_hidh_dev_lock(dev); - dev->added = param->add_dev.status == ESP_HIDH_OK ? true : false; - esp_hidh_dev_unlock(dev); - } while (0); - if (param->add_dev.status != ESP_HIDH_OK) { - ESP_LOGE(TAG, "ADD_DEV ERROR: %s", s_esp_hh_status_names[param->add_dev.status]); - open_failed_cb(dev, param->add_dev.status, &p, event_data_size); + conn_item = find_conn_item_by_handle(param->add_dev.handle); + if (!conn_item) { + // Non-connect flow (e.g. loading bonded devices from NVS after enable). + ESP_LOGW(TAG, "ADD_DEV without pending connection (handle %d), ignored", param->add_dev.handle); + break; } - if (dev != NULL) { - esp_hidh_dev_lock(dev); - dev->status = param->add_dev.status; - if (dev->status == ESP_HIDH_OK && dev->connected && dev->added) { - p.open.status = bt_hidh_get_status(ESP_HIDH_OK); - p.open.dev = dev; - esp_hidh_dev_unlock(dev); - esp_event_post_to(hidh_local_param.event_loop_handle, ESP_HIDH_EVENTS, ESP_HIDH_OPEN_EVENT, &p, - event_data_size, portMAX_DELAY); - } else { - esp_hidh_dev_unlock(dev); + + dev = conn_item->dev; + if (conn_item->state != CONN_FLOW_WAIT_ADD_DEV) { + ESP_LOGW(TAG, "Unexpected flow state for ADD_DEV: %s", conn_flow_state_str(conn_item->state)); + // Drop the stale entry so it can't block subsequent connections, and make + // sure the application is notified instead of silently hanging. + remove_conn_item(conn_item); + if (dev != NULL && esp_hidh_dev_exists(dev)) { + open_failed_cb(dev, ESP_HIDH_NO_CONNECTION, &p, event_data_size); } + break; + } + + remove_conn_item(conn_item); + if (dev == NULL || !esp_hidh_dev_exists(dev)) { + ESP_LOGE(TAG, "Device Not Found"); + break; + } + + int status = param->add_dev.status; + bool connected = false; + bool added = false; + + esp_hidh_dev_lock(dev); + dev->status = status; + dev->added = (status == ESP_HIDH_OK); + connected = dev->connected; + added = dev->added; + esp_hidh_dev_unlock(dev); + + if (status == ESP_HIDH_OK && connected && added) { + p.open.status = bt_hidh_get_status(ESP_HIDH_OK); + p.open.dev = dev; + esp_event_post_to(hidh_local_param.event_loop_handle, ESP_HIDH_EVENTS, ESP_HIDH_OPEN_EVENT, &p, + event_data_size, portMAX_DELAY); + } else { + ESP_LOGE(TAG, "ADD_DEV ERROR: %s connected:%d added:%d", s_esp_hh_status_names[status], connected, added); + open_failed_cb(dev, status != ESP_HIDH_OK ? status : ESP_HIDH_NO_CONNECTION, &p, event_data_size); } break; } @@ -440,6 +537,13 @@ static void esp_hh_cb(esp_hidh_cb_event_t event, esp_hidh_cb_param_t *param) break; } ESP_LOGV(TAG, "CLOSE: handle: %d, status: %s", param->close.handle, s_esp_hh_status_names[param->close.status]); + // Drop any still-pending connection entry for this handle. If the link is torn + // down mid-flow (WAIT_DSCP / WAIT_ADD_DEV), the device itself is freed via the + // CLOSE_EVENT wrapper below; the conn_item must be removed here to avoid a leak + // and a dangling conn_item->dev that could mis-fire a later same-handle connection. + // No OPEN failure is posted here on purpose: CLOSE_EVENT already frees the device, + // and posting OPEN would free the same device twice. + remove_conn_item(find_conn_item_by_handle(param->close.handle)); do { dev = esp_hidh_dev_get_by_handle(param->close.handle); if (dev == NULL) { @@ -809,7 +913,7 @@ static esp_err_t esp_bt_hidh_dev_set_report(esp_hidh_dev_t *dev, size_t map_inde esp_hidh_dev_report_t *report = NULL; do { if (!is_trans_done(dev)) { - ESP_LOGE(TAG, "Pending previous tansaction %s done, try later!", get_trans_type_str(dev->trans_type)); + ESP_LOGE(TAG, "Pending previous transaction %s done, try later!", get_trans_type_str(dev->trans_type)); ret = ESP_FAIL; break; } @@ -859,7 +963,7 @@ static esp_err_t esp_bt_hidh_dev_report_read(esp_hidh_dev_t *dev, size_t map_ind esp_hidh_dev_report_t *report = NULL; do { if (!is_trans_done(dev)) { - ESP_LOGE(TAG, "Pending previous tansaction %s done, try later!", get_trans_type_str(dev->trans_type)); + ESP_LOGE(TAG, "Pending previous transaction %s done, try later!", get_trans_type_str(dev->trans_type)); ret = ESP_FAIL; break; } @@ -885,7 +989,7 @@ static esp_err_t esp_bt_hidh_dev_get_idle(esp_hidh_dev_t *dev) esp_err_t ret = ESP_OK; do { if (!is_trans_done(dev)) { - ESP_LOGE(TAG, "Pending previous tansaction %s done, try later!", get_trans_type_str(dev->trans_type)); + ESP_LOGE(TAG, "Pending previous transaction %s done, try later!", get_trans_type_str(dev->trans_type)); ret = ESP_FAIL; break; } @@ -908,7 +1012,7 @@ static esp_err_t esp_bt_hidh_dev_set_idle(esp_hidh_dev_t *dev, uint8_t idle_time esp_err_t ret = ESP_OK; do { if (!is_trans_done(dev)) { - ESP_LOGE(TAG, "Pending previous tansaction %s done, try later!", get_trans_type_str(dev->trans_type)); + ESP_LOGE(TAG, "Pending previous transaction %s done, try later!", get_trans_type_str(dev->trans_type)); ret = ESP_FAIL; break; } @@ -931,7 +1035,7 @@ static esp_err_t esp_bt_hidh_dev_get_protocol(esp_hidh_dev_t *dev) esp_err_t ret = ESP_OK; do { if (!is_trans_done(dev)) { - ESP_LOGE(TAG, "Pending previous tansaction %s done, try later!", get_trans_type_str(dev->trans_type)); + ESP_LOGE(TAG, "Pending previous transaction %s done, try later!", get_trans_type_str(dev->trans_type)); ret = ESP_FAIL; break; } @@ -955,7 +1059,7 @@ static esp_err_t esp_bt_hidh_dev_set_protocol(esp_hidh_dev_t *dev, uint8_t proto do { if (!is_trans_done(dev)) { - ESP_LOGE(TAG, "Pending previous tansaction %s done, try later!", get_trans_type_str(dev->trans_type)); + ESP_LOGE(TAG, "Pending previous transaction %s done, try later!", get_trans_type_str(dev->trans_type)); ret = ESP_FAIL; break; } @@ -993,7 +1097,7 @@ esp_err_t esp_bt_hidh_init(const esp_hidh_config_t *config) esp_err_t ret = ESP_OK; ESP_RETURN_ON_FALSE(config, ESP_ERR_INVALID_ARG, TAG, "Config is NULL"); - hidh_local_param.connection_queue = fixed_queue_new(QUEUE_SIZE_MAX); + hidh_local_param.connection_queue = list_new(free); ESP_RETURN_ON_FALSE(hidh_local_param.connection_queue, ESP_ERR_NO_MEM, TAG, "Alloc failed"); hidh_local_param.event_loop_handle = esp_hidh_get_event_loop(); diff --git a/examples/bluetooth/esp_hid_host/main/esp_hid_gap.c b/examples/bluetooth/esp_hid_host/main/esp_hid_gap.c index a792ff5ec23..5fd7ec6004f 100644 --- a/examples/bluetooth/esp_hid_host/main/esp_hid_gap.c +++ b/examples/bluetooth/esp_hid_host/main/esp_hid_gap.c @@ -307,6 +307,7 @@ void print_uuid(esp_bt_uuid_t *uuid) uuid->uuid.uuid128[13], uuid->uuid.uuid128[14], uuid->uuid.uuid128[15]); } } +#endif /* !CONFIG_BT_NIMBLE_ENABLED */ #if CONFIG_BT_HID_HOST_ENABLED static void handle_bt_device_result(struct disc_res_param *disc_res) @@ -654,7 +655,6 @@ static void ble_gap_event_handler(esp_gap_ble_cb_event_t event, esp_ble_gap_cb_p break; } } -#endif static esp_err_t init_ble_gap(void) { From 4412ff55b21f948ad8bd458f4fefd044a75e5ca9 Mon Sep 17 00:00:00 2001 From: liqigan Date: Thu, 9 Jul 2026 19:24:04 +0800 Subject: [PATCH 4/7] fix(bt/bluedroid): Fixed use after free issue on osi_event_delete --- components/bt/common/btc/core/btc_task.c | 18 + components/bt/common/osi/include/osi/thread.h | 3 + components/bt/common/osi/thread.c | 398 ++++++++++++++++-- 3 files changed, 375 insertions(+), 44 deletions(-) diff --git a/components/bt/common/btc/core/btc_task.c b/components/bt/common/btc/core/btc_task.c index 2320bf4f85b..9ca3fe87e58 100644 --- a/components/bt/common/btc/core/btc_task.c +++ b/components/bt/common/btc/core/btc_task.c @@ -553,14 +553,26 @@ error_exit:; bt_status_t btc_init(void) { const size_t workqueue_len[] = {BTC_TASK_WORKQUEUE0_LEN, BTC_TASK_WORKQUEUE1_LEN}; + + /* The osi_event subsystem must be ready before any osi_event_create() + * (e.g. btc_gap_ble_init() below). It cannot live in osi_init(), which + * runs later in the BTC task via bte_main_boot_entry(). */ + if (osi_thread_event_init() != 0) { + return BT_STATUS_NOMEM; + } + btc_thread = osi_thread_create(BTC_TASK_NAME, BTC_TASK_STACK_SIZE, BTC_TASK_PRIO, BTC_TASK_PINNED_TO_CORE, BTC_TASK_WORKQUEUE_NUM, workqueue_len, false); if (btc_thread == NULL) { + osi_thread_event_deinit(); return BT_STATUS_NOMEM; } #if BTC_DYNAMIC_MEMORY if (btc_init_mem() != BT_STATUS_SUCCESS){ + osi_thread_free(btc_thread); + btc_thread = NULL; + osi_thread_event_deinit(); return BT_STATUS_NOMEM; } #endif @@ -610,6 +622,12 @@ void btc_deinit(void) osi_thread_free(btc_thread); btc_thread = NULL; + + /* Tear down the osi_event subsystem last: btc_gap_ble_deinit() above may + * still call osi_event_delete(), which needs the global event lock. This + * mirrors moving osi_thread_event_init() into btc_init(); osi_deinit() + * (run earlier via bte_main_shutdown()) no longer owns this lifecycle. */ + osi_thread_event_deinit(); } int get_btc_work_queue_size(void) diff --git a/components/bt/common/osi/include/osi/thread.h b/components/bt/common/osi/include/osi/thread.h index eb72d94e26d..a4038bf230a 100644 --- a/components/bt/common/osi/include/osi/thread.h +++ b/components/bt/common/osi/include/osi/thread.h @@ -118,4 +118,7 @@ void osi_event_delete(struct osi_event* event); */ bool osi_thread_post_event(struct osi_event *event, uint32_t timeout); +int osi_thread_event_init(void); +void osi_thread_event_deinit(void); + #endif /* __THREAD_H__ */ diff --git a/components/bt/common/osi/thread.c b/components/bt/common/osi/thread.c index 1b5bd8b00e5..ec409c634d1 100644 --- a/components/bt/common/osi/thread.c +++ b/components/bt/common/osi/thread.c @@ -18,12 +18,13 @@ #include -#include "osi/allocator.h" #include "freertos/FreeRTOS.h" #include "freertos/queue.h" +#include "osi/allocator.h" +#include "osi/list.h" +#include "osi/mutex.h" #include "osi/semaphore.h" #include "osi/thread.h" -#include "osi/mutex.h" struct work_item { osi_thread_func_t func; @@ -58,17 +59,43 @@ struct osi_thread_start_arg { struct osi_event { struct work_item item; osi_mutex_t lock; - uint16_t is_queued; - uint16_t queue_idx; osi_thread_t *thread; + size_t ref_count; + uint8_t flags; + uint8_t queue_idx; }; +#define OSI_EVENT_FLAG_QUEUED (1U << 0) +#define OSI_EVENT_FLAG_POSTING (1U << 1) +#define OSI_EVENT_FLAG_DELETING (1U << 2) +#define OSI_EVENT_FLAG_RUNNING (1U << 3) + +#define OSI_EVENT_HAS_FLAG(event, flag) (((event)->flags & (flag)) != 0) +#define OSI_EVENT_SET_FLAG(event, flag) ((event)->flags |= (uint8_t)(flag)) +#define OSI_EVENT_CLEAR_FLAG(event, flag) ((event)->flags &= (uint8_t)(~(flag))) + static const size_t DEFAULT_WORK_QUEUE_CAPACITY = 100; +static list_t *s_osi_event_list; +static osi_mutex_t s_osi_event_lock; #if OSI_THREAD_DEBUG static void osi_thread_run_item(osi_thread_t *thread, int wq_idx, struct work_item *item); #endif +static void osi_thread_generic_event_handler(void *context); +static void osi_thread_generic_event_drain(void *context); + +static void osi_event_lock(void) +{ + assert(s_osi_event_lock != NULL); + osi_mutex_lock(&s_osi_event_lock, OSI_MUTEX_MAX_TIMEOUT); +} + +static void osi_event_unlock(void) +{ + osi_mutex_unlock(&s_osi_event_lock); +} + static struct work_queue *osi_work_queue_create(size_t capacity) { if (capacity == 0) { @@ -353,6 +380,38 @@ void osi_thread_free(osi_thread_t *thread) osi_thread_stop(thread); + /* The thread has stopped, so any work items still queued will never be + * drained by osi_thread_run. We must reclaim them here before the queues + * are destroyed, but we MUST NOT blindly execute their handlers: + * + * - Event work items (func == osi_thread_generic_event_handler) hold a + * reference on their osi_event. If that reference is never released the + * osi_event leaks. We reclaim it via osi_thread_generic_event_drain(), + * which only drops the queued reference (and frees the event if it was + * already deleted) WITHOUT running the user callback. This is safe on + * every shutdown path, including ones where the osi_event subsystem has + * already been torn down (osi_thread_event_deinit() freed + * s_osi_event_lock): the release path uses the per-event lock only and + * never touches the global s_osi_event_lock. + * + * - Any other work item was posted directly via osi_thread_post() with an + * arbitrary handler (e.g. btu_hci_msg_process, bta_sys_event, alarm + * handlers). Running such a handler here would dispatch into + * protocol-stack state (L2CAP/BTA/...) that may already have been freed + * by the caller before osi_thread_free() (e.g. BTU_ShutDown() calls + * btu_task_shut_down() first), turning the drain into a use-after-free. + * These items are therefore discarded, matching the pre-existing + * behavior where a destroyed queue silently dropped its contents. */ + for (int i = 0; i < thread->work_queue_num; i++) { + struct work_item item; + while (thread->work_queues[i] && + osi_thead_work_queue_get(thread->work_queues[i], &item) == true) { + if (item.func == osi_thread_generic_event_handler) { + osi_thread_generic_event_drain(item.context); + } + } + } + for (int i = 0; i < thread->work_queue_num; i++) { if (thread->work_queues[i]) { osi_work_queue_delete(thread->work_queues[i]); @@ -435,22 +494,13 @@ int osi_thread_queue_wait_size(osi_thread_t *thread, int wq_idx) } -struct osi_event *osi_event_create(osi_thread_func_t func, void *context) +static bool osi_event_add_alive_locked(struct osi_event *event) { - struct osi_event *event = osi_calloc(sizeof(struct osi_event)); - if (event != NULL) { - if (osi_mutex_new(&event->lock) == 0) { - event->item.func = func; - event->item.context = context; - return event; - } - osi_free(event); - } - - return NULL; + assert(s_osi_event_list != NULL); + return list_append(s_osi_event_list, event); } -void osi_event_delete(struct osi_event* event) +static void osi_event_free(struct osi_event *event) { if (event != NULL) { osi_mutex_free(&event->lock); @@ -459,59 +509,319 @@ void osi_event_delete(struct osi_event* event) } } -bool osi_event_bind(struct osi_event* event, osi_thread_t *thread, int queue_idx) +static bool osi_event_is_idle(const struct osi_event *event) { - if (event == NULL || event->thread != NULL) { + return !OSI_EVENT_HAS_FLAG(event, OSI_EVENT_FLAG_QUEUED) && + !OSI_EVENT_HAS_FLAG(event, OSI_EVENT_FLAG_POSTING) && + !OSI_EVENT_HAS_FLAG(event, OSI_EVENT_FLAG_RUNNING); +} + +static bool osi_event_should_free(const struct osi_event *event) +{ + return OSI_EVENT_HAS_FLAG(event, OSI_EVENT_FLAG_DELETING) && + osi_event_is_idle(event); +} + +static bool osi_event_can_bind_locked(const struct osi_event *event, osi_thread_t *thread, int queue_idx) +{ + return !OSI_EVENT_HAS_FLAG(event, OSI_EVENT_FLAG_DELETING) && + event->thread == NULL && + thread != NULL && + queue_idx >= 0 && + queue_idx < thread->work_queue_num; +} + +static bool osi_event_can_post_locked(const struct osi_event *event) +{ + if (event->thread == NULL || event->queue_idx >= event->thread->work_queue_num) { return false; } - if (thread == NULL || queue_idx >= thread->work_queue_num) { + return !OSI_EVENT_HAS_FLAG(event, OSI_EVENT_FLAG_DELETING) && + event->item.func != NULL && + !OSI_EVENT_HAS_FLAG(event, OSI_EVENT_FLAG_QUEUED) && + !OSI_EVENT_HAS_FLAG(event, OSI_EVENT_FLAG_POSTING); +} + +static bool osi_event_is_alive_locked(const struct osi_event *event) +{ + /* Do not dereference event here: callers may pass a stale pointer racing + * with osi_event_delete(). The alive list is the ownership boundary. */ + return s_osi_event_list != NULL && list_contains(s_osi_event_list, event); +} + +static bool osi_event_remove_alive_locked(struct osi_event *event) +{ + bool removed = false; + + if (s_osi_event_list == NULL) { return false; } - event->thread = thread; - event->queue_idx = queue_idx; + removed = list_delete(s_osi_event_list, event); + return removed; +} - return true; +struct osi_event *osi_event_create(osi_thread_func_t func, void *context) +{ + bool added = false; + struct osi_event *event = osi_calloc(sizeof(struct osi_event)); + + if (event == NULL) { + return NULL; + } + + if (osi_mutex_new(&event->lock) != 0) { + osi_free(event); + return NULL; + } + + event->item.func = func; + event->item.context = context; + event->ref_count = 1; + + osi_event_lock(); + added = osi_event_add_alive_locked(event); + osi_event_unlock(); + if (added) { + return event; + } + + osi_mutex_free(&event->lock); + osi_free(event); + return NULL; +} + +/* ref_count is protected by the per-event lock (event->lock), NOT the global + * s_osi_event_lock. This keeps the reference-release path independent of the + * global event subsystem: it must stay valid even after + * osi_thread_event_deinit() has freed s_osi_event_lock (e.g. when a thread is + * freed on a shutdown path that tears the event subsystem down first). The + * global lock is only used to gate the alive-list membership that decides + * whether a new reference may be acquired. */ +static bool osi_event_acquire(struct osi_event *event) +{ + bool acquired = false; + + if (event == NULL) { + return false; + } + + /* Hold the global lock so the event cannot be removed from the alive list + * (and thus cannot be freed) while we take a fresh reference. Nesting is + * always global-lock-outer, event->lock-inner; no path takes them in the + * reverse order, so this cannot deadlock. */ + osi_event_lock(); + if (osi_event_is_alive_locked(event)) { + osi_mutex_lock(&event->lock, OSI_MUTEX_MAX_TIMEOUT); + assert(event->ref_count > 0); + event->ref_count++; + osi_mutex_unlock(&event->lock); + acquired = true; + } + osi_event_unlock(); + + return acquired; +} + +static void osi_event_retain(struct osi_event *event) +{ + osi_mutex_lock(&event->lock, OSI_MUTEX_MAX_TIMEOUT); + assert(event->ref_count > 0); + event->ref_count++; + osi_mutex_unlock(&event->lock); +} + +static void osi_event_release(struct osi_event *event) +{ + bool should_free = false; + + osi_mutex_lock(&event->lock, OSI_MUTEX_MAX_TIMEOUT); + assert(event->ref_count > 0); + event->ref_count--; + if (event->ref_count == 0) { + should_free = osi_event_should_free(event); + } + osi_mutex_unlock(&event->lock); + + if (should_free) { + osi_event_free(event); + } +} + +void osi_event_delete(struct osi_event *event) +{ + bool removed = false; + + if (event == NULL) { + return; + } + + osi_event_lock(); + removed = osi_event_remove_alive_locked(event); + osi_event_unlock(); + if (!removed) { + return; + } + + osi_mutex_lock(&event->lock, OSI_MUTEX_MAX_TIMEOUT); + OSI_EVENT_SET_FLAG(event, OSI_EVENT_FLAG_DELETING); + event->item.func = NULL; + event->item.context = NULL; + osi_mutex_unlock(&event->lock); + + osi_event_release(event); +} + +bool osi_event_bind(struct osi_event *event, osi_thread_t *thread, int queue_idx) +{ + bool ret = false; + + if (!osi_event_acquire(event)) { + return false; + } + + osi_mutex_lock(&event->lock, OSI_MUTEX_MAX_TIMEOUT); + if (osi_event_can_bind_locked(event, thread, queue_idx)) { + event->thread = thread; + event->queue_idx = queue_idx; + ret = true; + } + osi_mutex_unlock(&event->lock); + + osi_event_release(event); + + return ret; } static void osi_thread_generic_event_handler(void *context) { struct osi_event *event = (struct osi_event *)context; - if (event != NULL && event->item.func != NULL) { - osi_mutex_lock(&event->lock, OSI_MUTEX_MAX_TIMEOUT); - event->is_queued = 0; - osi_mutex_unlock(&event->lock); - event->item.func(event->item.context); + osi_thread_func_t func = NULL; + void *func_context = NULL; + + if (event == NULL) { + return; } + + osi_mutex_lock(&event->lock, OSI_MUTEX_MAX_TIMEOUT); + OSI_EVENT_CLEAR_FLAG(event, OSI_EVENT_FLAG_QUEUED); + if (OSI_EVENT_HAS_FLAG(event, OSI_EVENT_FLAG_DELETING)) { + osi_mutex_unlock(&event->lock); + osi_event_release(event); + return; + } + OSI_EVENT_SET_FLAG(event, OSI_EVENT_FLAG_RUNNING); + func = event->item.func; + func_context = event->item.context; + osi_mutex_unlock(&event->lock); + + if (func != NULL) { + func(func_context); + } + + osi_mutex_lock(&event->lock, OSI_MUTEX_MAX_TIMEOUT); + OSI_EVENT_CLEAR_FLAG(event, OSI_EVENT_FLAG_RUNNING); + osi_mutex_unlock(&event->lock); + + osi_event_release(event); +} + +/* Reclaim a queued event work item during thread teardown WITHOUT invoking the + * user callback. It only clears the QUEUED flag and drops the reference the + * queued item owns (osi_thread_post_event() retained it); this frees the event + * if it was already deleted, and leaves a still-live event untouched. Unlike + * osi_thread_generic_event_handler(), it never dispatches into stack state that + * may already have been freed on the shutdown path. */ +static void osi_thread_generic_event_drain(void *context) +{ + struct osi_event *event = (struct osi_event *)context; + + if (event == NULL) { + return; + } + + osi_mutex_lock(&event->lock, OSI_MUTEX_MAX_TIMEOUT); + OSI_EVENT_CLEAR_FLAG(event, OSI_EVENT_FLAG_QUEUED); + osi_mutex_unlock(&event->lock); + + osi_event_release(event); } bool osi_thread_post_event(struct osi_event *event, uint32_t timeout) { - assert(event != NULL && event->thread != NULL); - assert(event->queue_idx >= 0 && event->queue_idx < event->thread->work_queue_num); bool ret = false; - if (event->is_queued == 0) { - uint16_t acquire_cnt = 0; - osi_mutex_lock(&event->lock, OSI_MUTEX_MAX_TIMEOUT); - event->is_queued += 1; - acquire_cnt = event->is_queued; - osi_mutex_unlock(&event->lock); + osi_thread_t *thread = NULL; + uint8_t queue_idx = 0; - if (acquire_cnt == 1) { - ret = osi_thread_post(event->thread, osi_thread_generic_event_handler, event, event->queue_idx, timeout); - if (!ret) { - // clear "is_queued" when post failure, to allow for following event posts - osi_mutex_lock(&event->lock, OSI_MUTEX_MAX_TIMEOUT); - event->is_queued = 0; - osi_mutex_unlock(&event->lock); - } + if (!osi_event_acquire(event)) { + return false; + } + + osi_mutex_lock(&event->lock, OSI_MUTEX_MAX_TIMEOUT); + if (!osi_event_can_post_locked(event)) { + osi_mutex_unlock(&event->lock); + osi_event_release(event); + return false; + } + OSI_EVENT_SET_FLAG(event, OSI_EVENT_FLAG_QUEUED); + OSI_EVENT_SET_FLAG(event, OSI_EVENT_FLAG_POSTING); + thread = event->thread; + queue_idx = event->queue_idx; + osi_mutex_unlock(&event->lock); + + /* The queued work item owns a reference until the generic handler drains it. */ + osi_event_retain(event); + ret = osi_thread_post(thread, osi_thread_generic_event_handler, event, queue_idx, timeout); + osi_mutex_lock(&event->lock, OSI_MUTEX_MAX_TIMEOUT); + OSI_EVENT_CLEAR_FLAG(event, OSI_EVENT_FLAG_POSTING); + if (!ret) { + // clear "is_queued" when post failure, to allow for following event posts + OSI_EVENT_CLEAR_FLAG(event, OSI_EVENT_FLAG_QUEUED); + } + osi_mutex_unlock(&event->lock); + + if (!ret) { + osi_event_release(event); + } + osi_event_release(event); + + return ret; +} + +int osi_thread_event_init(void) +{ + int ret = -1; + + do { + if (osi_mutex_new(&s_osi_event_lock) != 0) { + break; } + + s_osi_event_list = list_new(NULL); + if (s_osi_event_list == NULL) { + break; + } + + ret = 0; + } while (0); + + if (ret != 0) { + osi_thread_event_deinit(); } return ret; } +void osi_thread_event_deinit(void) +{ + if (s_osi_event_list != NULL) { + list_free(s_osi_event_list); + s_osi_event_list = NULL; + } + osi_mutex_free(&s_osi_event_lock); +} + #if OSI_THREAD_DEBUG static void osi_thread_run_item(osi_thread_t *thread, int wq_idx, struct work_item *item) { From ad5a7d778e9bd7f0a98e7800beb0d7073b88ec66 Mon Sep 17 00:00:00 2001 From: zhanghaipeng Date: Tue, 18 Aug 2026 15:23:02 +0800 Subject: [PATCH 5/7] fix(ble/bluedroid): allow osi_event re-post during POSTING window Do not reject osi_thread_post_event() when only POSTING is set. QUEUED already prevents double-queueing; rejecting POSTING caused HCI downstream lost wakeup. Add generic osi_event and hci downstream diagnostics for post failures. --- components/bt/common/osi/thread.c | 31 +++++++++++++++++--- components/bt/host/bluedroid/hci/hci_layer.c | 19 +++++++++++- 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/components/bt/common/osi/thread.c b/components/bt/common/osi/thread.c index ec409c634d1..cd8294b7832 100644 --- a/components/bt/common/osi/thread.c +++ b/components/bt/common/osi/thread.c @@ -534,13 +534,30 @@ static bool osi_event_can_bind_locked(const struct osi_event *event, osi_thread_ static bool osi_event_can_post_locked(const struct osi_event *event) { if (event->thread == NULL || event->queue_idx >= event->thread->work_queue_num) { + OSI_TRACE_EVENT("%s deny ev=%p flags=0x%x qidx=%u", + __func__, event, event ? event->flags : 0, + event ? event->queue_idx : 0); return false; } - return !OSI_EVENT_HAS_FLAG(event, OSI_EVENT_FLAG_DELETING) && - event->item.func != NULL && - !OSI_EVENT_HAS_FLAG(event, OSI_EVENT_FLAG_QUEUED) && - !OSI_EVENT_HAS_FLAG(event, OSI_EVENT_FLAG_POSTING); + if (OSI_EVENT_HAS_FLAG(event, OSI_EVENT_FLAG_DELETING) || + event->item.func == NULL || + OSI_EVENT_HAS_FLAG(event, OSI_EVENT_FLAG_QUEUED)) { + OSI_TRACE_EVENT("%s deny ev=%p flags=0x%x qidx=%u wq_len=%d", + __func__, event, event->flags, event->queue_idx, + osi_thread_queue_wait_size(event->thread, event->queue_idx)); + return false; + } + + /* Do NOT gate on OSI_EVENT_FLAG_POSTING here. POSTING marks the window in + * osi_thread_post_event() between osi_thread_post() (enqueue) and the + * poster clearing the flag. During that window the generic event handler + * may already have run and cleared QUEUED. A concurrent post that arrives + * after QUEUED is cleared is a legitimate re-post (new work arrived while + * the handler was draining) and must be accepted; rejecting it causes a + * lost wakeup. QUEUED alone prevents genuine double-queueing. POSTING is + * retained only for osi_event_is_idle()/osi_event_should_free(). */ + return true; } static bool osi_event_is_alive_locked(const struct osi_event *event) @@ -714,6 +731,7 @@ static void osi_thread_generic_event_handler(void *context) OSI_EVENT_SET_FLAG(event, OSI_EVENT_FLAG_RUNNING); func = event->item.func; func_context = event->item.context; + OSI_TRACE_DEBUG("%s enter ev=%p flags=0x%x", __func__, event, event->flags); osi_mutex_unlock(&event->lock); if (func != NULL) { @@ -722,6 +740,7 @@ static void osi_thread_generic_event_handler(void *context) osi_mutex_lock(&event->lock, OSI_MUTEX_MAX_TIMEOUT); OSI_EVENT_CLEAR_FLAG(event, OSI_EVENT_FLAG_RUNNING); + OSI_TRACE_DEBUG("%s exit ev=%p flags=0x%x", __func__, event, event->flags); osi_mutex_unlock(&event->lock); osi_event_release(event); @@ -755,6 +774,7 @@ bool osi_thread_post_event(struct osi_event *event, uint32_t timeout) uint8_t queue_idx = 0; if (!osi_event_acquire(event)) { + OSI_TRACE_EVENT("%s acquire fail ev=%p", __func__, event); return false; } @@ -782,6 +802,9 @@ bool osi_thread_post_event(struct osi_event *event, uint32_t timeout) osi_mutex_unlock(&event->lock); if (!ret) { + OSI_TRACE_EVENT("%s enqueue fail ev=%p qidx=%u wq_len=%d", + __func__, event, queue_idx, + osi_thread_queue_wait_size(thread, queue_idx)); osi_event_release(event); } osi_event_release(event); diff --git a/components/bt/host/bluedroid/hci/hci_layer.c b/components/bt/host/bluedroid/hci/hci_layer.c index cb6bf9b6a4d..5f908b2ea09 100644 --- a/components/bt/host/bluedroid/hci/hci_layer.c +++ b/components/bt/host/bluedroid/hci/hci_layer.c @@ -155,11 +155,21 @@ void hci_shut_down(void) bool hci_downstream_data_post(uint32_t timeout) { + bool ret; + if (hci_host_env.downstream_data_ready == NULL) { HCI_TRACE_WARNING("%s downstream_data_ready event not created", __func__); return false; } - return osi_thread_post_event(hci_host_env.downstream_data_ready, timeout); + + ret = osi_thread_post_event(hci_host_env.downstream_data_ready, timeout); + if (!ret) { + HCI_TRACE_DEBUG("%s post fail credits=%d cmdq=%u pktq=%u", + __func__, hci_host_env.command_credits, + (unsigned)fixed_pkt_queue_length(hci_host_env.command_queue), + (unsigned)fixed_queue_length(hci_host_env.packet_queue)); + } + return ret; } static int hci_layer_init_env(void) @@ -249,6 +259,8 @@ static void hci_downstream_data_handler(void *arg) * All packets will be directly copied to single queue in driver layer with * H4 type header added (1 byte). */ + UNUSED(arg); + while (hci_host_check_send_available()) { /*Now Target only allowed one packet per TX*/ BT_HDR *pkt = packet_fragmenter->fragment_current_packet(); @@ -264,6 +276,11 @@ static void hci_downstream_data_handler(void *arg) break; } } + + HCI_TRACE_DEBUG("%s done credits=%d cmdq=%u pktq=%u", + __func__, hci_host_env.command_credits, + (unsigned)fixed_pkt_queue_length(hci_host_env.command_queue), + (unsigned)fixed_queue_length(hci_host_env.packet_queue)); } static void transmit_command( From 6d23cb727867540203f88591368d58042f1a6757 Mon Sep 17 00:00:00 2001 From: zhanghaipeng Date: Tue, 18 Aug 2026 16:14:48 +0800 Subject: [PATCH 6/7] docs(bt): clarify Bluedroid GATT signed write API comments Correct misleading CHAR_PROP_BIT_AUTH wording and document CSRK-based signed write usage for server permissions and client write APIs. --- .../bt/host/bluedroid/api/include/api/esp_gatt_defs.h | 10 +++++----- .../bt/host/bluedroid/api/include/api/esp_gattc_api.h | 2 ++ 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/components/bt/host/bluedroid/api/include/api/esp_gatt_defs.h b/components/bt/host/bluedroid/api/include/api/esp_gatt_defs.h index 03ba9a01624..435ab210f94 100644 --- a/components/bt/host/bluedroid/api/include/api/esp_gatt_defs.h +++ b/components/bt/host/bluedroid/api/include/api/esp_gatt_defs.h @@ -375,8 +375,8 @@ typedef enum { ESP_GATT_AUTH_REQ_NONE = 0, /*!< No authentication required. Corresponds to BTA_GATT_AUTH_REQ_NONE. */ ESP_GATT_AUTH_REQ_NO_MITM = 1, /*!< Unauthenticated encryption. Corresponds to BTA_GATT_AUTH_REQ_NO_MITM. */ ESP_GATT_AUTH_REQ_MITM = 2, /*!< Authenticated encryption (MITM protection). Corresponds to BTA_GATT_AUTH_REQ_MITM. */ - ESP_GATT_AUTH_REQ_SIGNED_NO_MITM = 3, /*!< Signed data, no MITM protection. Corresponds to BTA_GATT_AUTH_REQ_SIGNED_NO_MITM. */ - ESP_GATT_AUTH_REQ_SIGNED_MITM = 4, /*!< Signed data with MITM protection. Corresponds to BTA_GATT_AUTH_REQ_SIGNED_MITM. */ + ESP_GATT_AUTH_REQ_SIGNED_NO_MITM = 3, /*!< CSRK signed write, no MITM. Use with `ESP_GATT_WRITE_TYPE_NO_RSP` on a bonded, unencrypted link. */ + ESP_GATT_AUTH_REQ_SIGNED_MITM = 4, /*!< CSRK signed write with MITM. Use with `ESP_GATT_WRITE_TYPE_NO_RSP` on a bonded, unencrypted link. */ } esp_gatt_auth_req_t; @@ -410,10 +410,10 @@ typedef enum { /** @brief Permission to write to the attribute with encrypted MITM protection. Corresponds to BTA_GATT_PERM_WRITE_ENC_MITM. */ #define ESP_GATT_PERM_WRITE_ENC_MITM (1 << 6) -/** @brief Permission for signed writes to the attribute. Corresponds to BTA_GATT_PERM_WRITE_SIGNED. */ +/** @brief Signed write without link encryption (CSRK). Requires `ESP_GATT_CHAR_PROP_BIT_AUTH`. Corresponds to BTA_GATT_PERM_WRITE_SIGNED. */ #define ESP_GATT_PERM_WRITE_SIGNED (1 << 7) -/** @brief Permission for signed writes to the attribute with MITM protection. Corresponds to BTA_GATT_PERM_WRITE_SIGNED_MITM. */ +/** @brief Signed write with MITM-protected CSRK. Requires `ESP_GATT_CHAR_PROP_BIT_AUTH`. Corresponds to BTA_GATT_PERM_WRITE_SIGNED_MITM. */ #define ESP_GATT_PERM_WRITE_SIGNED_MITM (1 << 8) /** @brief Permission to read the attribute with authorization. */ @@ -463,7 +463,7 @@ typedef uint16_t esp_gatt_perm_t; ///< Type to represent GATT attribute permissi /** @brief Ability to indicate.Corresponds to BTA_GATT_CHAR_PROP_BIT_INDICATE. */ #define ESP_GATT_CHAR_PROP_BIT_INDICATE (1 << 5) -/** @brief Ability to authenticate.Corresponds to BTA_GATT_CHAR_PROP_BIT_AUTH. */ +/** @brief Authenticated Signed Writes (ATT Signed Write Command, 0xD2). Requires matching `ESP_GATT_PERM_WRITE_SIGNED`. Corresponds to BTA_GATT_CHAR_PROP_BIT_AUTH. */ #define ESP_GATT_CHAR_PROP_BIT_AUTH (1 << 6) /** @brief Has extended properties.Corresponds to BTA_GATT_CHAR_PROP_BIT_EXT_PROP. */ diff --git a/components/bt/host/bluedroid/api/include/api/esp_gattc_api.h b/components/bt/host/bluedroid/api/include/api/esp_gattc_api.h index 800bcf5c768..d73d6e29922 100644 --- a/components/bt/host/bluedroid/api/include/api/esp_gattc_api.h +++ b/components/bt/host/bluedroid/api/include/api/esp_gattc_api.h @@ -911,6 +911,8 @@ esp_err_t esp_ble_gattc_read_char_descr (esp_gatt_if_t gattc_if, * 3. `handle` must be greater than 0. * 4. If `auth_req` is not `ESP_GATT_AUTH_REQ_NONE`, the stack may start encryption * or SMP pairing before sending the ATT write. + * 5. `ESP_GATT_AUTH_REQ_SIGNED_*` with `ESP_GATT_WRITE_TYPE_NO_RSP` sends ATT Signed + * Write Command when bonded (CSRK) and the link is not encrypted. * * @return * - ESP_OK: Success From 822f1617bfbe14f0a9fa3c1630488a79e12b6cd9 Mon Sep 17 00:00:00 2001 From: liqigan Date: Thu, 20 Aug 2026 16:21:37 +0800 Subject: [PATCH 7/7] change(bt/bluedroid): Used dynamic osi event to reduce lock cost --- components/bt/common/osi/include/osi/thread.h | 19 +- components/bt/common/osi/thread.c | 496 +++++++++------- .../bluedroid/btc/profile/std/hid/btc_hh.c | 18 +- .../btc/profile/std/include/btc_hh.h | 2 +- .../basic_unit_test/main/CMakeLists.txt | 1 + .../basic_unit_test/main/test_osi_event.c | 535 ++++++++++++++++++ 6 files changed, 859 insertions(+), 212 deletions(-) create mode 100644 components/bt/test_apps/basic_unit_test/main/test_osi_event.c diff --git a/components/bt/common/osi/include/osi/thread.h b/components/bt/common/osi/include/osi/thread.h index a4038bf230a..c0e90eec02f 100644 --- a/components/bt/common/osi/include/osi/thread.h +++ b/components/bt/common/osi/include/osi/thread.h @@ -19,6 +19,7 @@ struct osi_thread; struct osi_event; +struct osi_dynamic_event; typedef struct osi_thread osi_thread_t; @@ -81,10 +82,12 @@ const char *osi_thread_name(osi_thread_t *thread); int osi_thread_queue_wait_size(osi_thread_t *thread, int wq_idx); /* - * brief: Create an osi_event struct and register the handler function and its argument + * brief: Create a session-stable osi_event and register its handler and argument. * An osi_event is a kind of work that can be posted to the workqueue of osi_thread to process, * but the work can have at most one instance the thread workqueue before it is processed. This * allows the "single post, multiple data processing" jobs. + * Delete is logical: storage remains valid until osi_thread_event_deinit(), allowing stale + * posts during session teardown to be rejected without a global alive-list lock. * param func: the handler to process the job * param context: the argument to be passed to the handler function when the job is being processed * return: NULL if no memory, otherwise a valid struct pointer @@ -103,7 +106,7 @@ struct osi_event *osi_event_create(osi_thread_func_t func, void *context); bool osi_event_bind(struct osi_event* event, osi_thread_t *thread, int queue_idx); /* - * brief: Destroy the osi_event struct created by osi_event_create and free the allocated memory + * brief: Logically delete an osi_event. Its memory is reclaimed by osi_thread_event_deinit(). * param event: the pointer to osi_event */ void osi_event_delete(struct osi_event* event); @@ -118,6 +121,18 @@ void osi_event_delete(struct osi_event* event); */ bool osi_thread_post_event(struct osi_event *event, uint32_t timeout); +/* + * Dynamic events may be created and destroyed repeatedly during one + * osi_thread event-subsystem session. Unlike session-stable osi_event objects, + * their storage can be released by delete, so all operations use a separate + * API whose post/bind entry points validate the opaque pointer without first + * dereferencing it. + */ +struct osi_dynamic_event *osi_dynamic_event_create(osi_thread_func_t func, void *context); +bool osi_dynamic_event_bind(struct osi_dynamic_event *event, osi_thread_t *thread, int queue_idx); +bool osi_dynamic_event_post(struct osi_dynamic_event *event, uint32_t timeout); +void osi_dynamic_event_delete(struct osi_dynamic_event *event); + int osi_thread_event_init(void); void osi_thread_event_deinit(void); diff --git a/components/bt/common/osi/thread.c b/components/bt/common/osi/thread.c index cd8294b7832..efe9b9d5283 100644 --- a/components/bt/common/osi/thread.c +++ b/components/bt/common/osi/thread.c @@ -56,7 +56,7 @@ struct osi_thread_start_arg { int error; }; -struct osi_event { +struct osi_event_core { struct work_item item; osi_mutex_t lock; osi_thread_t *thread; @@ -65,17 +65,25 @@ struct osi_event { uint8_t queue_idx; }; +struct osi_event { + struct osi_event_core core; +}; + +struct osi_dynamic_event { + struct osi_event_core core; +}; + #define OSI_EVENT_FLAG_QUEUED (1U << 0) -#define OSI_EVENT_FLAG_POSTING (1U << 1) -#define OSI_EVENT_FLAG_DELETING (1U << 2) -#define OSI_EVENT_FLAG_RUNNING (1U << 3) +#define OSI_EVENT_FLAG_DELETING (1U << 1) +#define OSI_EVENT_FLAG_RUNNING (1U << 2) #define OSI_EVENT_HAS_FLAG(event, flag) (((event)->flags & (flag)) != 0) #define OSI_EVENT_SET_FLAG(event, flag) ((event)->flags |= (uint8_t)(flag)) #define OSI_EVENT_CLEAR_FLAG(event, flag) ((event)->flags &= (uint8_t)(~(flag))) static const size_t DEFAULT_WORK_QUEUE_CAPACITY = 100; -static list_t *s_osi_event_list; +static list_t *s_osi_session_event_list; +static list_t *s_osi_dynamic_event_list; static osi_mutex_t s_osi_event_lock; #if OSI_THREAD_DEBUG @@ -494,35 +502,47 @@ int osi_thread_queue_wait_size(osi_thread_t *thread, int wq_idx) } -static bool osi_event_add_alive_locked(struct osi_event *event) +static struct osi_event_core *osi_event_core_new(size_t size, osi_thread_func_t func, void *context) { - assert(s_osi_event_list != NULL); - return list_append(s_osi_event_list, event); + struct osi_event_core *event = osi_calloc(size); + + if (event == NULL) { + return NULL; + } + if (osi_mutex_new(&event->lock) != 0) { + osi_free(event); + return NULL; + } + + event->item.func = func; + event->item.context = context; + /* Session events hold a registry pin; dynamic events hold an owner ref. */ + event->ref_count = 1; + return event; } -static void osi_event_free(struct osi_event *event) +static void osi_event_core_free(struct osi_event_core *event) { if (event != NULL) { osi_mutex_free(&event->lock); - memset(event, 0, sizeof(struct osi_event)); + memset(event, 0, sizeof(*event)); osi_free(event); } } -static bool osi_event_is_idle(const struct osi_event *event) +static bool osi_event_is_idle(const struct osi_event_core *event) { return !OSI_EVENT_HAS_FLAG(event, OSI_EVENT_FLAG_QUEUED) && - !OSI_EVENT_HAS_FLAG(event, OSI_EVENT_FLAG_POSTING) && !OSI_EVENT_HAS_FLAG(event, OSI_EVENT_FLAG_RUNNING); } -static bool osi_event_should_free(const struct osi_event *event) +static bool osi_event_should_free(const struct osi_event_core *event) { return OSI_EVENT_HAS_FLAG(event, OSI_EVENT_FLAG_DELETING) && osi_event_is_idle(event); } -static bool osi_event_can_bind_locked(const struct osi_event *event, osi_thread_t *thread, int queue_idx) +static bool osi_event_can_bind_locked(const struct osi_event_core *event, osi_thread_t *thread, int queue_idx) { return !OSI_EVENT_HAS_FLAG(event, OSI_EVENT_FLAG_DELETING) && event->thread == NULL && @@ -531,7 +551,7 @@ static bool osi_event_can_bind_locked(const struct osi_event *event, osi_thread_ queue_idx < thread->work_queue_num; } -static bool osi_event_can_post_locked(const struct osi_event *event) +static bool osi_event_can_post_locked(const struct osi_event_core *event) { if (event->thread == NULL || event->queue_idx >= event->thread->work_queue_num) { OSI_TRACE_EVENT("%s deny ev=%p flags=0x%x qidx=%u", @@ -549,173 +569,188 @@ static bool osi_event_can_post_locked(const struct osi_event *event) return false; } - /* Do NOT gate on OSI_EVENT_FLAG_POSTING here. POSTING marks the window in - * osi_thread_post_event() between osi_thread_post() (enqueue) and the - * poster clearing the flag. During that window the generic event handler - * may already have run and cleared QUEUED. A concurrent post that arrives - * after QUEUED is cleared is a legitimate re-post (new work arrived while - * the handler was draining) and must be accepted; rejecting it causes a - * lost wakeup. QUEUED alone prevents genuine double-queueing. POSTING is - * retained only for osi_event_is_idle()/osi_event_should_free(). */ + /* QUEUED alone prevents double-queueing. Do not gate on RUNNING: the + * generic handler clears QUEUED before invoking the user callback, and a + * concurrent post after that is a legitimate re-post of work that arrived + * while the handler was draining. */ return true; } -static bool osi_event_is_alive_locked(const struct osi_event *event) +/* Caller holds event->lock. Drops one reference. Returns true if the caller + * must free the event AFTER unlocking; never destroy the mutex while held. */ +static bool osi_event_release_locked(struct osi_event_core *event) { - /* Do not dereference event here: callers may pass a stale pointer racing - * with osi_event_delete(). The alive list is the ownership boundary. */ - return s_osi_event_list != NULL && list_contains(s_osi_event_list, event); -} - -static bool osi_event_remove_alive_locked(struct osi_event *event) -{ - bool removed = false; - - if (s_osi_event_list == NULL) { + assert(event->ref_count > 0); + event->ref_count--; + if (event->ref_count != 0) { return false; } + return osi_event_should_free(event); +} - removed = list_delete(s_osi_event_list, event); - return removed; +static void osi_event_unlock_and_maybe_free(struct osi_event_core *event, bool should_free) +{ + osi_mutex_unlock(&event->lock); + if (should_free) { + osi_event_core_free(event); + } +} + +static void osi_event_mark_deleting_locked(struct osi_event_core *event) +{ + OSI_EVENT_SET_FLAG(event, OSI_EVENT_FLAG_DELETING); + event->item.func = NULL; + event->item.context = NULL; } struct osi_event *osi_event_create(osi_thread_func_t func, void *context) { + struct osi_event *event = (struct osi_event *)osi_event_core_new(sizeof(*event), func, context); bool added = false; - struct osi_event *event = osi_calloc(sizeof(struct osi_event)); if (event == NULL) { return NULL; } - if (osi_mutex_new(&event->lock) != 0) { - osi_free(event); + osi_event_lock(); + if (s_osi_session_event_list != NULL) { + added = list_append(s_osi_session_event_list, event); + } + osi_event_unlock(); + if (!added) { + osi_event_core_free(&event->core); return NULL; } - - event->item.func = func; - event->item.context = context; - event->ref_count = 1; - - osi_event_lock(); - added = osi_event_add_alive_locked(event); - osi_event_unlock(); - if (added) { - return event; - } - - osi_mutex_free(&event->lock); - osi_free(event); - return NULL; -} - -/* ref_count is protected by the per-event lock (event->lock), NOT the global - * s_osi_event_lock. This keeps the reference-release path independent of the - * global event subsystem: it must stay valid even after - * osi_thread_event_deinit() has freed s_osi_event_lock (e.g. when a thread is - * freed on a shutdown path that tears the event subsystem down first). The - * global lock is only used to gate the alive-list membership that decides - * whether a new reference may be acquired. */ -static bool osi_event_acquire(struct osi_event *event) -{ - bool acquired = false; - - if (event == NULL) { - return false; - } - - /* Hold the global lock so the event cannot be removed from the alive list - * (and thus cannot be freed) while we take a fresh reference. Nesting is - * always global-lock-outer, event->lock-inner; no path takes them in the - * reverse order, so this cannot deadlock. */ - osi_event_lock(); - if (osi_event_is_alive_locked(event)) { - osi_mutex_lock(&event->lock, OSI_MUTEX_MAX_TIMEOUT); - assert(event->ref_count > 0); - event->ref_count++; - osi_mutex_unlock(&event->lock); - acquired = true; - } - osi_event_unlock(); - - return acquired; -} - -static void osi_event_retain(struct osi_event *event) -{ - osi_mutex_lock(&event->lock, OSI_MUTEX_MAX_TIMEOUT); - assert(event->ref_count > 0); - event->ref_count++; - osi_mutex_unlock(&event->lock); -} - -static void osi_event_release(struct osi_event *event) -{ - bool should_free = false; - - osi_mutex_lock(&event->lock, OSI_MUTEX_MAX_TIMEOUT); - assert(event->ref_count > 0); - event->ref_count--; - if (event->ref_count == 0) { - should_free = osi_event_should_free(event); - } - osi_mutex_unlock(&event->lock); - - if (should_free) { - osi_event_free(event); - } + return event; } void osi_event_delete(struct osi_event *event) { - bool removed = false; - if (event == NULL) { return; } - osi_event_lock(); - removed = osi_event_remove_alive_locked(event); - osi_event_unlock(); - if (!removed) { - return; - } - - osi_mutex_lock(&event->lock, OSI_MUTEX_MAX_TIMEOUT); - OSI_EVENT_SET_FLAG(event, OSI_EVENT_FLAG_DELETING); - event->item.func = NULL; - event->item.context = NULL; - osi_mutex_unlock(&event->lock); - - osi_event_release(event); + /* The registry pin keeps session-event storage valid until subsystem + * deinit, so delete is only a logical, idempotent operation. */ + osi_mutex_lock(&event->core.lock, OSI_MUTEX_MAX_TIMEOUT); + osi_event_mark_deleting_locked(&event->core); + osi_mutex_unlock(&event->core.lock); } bool osi_event_bind(struct osi_event *event, osi_thread_t *thread, int queue_idx) { bool ret = false; - if (!osi_event_acquire(event)) { + if (event == NULL) { return false; } - osi_mutex_lock(&event->lock, OSI_MUTEX_MAX_TIMEOUT); - if (osi_event_can_bind_locked(event, thread, queue_idx)) { - event->thread = thread; - event->queue_idx = queue_idx; + osi_mutex_lock(&event->core.lock, OSI_MUTEX_MAX_TIMEOUT); + if (osi_event_can_bind_locked(&event->core, thread, queue_idx)) { + event->core.thread = thread; + event->core.queue_idx = (uint8_t)queue_idx; ret = true; } - osi_mutex_unlock(&event->lock); - - osi_event_release(event); - + osi_mutex_unlock(&event->core.lock); return ret; } +struct osi_dynamic_event *osi_dynamic_event_create(osi_thread_func_t func, void *context) +{ + struct osi_dynamic_event *event = + (struct osi_dynamic_event *)osi_event_core_new(sizeof(*event), func, context); + bool added = false; + + if (event == NULL) { + return NULL; + } + + osi_event_lock(); + if (s_osi_dynamic_event_list != NULL) { + added = list_append(s_osi_dynamic_event_list, event); + } + osi_event_unlock(); + if (!added) { + osi_event_core_free(&event->core); + return NULL; + } + return event; +} + +/* The global alive list is the ownership boundary for dynamic events. Never + * dereference event until list membership is confirmed under the global lock. + * Lock nesting is always global-lock-outer, event-lock-inner. + * On success: drops the global lock and returns holding event->core.lock with + * an extra reference; the caller must unlock (and maybe free) via + * osi_event_unlock_and_maybe_free after osi_event_release_locked. */ +static bool osi_dynamic_event_acquire_locked(struct osi_dynamic_event *event) +{ + if (event == NULL) { + return false; + } + + osi_event_lock(); + if (s_osi_dynamic_event_list == NULL || + !list_contains(s_osi_dynamic_event_list, event)) { + osi_event_unlock(); + return false; + } + + osi_mutex_lock(&event->core.lock, OSI_MUTEX_MAX_TIMEOUT); + assert(event->core.ref_count > 0); + event->core.ref_count++; + osi_event_unlock(); + return true; +} + +bool osi_dynamic_event_bind(struct osi_dynamic_event *event, osi_thread_t *thread, int queue_idx) +{ + bool ret = false; + bool should_free; + + if (!osi_dynamic_event_acquire_locked(event)) { + return false; + } + if (osi_event_can_bind_locked(&event->core, thread, queue_idx)) { + event->core.thread = thread; + event->core.queue_idx = (uint8_t)queue_idx; + ret = true; + } + should_free = osi_event_release_locked(&event->core); + osi_event_unlock_and_maybe_free(&event->core, should_free); + return ret; +} + +void osi_dynamic_event_delete(struct osi_dynamic_event *event) +{ + bool removed = false; + bool should_free; + + if (event == NULL) { + return; + } + + osi_event_lock(); + if (s_osi_dynamic_event_list != NULL) { + removed = list_delete(s_osi_dynamic_event_list, event); + } + osi_event_unlock(); + if (!removed) { + return; + } + + osi_mutex_lock(&event->core.lock, OSI_MUTEX_MAX_TIMEOUT); + osi_event_mark_deleting_locked(&event->core); + should_free = osi_event_release_locked(&event->core); + osi_event_unlock_and_maybe_free(&event->core, should_free); +} + static void osi_thread_generic_event_handler(void *context) { - struct osi_event *event = (struct osi_event *)context; + struct osi_event_core *event = (struct osi_event_core *)context; osi_thread_func_t func = NULL; void *func_context = NULL; + bool should_free = false; if (event == NULL) { return; @@ -724,8 +759,8 @@ static void osi_thread_generic_event_handler(void *context) osi_mutex_lock(&event->lock, OSI_MUTEX_MAX_TIMEOUT); OSI_EVENT_CLEAR_FLAG(event, OSI_EVENT_FLAG_QUEUED); if (OSI_EVENT_HAS_FLAG(event, OSI_EVENT_FLAG_DELETING)) { - osi_mutex_unlock(&event->lock); - osi_event_release(event); + should_free = osi_event_release_locked(event); + osi_event_unlock_and_maybe_free(event, should_free); return; } OSI_EVENT_SET_FLAG(event, OSI_EVENT_FLAG_RUNNING); @@ -741,20 +776,17 @@ static void osi_thread_generic_event_handler(void *context) osi_mutex_lock(&event->lock, OSI_MUTEX_MAX_TIMEOUT); OSI_EVENT_CLEAR_FLAG(event, OSI_EVENT_FLAG_RUNNING); OSI_TRACE_DEBUG("%s exit ev=%p flags=0x%x", __func__, event, event->flags); - osi_mutex_unlock(&event->lock); - - osi_event_release(event); + should_free = osi_event_release_locked(event); + osi_event_unlock_and_maybe_free(event, should_free); } /* Reclaim a queued event work item during thread teardown WITHOUT invoking the - * user callback. It only clears the QUEUED flag and drops the reference the - * queued item owns (osi_thread_post_event() retained it); this frees the event - * if it was already deleted, and leaves a still-live event untouched. Unlike - * osi_thread_generic_event_handler(), it never dispatches into stack state that - * may already have been freed on the shutdown path. */ + * user callback. The release path only uses the per-event lock and remains + * valid after the global event subsystem has been deinitialized. */ static void osi_thread_generic_event_drain(void *context) { - struct osi_event *event = (struct osi_event *)context; + struct osi_event_core *event = (struct osi_event_core *)context; + bool should_free = false; if (event == NULL) { return; @@ -762,86 +794,150 @@ static void osi_thread_generic_event_drain(void *context) osi_mutex_lock(&event->lock, OSI_MUTEX_MAX_TIMEOUT); OSI_EVENT_CLEAR_FLAG(event, OSI_EVENT_FLAG_QUEUED); - osi_mutex_unlock(&event->lock); - - osi_event_release(event); + should_free = osi_event_release_locked(event); + osi_event_unlock_and_maybe_free(event, should_free); } bool osi_thread_post_event(struct osi_event *event, uint32_t timeout) { - bool ret = false; - osi_thread_t *thread = NULL; - uint8_t queue_idx = 0; + struct osi_event_core *core; + osi_thread_t *thread; + uint8_t queue_idx; + bool ret; + bool should_free = false; - if (!osi_event_acquire(event)) { - OSI_TRACE_EVENT("%s acquire fail ev=%p", __func__, event); + if (event == NULL) { return false; } + core = &event->core; - osi_mutex_lock(&event->lock, OSI_MUTEX_MAX_TIMEOUT); - if (!osi_event_can_post_locked(event)) { - osi_mutex_unlock(&event->lock); - osi_event_release(event); + /* Session-event storage is pinned until subsystem deinit, so the hot path + * needs only the per-event lock and a queue reference. */ + osi_mutex_lock(&core->lock, OSI_MUTEX_MAX_TIMEOUT); + if (!osi_event_can_post_locked(core)) { + OSI_TRACE_EVENT("%s post fail ev=%p qidx=%u wq_len=%d", __func__, event, core->queue_idx, + core->thread ? osi_thread_queue_wait_size(core->thread, core->queue_idx) : -1); + osi_mutex_unlock(&core->lock); return false; } - OSI_EVENT_SET_FLAG(event, OSI_EVENT_FLAG_QUEUED); - OSI_EVENT_SET_FLAG(event, OSI_EVENT_FLAG_POSTING); - thread = event->thread; - queue_idx = event->queue_idx; - osi_mutex_unlock(&event->lock); + OSI_EVENT_SET_FLAG(core, OSI_EVENT_FLAG_QUEUED); + core->ref_count++; + thread = core->thread; + queue_idx = core->queue_idx; + osi_mutex_unlock(&core->lock); - /* The queued work item owns a reference until the generic handler drains it. */ - osi_event_retain(event); - ret = osi_thread_post(thread, osi_thread_generic_event_handler, event, queue_idx, timeout); - osi_mutex_lock(&event->lock, OSI_MUTEX_MAX_TIMEOUT); - OSI_EVENT_CLEAR_FLAG(event, OSI_EVENT_FLAG_POSTING); - if (!ret) { - // clear "is_queued" when post failure, to allow for following event posts - OSI_EVENT_CLEAR_FLAG(event, OSI_EVENT_FLAG_QUEUED); - } - osi_mutex_unlock(&event->lock); + ret = osi_thread_post(thread, osi_thread_generic_event_handler, core, queue_idx, timeout); + osi_mutex_lock(&core->lock, OSI_MUTEX_MAX_TIMEOUT); if (!ret) { - OSI_TRACE_EVENT("%s enqueue fail ev=%p qidx=%u wq_len=%d", - __func__, event, queue_idx, + OSI_TRACE_EVENT("%s enqueue fail ev=%p qidx=%u wq_len=%d", __func__, event, queue_idx, osi_thread_queue_wait_size(thread, queue_idx)); - osi_event_release(event); + /* Clear QUEUED on post failure so a later post may enqueue. */ + OSI_EVENT_CLEAR_FLAG(core, OSI_EVENT_FLAG_QUEUED); + should_free = osi_event_release_locked(core); } - osi_event_release(event); + osi_event_unlock_and_maybe_free(core, should_free); + return ret; +} + +bool osi_dynamic_event_post(struct osi_dynamic_event *event, uint32_t timeout) +{ + struct osi_event_core *core; + osi_thread_t *thread; + uint8_t queue_idx; + bool ret; + bool should_free = false; + + if (!osi_dynamic_event_acquire_locked(event)) { + return false; + } + core = &event->core; + + if (!osi_event_can_post_locked(core)) { + OSI_TRACE_EVENT("%s post fail ev=%p qidx=%u wq_len=%d", __func__, event, core->queue_idx, + core->thread ? osi_thread_queue_wait_size(core->thread, core->queue_idx) : -1); + should_free = osi_event_release_locked(core); + osi_event_unlock_and_maybe_free(core, should_free); + return false; + } + + OSI_EVENT_SET_FLAG(core, OSI_EVENT_FLAG_QUEUED); + core->ref_count++; + thread = core->thread; + queue_idx = core->queue_idx; + osi_mutex_unlock(&core->lock); + + ret = osi_thread_post(thread, osi_thread_generic_event_handler, core, queue_idx, timeout); + + osi_mutex_lock(&core->lock, OSI_MUTEX_MAX_TIMEOUT); + if (!ret) { + OSI_TRACE_EVENT("%s enqueue fail ev=%p qidx=%u wq_len=%d", __func__, event, queue_idx, + osi_thread_queue_wait_size(thread, queue_idx)); + /* Drop the queue ownership taken above; handler will never run. */ + OSI_EVENT_CLEAR_FLAG(core, OSI_EVENT_FLAG_QUEUED); + should_free = osi_event_release_locked(core); + } + /* Always drop the acquire reference from osi_dynamic_event_acquire_locked. */ + if (osi_event_release_locked(core)) { + should_free = true; + } + osi_event_unlock_and_maybe_free(core, should_free); return ret; } +static void osi_event_retire(struct osi_event_core *event) +{ + bool should_free; + + osi_mutex_lock(&event->lock, OSI_MUTEX_MAX_TIMEOUT); + osi_event_mark_deleting_locked(event); + should_free = osi_event_release_locked(event); + osi_event_unlock_and_maybe_free(event, should_free); +} + int osi_thread_event_init(void) { - int ret = -1; - - do { - if (osi_mutex_new(&s_osi_event_lock) != 0) { - break; - } - - s_osi_event_list = list_new(NULL); - if (s_osi_event_list == NULL) { - break; - } - - ret = 0; - } while (0); - - if (ret != 0) { - osi_thread_event_deinit(); + if (osi_mutex_new(&s_osi_event_lock) != 0) { + return -1; } - return ret; + s_osi_session_event_list = list_new(NULL); + s_osi_dynamic_event_list = list_new(NULL); + if (s_osi_session_event_list == NULL || s_osi_dynamic_event_list == NULL) { + list_free(s_osi_session_event_list); + list_free(s_osi_dynamic_event_list); + s_osi_session_event_list = NULL; + s_osi_dynamic_event_list = NULL; + osi_mutex_free(&s_osi_event_lock); + return -1; + } + return 0; } void osi_thread_event_deinit(void) { - if (s_osi_event_list != NULL) { - list_free(s_osi_event_list); - s_osi_event_list = NULL; + if (s_osi_event_lock == NULL) { + return; } + + osi_event_lock(); + while (s_osi_session_event_list != NULL && !list_is_empty(s_osi_session_event_list)) { + struct osi_event *event = (struct osi_event *)list_front(s_osi_session_event_list); + list_delete(s_osi_session_event_list, event); + osi_event_retire(&event->core); + } + while (s_osi_dynamic_event_list != NULL && !list_is_empty(s_osi_dynamic_event_list)) { + struct osi_dynamic_event *event = + (struct osi_dynamic_event *)list_front(s_osi_dynamic_event_list); + list_delete(s_osi_dynamic_event_list, event); + osi_event_retire(&event->core); + } + list_free(s_osi_session_event_list); + list_free(s_osi_dynamic_event_list); + s_osi_session_event_list = NULL; + s_osi_dynamic_event_list = NULL; + osi_event_unlock(); osi_mutex_free(&s_osi_event_lock); } diff --git a/components/bt/host/bluedroid/btc/profile/std/hid/btc_hh.c b/components/bt/host/bluedroid/btc/profile/std/hid/btc_hh.c index dc0960f355c..ef66c390c7f 100644 --- a/components/bt/host/bluedroid/btc/profile/std/hid/btc_hh.c +++ b/components/bt/host/bluedroid/btc/profile/std/hid/btc_hh.c @@ -1305,7 +1305,7 @@ static void btc_hh_cb_arg_deep_free(btc_msg_t *msg) static void btc_hh_data_path_deinit_inner(btc_hh_device_t *p_dev) { struct pkt_queue *queue; - struct osi_event *event; + struct osi_dynamic_event *event; if (p_dev == NULL) { return; @@ -1319,7 +1319,7 @@ static void btc_hh_data_path_deinit_inner(btc_hh_device_t *p_dev) osi_mutex_unlock(&p_dev->lock); if (event != NULL) { - osi_event_delete(event); + osi_dynamic_event_delete(event); } if (queue != NULL) { @@ -1332,7 +1332,7 @@ bool btc_hh_data_enqueue_linked_pkt(pkt_linked_item_t *linked_pkt) tBTA_HH_DATA_PKT *pkt = linked_pkt != NULL ? (tBTA_HH_DATA_PKT *)linked_pkt->data : NULL; btc_hh_device_t *p_dev = pkt != NULL ? btc_hh_find_connected_dev_by_handle(pkt->dev_handle) : NULL; struct pkt_queue *data_queue; - struct osi_event *data_ready; + struct osi_dynamic_event *data_ready; pkt_linked_item_t *old = NULL; bool enqueue_ok; @@ -1357,7 +1357,7 @@ bool btc_hh_data_enqueue_linked_pkt(pkt_linked_item_t *linked_pkt) enqueue_ok = pkt_queue_enqueue(data_queue, linked_pkt); if (enqueue_ok && data_ready != NULL) { - osi_thread_post_event(data_ready, 0); + osi_dynamic_event_post(data_ready, 0); } osi_mutex_unlock(&p_dev->lock); @@ -1423,7 +1423,7 @@ static bool btc_hh_data_path_init_inner(btc_hh_device_t *p_dev) { bool result = false; struct pkt_queue *data_queue = NULL; - struct osi_event *data_ready = NULL; + struct osi_dynamic_event *data_ready = NULL; do { if (p_dev == NULL) { @@ -1446,13 +1446,13 @@ static bool btc_hh_data_path_init_inner(btc_hh_device_t *p_dev) break; } - data_ready = osi_event_create(btc_hh_data_pkt_handler, p_dev); + data_ready = osi_dynamic_event_create(btc_hh_data_pkt_handler, p_dev); if (data_ready == NULL) { BTC_TRACE_ERROR("%s: osi_event_create failed", __func__); break; } - if (!osi_event_bind(data_ready, btc_get_current_thread(), BTC_HH_DATA_QUEUE_IDX)) { + if (!osi_dynamic_event_bind(data_ready, btc_get_current_thread(), BTC_HH_DATA_QUEUE_IDX)) { BTC_TRACE_ERROR("%s: osi_event_bind failed", __func__); break; } @@ -1470,7 +1470,7 @@ static bool btc_hh_data_path_init_inner(btc_hh_device_t *p_dev) if (!result) { btc_hh_data_path_deinit_inner(p_dev); if (data_ready != NULL) { - osi_event_delete(data_ready); + osi_dynamic_event_delete(data_ready); } if (data_queue != NULL) { pkt_queue_destroy(data_queue, bta_hh_co_data_linked_pkt_free); @@ -1550,7 +1550,7 @@ static void btc_hh_data_pkt_handler(void *arg) } if (p_dev->data_ready != NULL && !pkt_queue_is_empty(p_dev->data_queue)) { - osi_thread_post_event(p_dev->data_ready, 0); + osi_dynamic_event_post(p_dev->data_ready, 0); } } diff --git a/components/bt/host/bluedroid/btc/profile/std/include/btc_hh.h b/components/bt/host/bluedroid/btc/profile/std/include/btc_hh.h index 3117cc7a30f..6627f1c45b4 100644 --- a/components/bt/host/bluedroid/btc/profile/std/include/btc_hh.h +++ b/components/bt/host/bluedroid/btc/profile/std/include/btc_hh.h @@ -84,7 +84,7 @@ typedef struct { uint32_t drop_pkt_cnt; osi_mutex_t lock; struct pkt_queue *data_queue; - struct osi_event *data_ready; + struct osi_dynamic_event *data_ready; osi_alarm_t *vup_timer; } btc_hh_device_t; diff --git a/components/bt/test_apps/basic_unit_test/main/CMakeLists.txt b/components/bt/test_apps/basic_unit_test/main/CMakeLists.txt index e6e2323fb75..b7298964ba7 100644 --- a/components/bt/test_apps/basic_unit_test/main/CMakeLists.txt +++ b/components/bt/test_apps/basic_unit_test/main/CMakeLists.txt @@ -4,6 +4,7 @@ idf_component_register(SRCS "test_bt_main.c" "test_tinycrypt_ecc.c" "test_osal.c" "test_prf_task.c" + "test_osi_event.c" INCLUDE_DIRS "." PRIV_REQUIRES unity bt WHOLE_ARCHIVE) diff --git a/components/bt/test_apps/basic_unit_test/main/test_osi_event.c b/components/bt/test_apps/basic_unit_test/main/test_osi_event.c new file mode 100644 index 00000000000..ee38e60952e --- /dev/null +++ b/components/bt/test_apps/basic_unit_test/main/test_osi_event.c @@ -0,0 +1,535 @@ +/* + * SPDX-FileCopyrightText: 2026 Espressif Systems (Shanghai) CO LTD + * + * SPDX-License-Identifier: Unlicense OR CC0-1.0 + */ + +/* + * Unit tests for osi_event (components/bt/common/osi/thread.c): coalesce, + * delete-while-queued, drain without user callbacks, stale post after delete, + * and re-post after QUEUED is cleared (lost-wakeup regression). + */ + +#include +#include +#include + +#include "unity.h" +#include "unity_test_runner.h" +#include "freertos/FreeRTOS.h" +#include "freertos/semphr.h" +#include "freertos/task.h" + +#include "osi/thread.h" + +#define TEST_WORKER_STACK 3072 +#define TEST_HELPER_STACK 2560 +#define TEST_WAIT_MS 1000 +#define TEST_NO_RUN_MS 50 +#define TEST_WORKER_PRIO_HIGH (configMAX_PRIORITIES - 2) +#define TEST_WORKER_PRIO_LOW 2 + +static SemaphoreHandle_t s_done; +static SemaphoreHandle_t s_gate; +static SemaphoreHandle_t s_join; +static volatile uint32_t s_run_count; +static volatile uint32_t s_dummy_count; +static struct osi_event *s_event; +static volatile bool s_stop_posters; + +static void handler_count(void *context) +{ + (void)context; + s_run_count++; + xSemaphoreGive(s_done); +} + +static void handler_gated(void *context) +{ + (void)context; + s_run_count++; + xSemaphoreGive(s_done); + TEST_ASSERT_EQUAL(pdTRUE, xSemaphoreTake(s_gate, pdMS_TO_TICKS(TEST_WAIT_MS))); +} + +static void handler_repost(void *context) +{ + (void)context; + uint32_t n = ++s_run_count; + if (n < 3) { + TEST_ASSERT_TRUE(osi_thread_post_event(s_event, 0)); + } + if (n == 3) { + xSemaphoreGive(s_done); + } +} + +static void handler_self_delete(void *context) +{ + (void)context; + s_run_count++; + osi_event_delete(s_event); + s_event = NULL; + xSemaphoreGive(s_done); +} + +static void dummy_work(void *context) +{ + (void)context; + s_dummy_count++; +} + +static osi_thread_t *test_thread_create(int priority, size_t queue_len) +{ + const size_t workqueue_len[] = {queue_len}; + /* Pin to core 0: OSI_THREAD_CORE_AFFINITY (== 2) is not a valid FreeRTOS + * core id / tskNO_AFFINITY and trips xTaskCreatePinnedToCore on ESP32. */ + osi_thread_t *thread = osi_thread_create("osi_ev_test", TEST_WORKER_STACK, priority, + OSI_THREAD_CORE_0, 1, workqueue_len, false); + TEST_ASSERT_NOT_NULL(thread); + return thread; +} + +static struct osi_event *test_event_bind(osi_thread_t *thread, osi_thread_func_t func) +{ + struct osi_event *event = osi_event_create(func, NULL); + TEST_ASSERT_NOT_NULL(event); + TEST_ASSERT_TRUE(osi_event_bind(event, thread, 0)); + return event; +} + +static struct osi_dynamic_event *test_dynamic_event_bind(osi_thread_t *thread, + osi_thread_func_t func) +{ + struct osi_dynamic_event *event = osi_dynamic_event_create(func, NULL); + TEST_ASSERT_NOT_NULL(event); + TEST_ASSERT_TRUE(osi_dynamic_event_bind(event, thread, 0)); + return event; +} + +static void osi_event_test_begin(void) +{ + s_run_count = 0; + s_dummy_count = 0; + s_event = NULL; + s_stop_posters = false; + + s_done = xSemaphoreCreateBinary(); + TEST_ASSERT_NOT_NULL(s_done); + s_gate = xSemaphoreCreateBinary(); + TEST_ASSERT_NOT_NULL(s_gate); + s_join = xSemaphoreCreateCounting(8, 0); + TEST_ASSERT_NOT_NULL(s_join); + + TEST_ASSERT_EQUAL(0, osi_thread_event_init()); +} + +static void osi_event_test_end(osi_thread_t *thread, struct osi_event *event) +{ + if (event != NULL) { + osi_event_delete(event); + } + if (thread != NULL) { + osi_thread_free(thread); + } + osi_thread_event_deinit(); + + vSemaphoreDelete(s_done); + s_done = NULL; + vSemaphoreDelete(s_gate); + s_gate = NULL; + vSemaphoreDelete(s_join); + s_join = NULL; + + /* Idle task reclaims deleted worker TCBs asynchronously. */ + vTaskDelay(pdMS_TO_TICKS(20)); +} + +TEST_CASE("osi_event duplicate post coalesces until handler runs", "[osi_event]") +{ + UBaseType_t saved_prio = uxTaskPriorityGet(NULL); + osi_thread_t *thread; + struct osi_event *event; + + osi_event_test_begin(); + vTaskPrioritySet(NULL, TEST_WORKER_PRIO_HIGH); + thread = test_thread_create(TEST_WORKER_PRIO_LOW, 8); + event = test_event_bind(thread, handler_count); + + TEST_ASSERT_TRUE(osi_thread_post_event(event, 0)); + TEST_ASSERT_FALSE(osi_thread_post_event(event, 0)); + TEST_ASSERT_EQUAL_UINT32(0, s_run_count); + + vTaskPrioritySet(NULL, saved_prio); + TEST_ASSERT_EQUAL(pdTRUE, xSemaphoreTake(s_done, pdMS_TO_TICKS(TEST_WAIT_MS))); + TEST_ASSERT_EQUAL_UINT32(1, s_run_count); + TEST_ASSERT_EQUAL(pdFALSE, xSemaphoreTake(s_done, pdMS_TO_TICKS(TEST_NO_RUN_MS))); + TEST_ASSERT_EQUAL_UINT32(1, s_run_count); + + osi_event_test_end(thread, event); +} + +TEST_CASE("osi_event re-post while handler is running is accepted", "[osi_event]") +{ + osi_thread_t *thread; + struct osi_event *event; + + osi_event_test_begin(); + thread = test_thread_create(TEST_WORKER_PRIO_HIGH, 8); + event = test_event_bind(thread, handler_gated); + + TEST_ASSERT_TRUE(osi_thread_post_event(event, OSI_THREAD_MAX_TIMEOUT)); + TEST_ASSERT_EQUAL(pdTRUE, xSemaphoreTake(s_done, pdMS_TO_TICKS(TEST_WAIT_MS))); + TEST_ASSERT_EQUAL_UINT32(1, s_run_count); + + /* QUEUED is already clear; POSTING must not reject this re-post. */ + TEST_ASSERT_TRUE(osi_thread_post_event(event, 0)); + xSemaphoreGive(s_gate); + + TEST_ASSERT_EQUAL(pdTRUE, xSemaphoreTake(s_done, pdMS_TO_TICKS(TEST_WAIT_MS))); + TEST_ASSERT_EQUAL_UINT32(2, s_run_count); + xSemaphoreGive(s_gate); + + osi_event_test_end(thread, event); +} + +TEST_CASE("osi_event callback may self-repost", "[osi_event]") +{ + osi_thread_t *thread; + + osi_event_test_begin(); + thread = test_thread_create(TEST_WORKER_PRIO_HIGH, 8); + s_event = test_event_bind(thread, handler_repost); + + TEST_ASSERT_TRUE(osi_thread_post_event(s_event, 0)); + TEST_ASSERT_EQUAL(pdTRUE, xSemaphoreTake(s_done, pdMS_TO_TICKS(TEST_WAIT_MS))); + TEST_ASSERT_EQUAL_UINT32(3, s_run_count); + + osi_event_test_end(thread, s_event); + s_event = NULL; +} + +TEST_CASE("osi_event callback may self-delete", "[osi_event]") +{ + osi_thread_t *thread; + + osi_event_test_begin(); + thread = test_thread_create(TEST_WORKER_PRIO_HIGH, 8); + s_event = test_event_bind(thread, handler_self_delete); + + TEST_ASSERT_TRUE(osi_thread_post_event(s_event, 0)); + TEST_ASSERT_EQUAL(pdTRUE, xSemaphoreTake(s_done, pdMS_TO_TICKS(TEST_WAIT_MS))); + TEST_ASSERT_EQUAL_UINT32(1, s_run_count); + TEST_ASSERT_NULL(s_event); + + osi_event_test_end(thread, NULL); +} + +TEST_CASE("osi_event delete then stale post is rejected", "[osi_event]") +{ + osi_thread_t *thread; + struct osi_event *event; + + osi_event_test_begin(); + thread = test_thread_create(TEST_WORKER_PRIO_HIGH, 8); + event = test_event_bind(thread, handler_count); + + osi_event_delete(event); + TEST_ASSERT_FALSE(osi_thread_post_event(event, 0)); + TEST_ASSERT_FALSE(osi_thread_post_event(NULL, 0)); + osi_event_delete(event); + TEST_ASSERT_EQUAL_UINT32(0, s_run_count); + + osi_event_test_end(thread, NULL); +} + +TEST_CASE("osi_event delete while queued skips callback and drains refs", "[osi_event]") +{ + UBaseType_t saved_prio = uxTaskPriorityGet(NULL); + osi_thread_t *thread; + struct osi_event *event; + + osi_event_test_begin(); + vTaskPrioritySet(NULL, TEST_WORKER_PRIO_HIGH); + thread = test_thread_create(TEST_WORKER_PRIO_LOW, 8); + event = test_event_bind(thread, handler_count); + + TEST_ASSERT_TRUE(osi_thread_post_event(event, 0)); + osi_event_delete(event); + vTaskPrioritySet(NULL, saved_prio); + + TEST_ASSERT_EQUAL(pdFALSE, xSemaphoreTake(s_done, pdMS_TO_TICKS(TEST_NO_RUN_MS))); + TEST_ASSERT_EQUAL_UINT32(0, s_run_count); + + osi_event_test_end(thread, NULL); +} + +TEST_CASE("osi_event thread free drains pending event without callback", "[osi_event]") +{ + UBaseType_t saved_prio = uxTaskPriorityGet(NULL); + osi_thread_t *thread; + struct osi_event *event; + + osi_event_test_begin(); + vTaskPrioritySet(NULL, TEST_WORKER_PRIO_HIGH); + thread = test_thread_create(TEST_WORKER_PRIO_LOW, 8); + event = test_event_bind(thread, handler_count); + + TEST_ASSERT_TRUE(osi_thread_post_event(event, 0)); + osi_thread_free(thread); + vTaskPrioritySet(NULL, saved_prio); + + TEST_ASSERT_EQUAL_UINT32(0, s_run_count); + osi_event_delete(event); + osi_thread_event_deinit(); + + vSemaphoreDelete(s_done); + s_done = NULL; + vSemaphoreDelete(s_gate); + s_gate = NULL; + vSemaphoreDelete(s_join); + s_join = NULL; + vTaskDelay(pdMS_TO_TICKS(20)); +} + +TEST_CASE("osi_event drain after event subsystem deinit", "[osi_event]") +{ + UBaseType_t saved_prio = uxTaskPriorityGet(NULL); + osi_thread_t *thread; + struct osi_event *event; + + osi_event_test_begin(); + vTaskPrioritySet(NULL, TEST_WORKER_PRIO_HIGH); + thread = test_thread_create(TEST_WORKER_PRIO_LOW, 8); + event = test_event_bind(thread, handler_count); + + TEST_ASSERT_TRUE(osi_thread_post_event(event, 0)); + osi_event_delete(event); + osi_thread_event_deinit(); + osi_thread_free(thread); + vTaskPrioritySet(NULL, saved_prio); + + TEST_ASSERT_EQUAL_UINT32(0, s_run_count); + + vSemaphoreDelete(s_done); + s_done = NULL; + vSemaphoreDelete(s_gate); + s_gate = NULL; + vSemaphoreDelete(s_join); + s_join = NULL; + vTaskDelay(pdMS_TO_TICKS(20)); +} + +TEST_CASE("osi_event post timeout 0 fails when work queue is full", "[osi_event]") +{ + UBaseType_t saved_prio = uxTaskPriorityGet(NULL); + osi_thread_t *thread; + struct osi_event *event; + const size_t queue_len = 2; + + osi_event_test_begin(); + vTaskPrioritySet(NULL, TEST_WORKER_PRIO_HIGH); + thread = test_thread_create(TEST_WORKER_PRIO_LOW, queue_len); + event = test_event_bind(thread, handler_count); + + for (size_t i = 0; i < queue_len; i++) { + TEST_ASSERT_TRUE(osi_thread_post(thread, dummy_work, NULL, 0, 0)); + } + TEST_ASSERT_FALSE(osi_thread_post_event(event, 0)); + TEST_ASSERT_EQUAL_UINT32(0, s_run_count); + + /* Queue reservation was rolled back; a later post can succeed once space exists. */ + vTaskPrioritySet(NULL, saved_prio); + TEST_ASSERT_EQUAL(pdFALSE, xSemaphoreTake(s_done, pdMS_TO_TICKS(TEST_NO_RUN_MS))); + + vTaskPrioritySet(NULL, TEST_WORKER_PRIO_HIGH); + TEST_ASSERT_TRUE(osi_thread_post_event(event, 0)); + vTaskPrioritySet(NULL, saved_prio); + TEST_ASSERT_EQUAL(pdTRUE, xSemaphoreTake(s_done, pdMS_TO_TICKS(TEST_WAIT_MS))); + TEST_ASSERT_EQUAL_UINT32(1, s_run_count); + + osi_event_test_end(thread, event); +} + +static void poster_task(void *arg) +{ + struct osi_event *event = (struct osi_event *)arg; + + while (!s_stop_posters) { + osi_thread_post_event(event, 0); + /* Must block, not only yield: two ready posters can pin both CPUs and + * starve IDLE*, which trips the task WDT. */ + vTaskDelay(1); + } + xSemaphoreGive(s_join); + vTaskDelete(NULL); +} + +TEST_CASE("osi_event concurrent post and delete", "[osi_event]") +{ + osi_thread_t *thread; + struct osi_event *event; + + osi_event_test_begin(); + thread = test_thread_create(TEST_WORKER_PRIO_HIGH, 16); + event = test_event_bind(thread, handler_count); + + TEST_ASSERT_EQUAL(pdPASS, xTaskCreate(poster_task, "osi_ev_p1", TEST_HELPER_STACK, + event, TEST_WORKER_PRIO_LOW, NULL)); + TEST_ASSERT_EQUAL(pdPASS, xTaskCreate(poster_task, "osi_ev_p2", TEST_HELPER_STACK, + event, TEST_WORKER_PRIO_LOW, NULL)); + + vTaskDelay(pdMS_TO_TICKS(30)); + osi_event_delete(event); + s_stop_posters = true; + + TEST_ASSERT_EQUAL(pdTRUE, xSemaphoreTake(s_join, pdMS_TO_TICKS(TEST_WAIT_MS))); + TEST_ASSERT_EQUAL(pdTRUE, xSemaphoreTake(s_join, pdMS_TO_TICKS(TEST_WAIT_MS))); + + TEST_ASSERT_FALSE(osi_thread_post_event(event, 0)); + + osi_event_test_end(thread, NULL); +} + +TEST_CASE("osi_event create bind post delete stop cycles", "[osi_event]") +{ + for (int cycle = 0; cycle < 8; cycle++) { + osi_thread_t *thread; + struct osi_event *event; + + osi_event_test_begin(); + thread = test_thread_create(TEST_WORKER_PRIO_HIGH, 8); + event = test_event_bind(thread, handler_count); + + TEST_ASSERT_TRUE(osi_thread_post_event(event, OSI_THREAD_MAX_TIMEOUT)); + TEST_ASSERT_EQUAL(pdTRUE, xSemaphoreTake(s_done, pdMS_TO_TICKS(TEST_WAIT_MS))); + TEST_ASSERT_EQUAL_UINT32(1, s_run_count); + + osi_event_test_end(thread, event); + } +} + +TEST_CASE("osi_dynamic_event delete then stale post is rejected", "[osi_event]") +{ + osi_thread_t *thread; + struct osi_dynamic_event *event; + + osi_event_test_begin(); + thread = test_thread_create(TEST_WORKER_PRIO_HIGH, 8); + event = test_dynamic_event_bind(thread, handler_count); + + osi_dynamic_event_delete(event); + TEST_ASSERT_FALSE(osi_dynamic_event_post(event, 0)); + TEST_ASSERT_FALSE(osi_dynamic_event_post(NULL, 0)); + osi_dynamic_event_delete(event); + TEST_ASSERT_EQUAL_UINT32(0, s_run_count); + + osi_event_test_end(thread, NULL); +} + +TEST_CASE("osi_dynamic_event delete while queued skips callback", "[osi_event]") +{ + UBaseType_t saved_prio = uxTaskPriorityGet(NULL); + osi_thread_t *thread; + struct osi_dynamic_event *event; + + osi_event_test_begin(); + vTaskPrioritySet(NULL, TEST_WORKER_PRIO_HIGH); + thread = test_thread_create(TEST_WORKER_PRIO_LOW, 8); + event = test_dynamic_event_bind(thread, handler_count); + + TEST_ASSERT_TRUE(osi_dynamic_event_post(event, 0)); + osi_dynamic_event_delete(event); + vTaskPrioritySet(NULL, saved_prio); + + TEST_ASSERT_EQUAL(pdFALSE, xSemaphoreTake(s_done, pdMS_TO_TICKS(TEST_NO_RUN_MS))); + TEST_ASSERT_EQUAL_UINT32(0, s_run_count); + + osi_event_test_end(thread, NULL); +} + +static void dynamic_poster_task(void *arg) +{ + struct osi_dynamic_event *event = (struct osi_dynamic_event *)arg; + + while (!s_stop_posters) { + osi_dynamic_event_post(event, 0); + vTaskDelay(1); + } + xSemaphoreGive(s_join); + vTaskDelete(NULL); +} + +TEST_CASE("osi_dynamic_event concurrent post and delete", "[osi_event]") +{ + osi_thread_t *thread; + struct osi_dynamic_event *event; + + osi_event_test_begin(); + thread = test_thread_create(TEST_WORKER_PRIO_HIGH, 16); + event = test_dynamic_event_bind(thread, handler_count); + + TEST_ASSERT_EQUAL(pdPASS, xTaskCreate(dynamic_poster_task, "osi_dev_p1", TEST_HELPER_STACK, + event, TEST_WORKER_PRIO_LOW, NULL)); + TEST_ASSERT_EQUAL(pdPASS, xTaskCreate(dynamic_poster_task, "osi_dev_p2", TEST_HELPER_STACK, + event, TEST_WORKER_PRIO_LOW, NULL)); + + vTaskDelay(pdMS_TO_TICKS(30)); + osi_dynamic_event_delete(event); + s_stop_posters = true; + + TEST_ASSERT_EQUAL(pdTRUE, xSemaphoreTake(s_join, pdMS_TO_TICKS(TEST_WAIT_MS))); + TEST_ASSERT_EQUAL(pdTRUE, xSemaphoreTake(s_join, pdMS_TO_TICKS(TEST_WAIT_MS))); + TEST_ASSERT_FALSE(osi_dynamic_event_post(event, 0)); + + osi_event_test_end(thread, NULL); +} + +TEST_CASE("osi_dynamic_event create post delete cycles in one session", "[osi_event]") +{ + osi_thread_t *thread; + + osi_event_test_begin(); + thread = test_thread_create(TEST_WORKER_PRIO_HIGH, 8); + + for (int cycle = 0; cycle < 16; cycle++) { + struct osi_dynamic_event *event = test_dynamic_event_bind(thread, handler_count); + + TEST_ASSERT_TRUE(osi_dynamic_event_post(event, OSI_THREAD_MAX_TIMEOUT)); + TEST_ASSERT_EQUAL(pdTRUE, xSemaphoreTake(s_done, pdMS_TO_TICKS(TEST_WAIT_MS))); + osi_dynamic_event_delete(event); + } + TEST_ASSERT_EQUAL_UINT32(16, s_run_count); + + osi_event_test_end(thread, NULL); +} + +TEST_CASE("osi_event deinit retires queued session and dynamic events", "[osi_event]") +{ + UBaseType_t saved_prio = uxTaskPriorityGet(NULL); + osi_thread_t *thread; + struct osi_event *session_event; + struct osi_dynamic_event *dynamic_event; + + osi_event_test_begin(); + vTaskPrioritySet(NULL, TEST_WORKER_PRIO_HIGH); + thread = test_thread_create(TEST_WORKER_PRIO_LOW, 8); + session_event = test_event_bind(thread, handler_count); + dynamic_event = test_dynamic_event_bind(thread, handler_count); + + TEST_ASSERT_TRUE(osi_thread_post_event(session_event, 0)); + TEST_ASSERT_TRUE(osi_dynamic_event_post(dynamic_event, 0)); + osi_thread_event_deinit(); + osi_thread_free(thread); + vTaskPrioritySet(NULL, saved_prio); + + TEST_ASSERT_EQUAL_UINT32(0, s_run_count); + + vSemaphoreDelete(s_done); + s_done = NULL; + vSemaphoreDelete(s_gate); + s_gate = NULL; + vSemaphoreDelete(s_join); + s_join = NULL; + vTaskDelay(pdMS_TO_TICKS(20)); +}