From cd1a5fc1f32eb5eb421875b14ce5ecebff6847a3 Mon Sep 17 00:00:00 2001 From: Yogesh Mantri Date: Wed, 9 Sep 2026 12:02:14 +0800 Subject: [PATCH 1/3] fix(console): join the REPL task before esp_console_stop_repl frees it --- components/console/esp_console_common.c | 10 ++++++++++ components/console/esp_console_repl_internal.c | 11 +++++++++++ components/console/private_include/console_private.h | 1 + 3 files changed, 22 insertions(+) diff --git a/components/console/esp_console_common.c b/components/console/esp_console_common.c index ba80b13806a..a0844af386b 100644 --- a/components/console/esp_console_common.c +++ b/components/console/esp_console_common.c @@ -33,6 +33,11 @@ void esp_console_repl_task(void *args) xSemaphoreTake(repl_com->state_mux, portMAX_DELAY); } + /* Tell esp_console_start_repl() that state_mux is now held. */ + if (repl_com->task_ready != NULL) { + xSemaphoreGive(repl_com->task_ready); + } + /* Change standard input and output of the task if the requested UART is * NOT the default one. This block will replace stdin, stdout and stderr. */ @@ -356,5 +361,10 @@ esp_err_t esp_console_start_repl(esp_console_repl_t *repl) repl_com->state = CONSOLE_REPL_STATE_START; xTaskNotifyGive(repl_com->task_hdl); + + /* Return only once the task holds state_mux, so a stop can join it. */ + if (repl_com->task_ready != NULL) { + xSemaphoreTake(repl_com->task_ready, portMAX_DELAY); + } return ESP_OK; } diff --git a/components/console/esp_console_repl_internal.c b/components/console/esp_console_repl_internal.c index b2c291d4676..ca766ea22af 100644 --- a/components/console/esp_console_repl_internal.c +++ b/components/console/esp_console_repl_internal.c @@ -92,6 +92,14 @@ esp_err_t esp_console_internal_set_event_fd(esp_console_repl_com_t *repl_com) } xSemaphoreGive(repl_com->state_mux); + repl_com->task_ready = xSemaphoreCreateBinary(); + if (repl_com->task_ready == NULL) { + ESP_LOGE(TAG, "task_ready create error"); + vSemaphoreDelete(repl_com->state_mux); + repl_com->state_mux = NULL; + return ESP_ERR_NO_MEM; + } + return ESP_OK; } @@ -123,6 +131,9 @@ esp_err_t esp_console_common_deinit(esp_console_repl_com_t *repl_com) vSemaphoreDelete(repl_com->state_mux); repl_com->state_mux = NULL; + vSemaphoreDelete(repl_com->task_ready); + repl_com->task_ready = NULL; + /* Unregister the heap function to avoid memory leak, since it is created * every time a console init is called. */ esp_err_t ret = esp_console_deregister_help_command(); diff --git a/components/console/private_include/console_private.h b/components/console/private_include/console_private.h index 863415ad623..45b986dd3c8 100644 --- a/components/console/private_include/console_private.h +++ b/components/console/private_include/console_private.h @@ -42,6 +42,7 @@ typedef struct { TaskHandle_t task_hdl; // REPL task handle size_t max_cmdline_length; // Maximum length of a command line. If 0, default value will be used. size_t max_cmdline_args; // Maximum number of command line arguments to parse. If 0, default value will be used. + SemaphoreHandle_t task_ready; // Given once the REPL task holds state_mux } esp_console_repl_com_t; typedef struct { From 4532e7e8c84e0672a3a6abf3f2533d97d10b6c8a Mon Sep 17 00:00:00 2001 From: Yogesh Mantri Date: Wed, 9 Sep 2026 16:49:01 +0800 Subject: [PATCH 2/3] fix(console): join with an empty semaphore instead of a given mutex --- components/console/esp_console.h | 1 + components/console/esp_console_common.c | 14 ------------- .../console/esp_console_repl_internal.c | 21 +++++++------------ .../console/private_include/console_private.h | 1 - 4 files changed, 8 insertions(+), 29 deletions(-) diff --git a/components/console/esp_console.h b/components/console/esp_console.h index 9bd5317f54c..73c8aed5c07 100644 --- a/components/console/esp_console.h +++ b/components/console/esp_console.h @@ -455,6 +455,7 @@ esp_err_t esp_console_start_repl(esp_console_repl_t *repl); * @brief Stop REPL environment * * @param[in] repl REPL handle returned from esp_console_new_repl_xxx + * @note This function waits for the console task to exit before it returns. * @return * - ESP_OK on success * - others on failure diff --git a/components/console/esp_console_common.c b/components/console/esp_console_common.c index a0844af386b..ac8dc65e7d2 100644 --- a/components/console/esp_console_common.c +++ b/components/console/esp_console_common.c @@ -29,15 +29,6 @@ void esp_console_repl_task(void *args) * function is called. */ ulTaskNotifyTake(pdTRUE, portMAX_DELAY); - if (repl_com->state_mux != NULL) { - xSemaphoreTake(repl_com->state_mux, portMAX_DELAY); - } - - /* Tell esp_console_start_repl() that state_mux is now held. */ - if (repl_com->task_ready != NULL) { - xSemaphoreGive(repl_com->task_ready); - } - /* Change standard input and output of the task if the requested UART is * NOT the default one. This block will replace stdin, stdout and stderr. */ @@ -361,10 +352,5 @@ esp_err_t esp_console_start_repl(esp_console_repl_t *repl) repl_com->state = CONSOLE_REPL_STATE_START; xTaskNotifyGive(repl_com->task_hdl); - - /* Return only once the task holds state_mux, so a stop can join it. */ - if (repl_com->task_ready != NULL) { - xSemaphoreTake(repl_com->task_ready, portMAX_DELAY); - } return ESP_OK; } diff --git a/components/console/esp_console_repl_internal.c b/components/console/esp_console_repl_internal.c index ca766ea22af..1186c05a64d 100644 --- a/components/console/esp_console_repl_internal.c +++ b/components/console/esp_console_repl_internal.c @@ -85,20 +85,13 @@ esp_err_t esp_console_internal_set_event_fd(esp_console_repl_com_t *repl_com) return ESP_FAIL; } - repl_com->state_mux = xSemaphoreCreateMutex(); + /* Empty on purpose, and the task only ever gives it, never takes it: a + * mutex created given lets a stop take it before the task has run. */ + repl_com->state_mux = xSemaphoreCreateBinary(); if (repl_com->state_mux == NULL) { ESP_LOGE(TAG, "state_mux create error"); return ESP_ERR_NO_MEM; } - xSemaphoreGive(repl_com->state_mux); - - repl_com->task_ready = xSemaphoreCreateBinary(); - if (repl_com->task_ready == NULL) { - ESP_LOGE(TAG, "task_ready create error"); - vSemaphoreDelete(repl_com->state_mux); - repl_com->state_mux = NULL; - return ESP_ERR_NO_MEM; - } return ESP_OK; } @@ -111,6 +104,7 @@ esp_err_t esp_console_common_deinit(esp_console_repl_com_t *repl_com) { // set the state to deinit to force the while loop in // esp_console_repl_task to break + const bool was_started = (repl_com->state == CONSOLE_REPL_STATE_START); repl_com->state = CONSOLE_REPL_STATE_DEINIT; if (s_interrupt_reading_fd == -1) { @@ -125,15 +119,14 @@ esp_err_t esp_console_common_deinit(esp_console_repl_com_t *repl_com) // wait for the task to notify that // esp_console_repl_task returned assert(repl_com->state_mux != NULL); - xSemaphoreTake(repl_com->state_mux, portMAX_DELAY); + if (was_started) { + xSemaphoreTake(repl_com->state_mux, portMAX_DELAY); + } // delete the semaphore for the repl state vSemaphoreDelete(repl_com->state_mux); repl_com->state_mux = NULL; - vSemaphoreDelete(repl_com->task_ready); - repl_com->task_ready = NULL; - /* Unregister the heap function to avoid memory leak, since it is created * every time a console init is called. */ esp_err_t ret = esp_console_deregister_help_command(); diff --git a/components/console/private_include/console_private.h b/components/console/private_include/console_private.h index 45b986dd3c8..863415ad623 100644 --- a/components/console/private_include/console_private.h +++ b/components/console/private_include/console_private.h @@ -42,7 +42,6 @@ typedef struct { TaskHandle_t task_hdl; // REPL task handle size_t max_cmdline_length; // Maximum length of a command line. If 0, default value will be used. size_t max_cmdline_args; // Maximum number of command line arguments to parse. If 0, default value will be used. - SemaphoreHandle_t task_ready; // Given once the REPL task holds state_mux } esp_console_repl_com_t; typedef struct { From a287bbc25e127510c86c316a791cffc59146bbb5 Mon Sep 17 00:00:00 2001 From: Yogesh Mantri Date: Wed, 9 Sep 2026 18:48:49 +0800 Subject: [PATCH 3/3] fix(console): delete state_mux when REPL creation fails --- components/console/esp_console_common.c | 4 ++++ components/console/esp_console_repl_chip.c | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/components/console/esp_console_common.c b/components/console/esp_console_common.c index ac8dc65e7d2..cbd58f07b94 100644 --- a/components/console/esp_console_common.c +++ b/components/console/esp_console_common.c @@ -334,6 +334,10 @@ esp_err_t esp_console_new_repl_stdio(const esp_console_repl_config_t *repl_confi _exit: if (universal_repl) { esp_console_deinit(); + /* Only common_deinit() deletes state_mux, and the weak set_event_fd() never creates it. */ + if (universal_repl->repl_com.state_mux) { + vSemaphoreDelete(universal_repl->repl_com.state_mux); + } free(universal_repl); } if (ret_repl) { diff --git a/components/console/esp_console_repl_chip.c b/components/console/esp_console_repl_chip.c index b16c090ce33..cc3c42d9788 100644 --- a/components/console/esp_console_repl_chip.c +++ b/components/console/esp_console_repl_chip.c @@ -152,6 +152,10 @@ static esp_err_t esp_console_new_repl_uart_legacy(const esp_console_dev_uart_con _exit: if (uart_repl) { esp_console_deinit(); + /* Only common_deinit() deletes state_mux, and the weak set_event_fd() never creates it. */ + if (uart_repl->repl_com.state_mux) { + vSemaphoreDelete(uart_repl->repl_com.state_mux); + } free(uart_repl); } if (ret_repl) {