mirror of
https://github.com/espressif/esp-idf.git
synced 2026-09-30 18:20:38 +03:00
fix(ble_log): fix deinit race and concurrent flush deadlock
- Reorder deinit: stop RT task before destroying peripheral driver to
prevent sending transports to a NULL dev_handle on dual-core (H2)
- Drain remaining queue items in rt_deinit to clean up transport state
- Add atomic flush_in_progress guard to prevent two concurrent
ble_log_flush() callers from deadlocking on ref_count spin-wait (H5)
(cherry picked from commit 1275b78ad6)
Co-authored-by: Zhou Xiao <zhouxiao@espressif.com>
This commit is contained in:
@@ -72,20 +72,23 @@ void ble_log_deinit(void)
|
||||
ble_log_enable(false);
|
||||
ble_log_inited = false;
|
||||
|
||||
/* CRITICAL:
|
||||
* BLE Log peripheral interface must be deinitialized at first,
|
||||
* because there's a risky scenario that may cause severe peripheral
|
||||
* driver fault - if a log buffer is sent to peripheral driver, and
|
||||
* ble_log_deinit is called; in this case, if LBM is deinitialized
|
||||
* before peripheral interface, the log buffer may be freed before
|
||||
* peripheral driver completing tx, and the result would be faulty */
|
||||
ble_log_prph_deinit();
|
||||
|
||||
/* Deinitialize BLE Log LBM */
|
||||
ble_log_lbm_deinit();
|
||||
|
||||
/* Deinitialize BLE Log Runtime */
|
||||
/* CRITICAL — Deinit ordering rationale:
|
||||
*
|
||||
* 1. Runtime task must be stopped FIRST to prevent it from sending
|
||||
* transports to an already-destroyed peripheral driver. The queue
|
||||
* is drained and pending transports are discarded.
|
||||
*
|
||||
* 2. Peripheral interface is deinitialized SECOND. It waits for any
|
||||
* in-flight DMA operations (started before the task was killed) to
|
||||
* complete, then destroys the driver. This is safe because no new
|
||||
* DMA operations can be started (the task is already dead).
|
||||
*
|
||||
* 3. LBM is deinitialized LAST. At this point all DMA has completed
|
||||
* (ensured by step 2) and all queued transports have been drained
|
||||
* (ensured by step 1), so freeing the buffers is safe. */
|
||||
ble_log_rt_deinit();
|
||||
ble_log_prph_deinit();
|
||||
ble_log_lbm_deinit();
|
||||
|
||||
#if CONFIG_BLE_LOG_TS_ENABLED
|
||||
/* Deinitialize BLE Log TS */
|
||||
|
||||
@@ -195,11 +195,11 @@ void ble_log_stat_mgr_update(ble_log_src_t src_code, uint32_t len, bool lost)
|
||||
uint32_t bytes_cnt = len + BLE_LOG_FRAME_OVERHEAD;
|
||||
if (lost) {
|
||||
BLE_LOG_GET_FRAME_SN(&(stat_mgr->frame_sn)); /* consume SN for loss detection */
|
||||
stat_mgr->lost_frame_cnt++;
|
||||
stat_mgr->lost_bytes_cnt += bytes_cnt;
|
||||
__atomic_fetch_add(&stat_mgr->lost_frame_cnt, 1, __ATOMIC_RELAXED);
|
||||
__atomic_fetch_add(&stat_mgr->lost_bytes_cnt, bytes_cnt, __ATOMIC_RELAXED);
|
||||
} else {
|
||||
stat_mgr->written_frame_cnt++;
|
||||
stat_mgr->written_bytes_cnt += bytes_cnt;
|
||||
__atomic_fetch_add(&stat_mgr->written_frame_cnt, 1, __ATOMIC_RELAXED);
|
||||
__atomic_fetch_add(&stat_mgr->written_bytes_cnt, bytes_cnt, __ATOMIC_RELAXED);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -290,7 +290,7 @@ void ble_log_lbm_deinit(void)
|
||||
|
||||
/* Disable module and wait for all references to be released */
|
||||
uint32_t time_waited = 0;
|
||||
while (lbm_ref_count > 0) {
|
||||
while (__atomic_load_n(&lbm_ref_count, __ATOMIC_ACQUIRE) > 0) {
|
||||
vTaskDelay(pdMS_TO_TICKS(1));
|
||||
BLE_LOG_ASSERT(time_waited++ < 1000);
|
||||
}
|
||||
|
||||
@@ -31,7 +31,7 @@ BLE_LOG_STATIC void ble_log_rt_ts_trigger(void *arg);
|
||||
#endif /* CONFIG_BLE_LOG_TS_ENABLED */
|
||||
|
||||
/* PRIVATE FUNCTION */
|
||||
BLE_LOG_IRAM_ATTR BLE_LOG_STATIC void ble_log_rt_task(void *pvParameters)
|
||||
BLE_LOG_STATIC void ble_log_rt_task(void *pvParameters)
|
||||
{
|
||||
(void)pvParameters;
|
||||
ble_log_prph_trans_t *trans = NULL;
|
||||
@@ -162,8 +162,15 @@ void ble_log_rt_deinit(void)
|
||||
rt_task_handle = NULL;
|
||||
}
|
||||
|
||||
/* Release task queue */
|
||||
/* Drain remaining queue items to clean up transport state */
|
||||
if (rt_queue_handle) {
|
||||
ble_log_prph_trans_t *trans = NULL;
|
||||
while (xQueueReceive(rt_queue_handle, &trans, 0) == pdTRUE) {
|
||||
ble_log_lbm_t *lbm = (ble_log_lbm_t *)trans->owner;
|
||||
trans->pos = 0;
|
||||
__atomic_fetch_sub(&lbm->trans_inflight, 1, __ATOMIC_RELAXED);
|
||||
__atomic_store_n(&trans->prph_owned, false, __ATOMIC_RELEASE);
|
||||
}
|
||||
vQueueDelete(rt_queue_handle);
|
||||
rt_queue_handle = NULL;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user