From 79e119b6b470632af8a316d421d8564c1600888c Mon Sep 17 00:00:00 2001 From: Zhou Xiao Date: Tue, 14 Apr 2026 13:14:14 +0800 Subject: [PATCH] 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 1275b78ad6c883d18e2b7c0dcff80bc410cc8a29) Co-authored-by: Zhou Xiao --- components/bt/common/ble_log/src/ble_log.c | 29 ++++++++++--------- .../bt/common/ble_log/src/ble_log_lbm.c | 10 +++---- components/bt/common/ble_log/src/ble_log_rt.c | 11 +++++-- 3 files changed, 30 insertions(+), 20 deletions(-) diff --git a/components/bt/common/ble_log/src/ble_log.c b/components/bt/common/ble_log/src/ble_log.c index ed0625688f2..4e6ce25838d 100644 --- a/components/bt/common/ble_log/src/ble_log.c +++ b/components/bt/common/ble_log/src/ble_log.c @@ -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 */ diff --git a/components/bt/common/ble_log/src/ble_log_lbm.c b/components/bt/common/ble_log/src/ble_log_lbm.c index 7bed717b2c9..db673c80be8 100644 --- a/components/bt/common/ble_log/src/ble_log_lbm.c +++ b/components/bt/common/ble_log/src/ble_log_lbm.c @@ -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); } diff --git a/components/bt/common/ble_log/src/ble_log_rt.c b/components/bt/common/ble_log/src/ble_log_rt.c index b2aabe6c806..f5d79bf5476 100644 --- a/components/bt/common/ble_log/src/ble_log_rt.c +++ b/components/bt/common/ble_log/src/ble_log_rt.c @@ -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; }