From f62a168307f09cb4e1f8f9a6cc7aa72be41604f8 Mon Sep 17 00:00:00 2001 From: Xu Si Yu Date: Fri, 21 Aug 2026 17:24:09 +0800 Subject: [PATCH] fix(openthread): fix stack deinit by reversing netif glue teardown and hardening workflow cleanup --- components/esp_coex/src/coexist.c | 2 - .../openthread/include/esp_openthread.h | 7 +++- .../include/esp_openthread_border_router.h | 7 +++- .../private_include/esp_openthread_platform.h | 6 ++- components/openthread/src/esp_openthread.cpp | 5 +-- .../src/esp_openthread_netif_glue.c | 37 +++++++++++++------ .../src/esp_openthread_platform.cpp | 22 ++++++++--- .../src/esp_openthread_task_queue.c | 4 ++ 8 files changed, 63 insertions(+), 27 deletions(-) diff --git a/components/esp_coex/src/coexist.c b/components/esp_coex/src/coexist.c index c303350a1ce..8cb65fe61a9 100644 --- a/components/esp_coex/src/coexist.c +++ b/components/esp_coex/src/coexist.c @@ -287,8 +287,6 @@ esp_err_t esp_disable_extern_coex_gpio_pin(void) #if CONFIG_ESP_COEX_SW_COEXIST_ENABLE && CONFIG_SOC_IEEE802154_SUPPORTED esp_err_t esp_coex_wifi_i154_enable(void) { - // TODO: Add a scheme for wifi and 154 coex. - // Remove this function if FCC-50 closes. coex_enable(); esp_coex_ieee802154_status_enable(); return ESP_OK; diff --git a/components/openthread/include/esp_openthread.h b/components/openthread/include/esp_openthread.h index 7ce38a58844..f038ef2690a 100644 --- a/components/openthread/include/esp_openthread.h +++ b/components/openthread/include/esp_openthread.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2021-2025 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2021-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -106,6 +106,11 @@ esp_err_t esp_openthread_start(const esp_openthread_config_t *config); /** * @brief This function performs OpenThread stack and platform driver deinitialization and delete the handle task. + * + * @note Thread must already be inactive (`otThreadSetEnabled(false)` and + * `otIp6SetEnabled(false)`). If border routing was initialized, call + * `esp_openthread_border_router_deinit()` first. + * * @return * - ESP_OK on success * - ESP_ERR_INVALID_STATE if Thread is already active diff --git a/components/openthread/include/esp_openthread_border_router.h b/components/openthread/include/esp_openthread_border_router.h index e78c28bddd1..3d07c6496ff 100644 --- a/components/openthread/include/esp_openthread_border_router.h +++ b/components/openthread/include/esp_openthread_border_router.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2021-2025 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2021-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -44,6 +44,11 @@ esp_err_t esp_openthread_border_router_init(void); /** * @brief Deinitializes the border router features of OpenThread. * + * @note This function must be called while the OpenThread mainloop is still running + * and after Thread IPv6 has been brought down: + * `otThreadSetEnabled(instance, false)` then `otIp6SetEnabled(instance, false)`. + * Call `esp_openthread_stop()` only after this function returns. + * * @return * - ESP_OK on success * - ESP_ERR_INVALID_STATE if not initialized diff --git a/components/openthread/private_include/esp_openthread_platform.h b/components/openthread/private_include/esp_openthread_platform.h index faa78ec1d65..9fa83d00215 100644 --- a/components/openthread/private_include/esp_openthread_platform.h +++ b/components/openthread/private_include/esp_openthread_platform.h @@ -15,7 +15,7 @@ extern "C" { #endif -#define WORKFLOW_MAX_NAMELEN 16 +#define WORKFLOW_MAX_NAMELEN 32 /** * @brief update function declaration @@ -60,11 +60,13 @@ typedef struct esp_openthread_platform_workflow { * * @param[in] updatefcn The update function of the workflow added to the list. * @param[in] processfcn The process function of the workflow added to the list. - * @param[in] name The name of the added workflow + * @param[in] name The name of the added workflow. Must be non-empty and shorter + * than WORKFLOW_MAX_NAMELEN (including the terminating '\\0'). * * @return * - ESP_OK on success * - ESP_ERR_NO_MEM on allocation failure + * - ESP_ERR_INVALID_ARG if name is NULL, empty, or too long * - ESP_FAIL on other failures * */ diff --git a/components/openthread/src/esp_openthread.cpp b/components/openthread/src/esp_openthread.cpp index 2fea58c0750..f0a64698bed 100644 --- a/components/openthread/src/esp_openthread.cpp +++ b/components/openthread/src/esp_openthread.cpp @@ -225,12 +225,9 @@ esp_err_t esp_openthread_launch_mainloop(void) esp_openthread_lock_release(); if (error != ESP_OK) { ESP_LOGE(OT_PLAT_LOG_TAG, "esp_openthread_platform_process failed"); - break; } } else { - error = ESP_FAIL; - ESP_LOGE(OT_PLAT_LOG_TAG, "OpenThread system polling failed"); - break; + ESP_LOGE(OT_PLAT_LOG_TAG, "OpenThread system polling failed (errno: %d)", errno); } } #if CONFIG_OPENTHREAD_TASK_BLOCK_MONITOR diff --git a/components/openthread/src/esp_openthread_netif_glue.c b/components/openthread/src/esp_openthread_netif_glue.c index fc371d48490..ad1f714cb41 100644 --- a/components/openthread/src/esp_openthread_netif_glue.c +++ b/components/openthread/src/esp_openthread_netif_glue.c @@ -150,6 +150,9 @@ static esp_err_t process_thread_transmit(otInstance *instance) int ret = read(s_openthread_netif_glue.event_fd, &event, sizeof(event)); assert(ret == sizeof(event)); + if (s_packet_queue == NULL) { + return ESP_OK; + } while (xQueueReceive(s_packet_queue, &msg, 0) == pdTRUE) { if (msg) { otError ot_error = otIp6Send(esp_openthread_get_instance(), msg); @@ -179,9 +182,15 @@ static esp_err_t openthread_netif_transmit(void *handle, void *buffer, size_t le { esp_err_t error = ESP_OK; otError ot_error = OT_ERROR_NONE; + otMessage *message = NULL; esp_openthread_task_switching_lock_acquire(portMAX_DELAY); + if (s_packet_queue == NULL) { + ESP_LOGW(OT_PLAT_LOG_TAG, "Thread netif transmit after glue deinit"); + ExitNow(error = ESP_ERR_INVALID_STATE); + } + otMessageSettings settings = {}; switch (otThreadGetDeviceRole(esp_openthread_get_instance())) { @@ -195,7 +204,7 @@ static esp_err_t openthread_netif_transmit(void *handle, void *buffer, size_t le break; } - otMessage *message = otIp6NewMessage(esp_openthread_get_instance(), &settings); + message = otIp6NewMessage(esp_openthread_get_instance(), &settings); if (message == NULL) { ESP_LOGE(OT_PLAT_LOG_TAG, "Failed to allocate OpenThread message"); ExitNow(error = ESP_ERR_NO_MEM); @@ -359,22 +368,28 @@ exit: void esp_openthread_netif_glue_deinit(void) { otInstance *instance = esp_openthread_get_instance(); - otIp6SetAddressCallback(instance, NULL, NULL); - otIp6SetReceiveCallback(instance, NULL, NULL); - if (s_packet_queue) { - vQueueDelete(s_packet_queue); - s_packet_queue = NULL; + if (s_openthread_netif) { + esp_netif_action_stop(s_openthread_netif, OPENTHREAD_EVENT, OPENTHREAD_EVENT_STOP, NULL); + s_openthread_netif = NULL; } + unregister_openthread_event_handlers(); + esp_openthread_platform_workflow_unregister(netif_glue_workflow); if (s_openthread_netif_glue.event_fd >= 0) { close(s_openthread_netif_glue.event_fd); s_openthread_netif_glue.event_fd = -1; } - if (esp_event_post(OPENTHREAD_EVENT, OPENTHREAD_EVENT_STOP, NULL, 0, 0) != ESP_OK) { - ESP_LOGE(OT_PLAT_LOG_TAG, "Failed to stop OpenThread netif"); + otIp6SetReceiveCallback(instance, NULL, NULL); + otIp6SetAddressCallback(instance, NULL, NULL); + if (s_packet_queue) { + otMessage *msg = NULL; + while (xQueueReceive(s_packet_queue, &msg, 0) == pdTRUE) { + if (msg) { + otMessageFree(msg); + } + } + vQueueDelete(s_packet_queue); + s_packet_queue = NULL; } - s_openthread_netif = NULL; - unregister_openthread_event_handlers(); - esp_openthread_platform_workflow_unregister(netif_glue_workflow); } void esp_openthread_netif_glue_update(esp_openthread_mainloop_context_t *mainloop) diff --git a/components/openthread/src/esp_openthread_platform.cpp b/components/openthread/src/esp_openthread_platform.cpp index 991ab07267d..49468dec796 100644 --- a/components/openthread/src/esp_openthread_platform.cpp +++ b/components/openthread/src/esp_openthread_platform.cpp @@ -33,15 +33,18 @@ static esp_openthread_platform_workflow_t *s_workflow_list = NULL; esp_err_t esp_openthread_platform_workflow_register(esp_openthread_update_func update_func, esp_openthread_process_func process_func, const char *name) { - uint8_t name_len = strnlen(name, WORKFLOW_MAX_NAMELEN - 1); + assert(name != NULL); + size_t name_len = strlen(name); + ESP_RETURN_ON_FALSE(name_len > 0 && name_len < WORKFLOW_MAX_NAMELEN, ESP_ERR_INVALID_ARG, OT_PLAT_LOG_TAG, + "Workflow name '%s' is invalid", name); + esp_openthread_platform_workflow_t *current_workflow = s_workflow_list; esp_openthread_platform_workflow_t *before_workflow = NULL; esp_openthread_platform_workflow_t *add_workflow = static_cast(calloc(1, sizeof(esp_openthread_platform_workflow_t))); ESP_RETURN_ON_FALSE(add_workflow != NULL, ESP_ERR_NO_MEM, OT_PLAT_LOG_TAG, "Failed to alloc memory for esp_openthread_workflow"); - strncpy(add_workflow->name, name, name_len); - add_workflow->name[name_len] = '\0'; + memcpy(add_workflow->name, name, name_len + 1); add_workflow->update_func = update_func; add_workflow->process_func = process_func; add_workflow->next = NULL; @@ -67,6 +70,9 @@ esp_err_t esp_openthread_platform_workflow_register(esp_openthread_update_func u void esp_openthread_platform_workflow_unregister(const char *name) { + if (name == NULL) { + return; + } esp_openthread_platform_workflow_t *current_workflow = s_workflow_list; esp_openthread_platform_workflow_t *before_workflow = NULL; while (current_workflow) { @@ -206,13 +212,17 @@ void esp_openthread_platform_update(esp_openthread_mainloop_context_t *mainloop) esp_err_t esp_openthread_platform_process(otInstance *instance, const esp_openthread_mainloop_context_t *mainloop) { + esp_err_t error = ESP_OK; esp_openthread_platform_workflow_t *current_workflow = s_workflow_list; while (current_workflow) { - ESP_RETURN_ON_ERROR(current_workflow->process_func(instance, mainloop), OT_PLAT_LOG_TAG, "process %s failed", - current_workflow->name); + esp_err_t ret = current_workflow->process_func(instance, mainloop); + if (ret != ESP_OK) { + ESP_LOGE(OT_PLAT_LOG_TAG, "process %s failed", current_workflow->name); + error = ret; + } current_workflow = current_workflow->next; } - return ESP_OK; + return error; } uint32_t esp_openthread_get_alloc_caps(void) diff --git a/components/openthread/src/esp_openthread_task_queue.c b/components/openthread/src/esp_openthread_task_queue.c index d656b77dde2..1bd4c875d1e 100644 --- a/components/openthread/src/esp_openthread_task_queue.c +++ b/components/openthread/src/esp_openthread_task_queue.c @@ -63,9 +63,13 @@ esp_err_t IRAM_ATTR esp_openthread_task_queue_post(esp_openthread_task_t task, v BaseType_t task_woken = pdFALSE; if (!xPortCanYield()) { + ESP_RETURN_ON_FALSE_ISR(s_task_queue != NULL && s_task_queue_event_fd >= 0, ESP_ERR_INVALID_STATE, OT_PLAT_LOG_TAG, + "OpenThread task queue not initialized"); ESP_RETURN_ON_FALSE_ISR(xQueueSendFromISR(s_task_queue, &task_storage, &task_woken), ESP_FAIL, OT_PLAT_LOG_TAG, "Failed to post task to OpenThread task queue"); } else { + ESP_RETURN_ON_FALSE(s_task_queue != NULL && s_task_queue_event_fd >= 0, ESP_ERR_INVALID_STATE, OT_PLAT_LOG_TAG, + "OpenThread task queue not initialized"); ESP_RETURN_ON_FALSE(xQueueSend(s_task_queue, &task_storage, OT_TASK_QUEUE_SENDING_WAIT_TIME), ESP_FAIL, OT_PLAT_LOG_TAG, "Failed to post task to OpenThread task queue"); }