fix(driver_twai): fix potential crash when node_delete from low priority task

Closes https://github.com/espressif/esp-idf/issues/18803

components/esp_driver_twai/esp_twai_onchip.c

@@ -8,6 +8,8 @@
 #include "esp_twai.h"
 #include "esp_twai_onchip.h"
 #include "twai_private.h"
+#include "freertos/semphr.h"
+#include "freertos/timers.h"
 #include "esp_private/twai_interface.h"
 #include "esp_private/twai_utils.h"
 #include "esp_private/twai_frame_queue.h"
@@ -322,6 +324,26 @@ static void _node_isr_main(void *arg)
     }
 }
 
+static void _node_pended_flush_marker(void *sem, uint32_t unused)
+{
+    (void)unused;
+    xSemaphoreGive((SemaphoreHandle_t)sem);
+}
+
+static void _node_flush_pended_set_bits(void)
+{
+    StaticSemaphore_t sem_storage;
+    SemaphoreHandle_t sem = xSemaphoreCreateBinaryStatic(&sem_storage);
+    assert(sem);
+
+    // The `xEventGroupSetBitsFromISR` which used in ISR is not done immediately but just called `xTimerPendFunctionCall`,
+    // wait a semaphore from xTimerPendFunctionCall again is able to ensure the timer task fifo is all done.
+    if (xTimerPendFunctionCall(_node_pended_flush_marker, sem, 0, portMAX_DELAY) == pdPASS) {
+        xSemaphoreTake(sem, portMAX_DELAY);
+    }
+    vSemaphoreDelete(sem);
+}
+
 static void _node_destroy(twai_onchip_ctx_t *twai_ctx)
 {
 #ifdef CONFIG_PM_ENABLE
@@ -348,6 +370,8 @@ static void _node_destroy(twai_onchip_ctx_t *twai_ctx)
     }
     twai_frame_queue_del(twai_ctx->tx_queue);
     if (twai_ctx->event_group) {
+        // xEventGroupSetBitsFromISR is not done immediately, need flush it before deleting
+        _node_flush_pended_set_bits();
         vEventGroupDeleteWithCaps(twai_ctx->event_group);
     }
     if (twai_ctx->ctrlr_id != -1) {
@@ -767,7 +791,7 @@ esp_err_t twai_new_node_onchip(const twai_onchip_node_config_t *node_config, twa
 #endif //CONFIG_PM_ENABLE
 
     // Set clock source, enable bus clock and reset controller
-    ESP_RETURN_ON_ERROR(esp_clk_tree_enable_src(node->curr_clk_src, true), TAG, "enable clock source failed");
+    ESP_GOTO_ON_ERROR(esp_clk_tree_enable_src(node->curr_clk_src, true), err, TAG, "enable clock source failed");
     ESP_LOGD(TAG, "set clock source to %d, freq: %ld Hz", node->curr_clk_src, node->src_freq_hz);
     _twai_rcc_clock_sel(node->ctrlr_id, node->curr_clk_src);
     _twai_rcc_clock_ctrl(ctrlr_id, true);

components/esp_driver_twai/include/esp_twai.h

@@ -44,6 +44,7 @@ esp_err_t twai_node_recover(twai_node_handle_t node);
 
 /**
  * @brief Delete the TWAI node and release resources
+ * @note  Do NOT call this function in the FreeRTOS timer task.
  *
  * @param node Handle to the TWAI node
  * @return  - ESP_OK: Success

components/esp_driver_twai/test_apps/test_twai/main/test_twai_common.cpp

@@ -999,3 +999,65 @@ TEST_CASE("twai schedule transmit", "[twai]")
     TEST_ESP_OK(twai_node_disable(node_hdl));
     TEST_ESP_OK(twai_node_delete(node_hdl));
 }
+
+static IRAM_ATTR bool test_event_tx_done_cb(twai_node_handle_t handle, const twai_tx_done_event_data_t *edata, void *user_ctx)
+{
+    *((bool *)user_ctx) = true;
+    return false;
+}
+
+static void test_driver_event_group(void *args)
+{
+    twai_node_handle_t node_hdl;
+    twai_onchip_node_config_t node_config = {};
+    node_config.io_cfg.tx = TEST_TX_GPIO;
+    node_config.io_cfg.rx = TEST_TX_GPIO; // Using same pin for test without transceiver
+    node_config.io_cfg.quanta_clk_out = GPIO_NUM_NC;
+    node_config.io_cfg.bus_off_indicator = GPIO_NUM_NC;
+    node_config.bit_timing.bitrate = 500000;
+    node_config.tx_queue_depth = 1;
+    node_config.flags.enable_loopback = true;
+    node_config.flags.enable_self_test = true;
+    TEST_ESP_OK(twai_new_node_onchip(&node_config, &node_hdl));
+
+    bool tx_done = false;
+    twai_event_callbacks_t user_cbs = {};
+    user_cbs.on_tx_done = test_event_tx_done_cb;
+    TEST_ESP_OK(twai_node_register_event_callbacks(node_hdl, &user_cbs, &tx_done));
+    TEST_ESP_OK(twai_node_enable(node_hdl));
+
+    twai_frame_t tx_frame = {};
+    TEST_ESP_OK(twai_node_transmit(node_hdl, &tx_frame, 100));
+
+    // TX done ISR has pended xEventGroupSetBitsFromISR now, but this high-priority
+    // task get run before lower-priority timer task, so xEventGroupSetBitsFromISR not actually finish now.
+    while (!tx_done);
+    TEST_ESP_OK(twai_node_disable(node_hdl));
+    TEST_ESP_OK(twai_node_delete(node_hdl));
+
+    // alloc some memory to let deleted node's memory got polluted
+    void *blocks[20] = {};
+    for (int i = 1; i < 20; i++) {
+        blocks[i] = heap_caps_calloc(i, 8, MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT);
+    }
+    for (int i = 0; i < 20; i++) {
+        free(blocks[i]);
+    }
+    vTaskDelete(NULL);
+}
+
+//   twai_task           twai_isr               timer task
+//      tx                  |                        |
+//       |               tx done                     |
+//       |          trigger timer task               |
+//       |             to set event                  |
+//  delete node                            (low priority not run)
+//                                           (start set event
+//                                           but node deleted)
+//===============================================================
+TEST_CASE("twai delete event group before setbits", "[twai]")
+{
+    // let twai task higher than timer task so it can run first
+    TEST_ASSERT_EQUAL(pdPASS, xTaskCreate(test_driver_event_group, "twai_del", 4096, NULL, configTIMER_TASK_PRIORITY + 1, NULL));
+    vTaskDelay(pdMS_TO_TICKS(500));
+}

docs/en/api-reference/peripherals/twai.rst

@@ -210,6 +210,8 @@ Stopping and Deleting the Node
 
 When the TWAI node is no longer needed, you should call :cpp:func:`twai_node_delete` to release software and hardware resources. Make sure the TWAI controller is stopped before deleting the node.
 
+Note: Due to the design of the driver, please do not call the delete function in the FreeRTOS timer task.
+
 Advanced Features
 =================
 

docs/zh_CN/api-reference/peripherals/twai.rst

@@ -210,6 +210,8 @@ TWAI 驱动支持为每个成功接收的报文创建一个 64 位的时间戳
 
 当不再需要使用 TWAI 时，应该调用 :cpp:func:`twai_node_delete` 函数来释放软硬件资源。删除前请确保 TWAI 已经处于停止状态。
 
+注意：由于驱动设计的原因，请不要在 FreeRTOS 定时器任务 (Timer task) 中调用删除函数。
+
 进阶功能
 ========