fix(console): add ability to unblock linenoise, to fix deadlock

Closes https://github.com/espressif/esp-idf/pull/10580
Closes https://github.com/espressif/esp-idf/issues/9974
This commit is contained in:
Guillaume Souchere
2025-03-05 12:23:08 +01:00
parent 2c990a6933
commit 5b2f2e05f3
7 changed files with 232 additions and 53 deletions

View File

@@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2024-2025 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
@@ -10,9 +10,14 @@
#include "console_private.h"
#include "esp_log.h"
#include "linenoise/linenoise.h"
#include "esp_vfs_eventfd.h"
static const char *TAG = "console.common";
static bool s_init = false;
static int s_interrupt_reading_fd = -1;
static uint64_t s_interrupt_reading_signal = 1;
esp_err_t esp_console_setup_prompt(const char *prompt, esp_console_repl_com_t *repl_com)
{
/* set command line prompt */
@@ -94,8 +99,33 @@ esp_err_t esp_console_common_init(size_t max_cmdline_length, esp_console_repl_co
linenoiseSetCompletionCallback(&esp_console_get_completion);
linenoiseSetHintsCallback((linenoiseHintsCallback *)&esp_console_get_hint);
/* Tell linenoise what file descriptor to add to the read file descriptor set,
* that will be used to signal a read termination */
esp_vfs_eventfd_config_t config = ESP_VFS_EVENTD_CONFIG_DEFAULT();
ret = esp_vfs_eventfd_register(&config);
if (ret == ESP_OK) {
/* first time calling the eventfd register function */
s_interrupt_reading_fd = eventfd(0, 0);
linenoiseSetInterruptReadingData(s_interrupt_reading_fd, s_interrupt_reading_signal);
} else if (ret == ESP_ERR_INVALID_STATE) {
/* the evenfd has already been registered*/
} else {
/* issue with arg, this should not happen */
goto _exit;
}
repl_com->state_mux = xSemaphoreCreateMutex();
if (repl_com->state_mux == NULL) {
ESP_LOGE(TAG, "state_mux create error");
ret = ESP_ERR_NO_MEM;
goto _exit;
}
xSemaphoreGive(repl_com->state_mux);
s_init = true;
return ESP_OK;
_exit:
s_init = false;
return ret;
}
@@ -126,6 +156,10 @@ void esp_console_repl_task(void *args)
* function is called. */
ulTaskNotifyTake(pdTRUE, portMAX_DELAY);
assert(repl_com->state_mux != NULL);
BaseType_t ret_val = xSemaphoreTake(repl_com->state_mux, portMAX_DELAY);
assert(ret_val == pdTRUE);
/* 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.
*/
@@ -187,6 +221,18 @@ void esp_console_repl_task(void *args)
/* linenoise allocates line buffer on the heap, so need to free it */
linenoiseFree(line);
}
xSemaphoreGive(repl_com->state_mux);
ESP_LOGD(TAG, "The End");
vTaskDelete(NULL);
}
esp_err_t esp_console_interrupt_reading(void)
{
if (!s_init) {
return ESP_FAIL;
}
ssize_t ret = write(s_interrupt_reading_fd, &s_interrupt_reading_signal, sizeof(s_interrupt_reading_signal));
assert(ret == sizeof(s_interrupt_reading_signal));
return ESP_OK;
}

View File

@@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2016-2024 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2016-2025 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
@@ -12,13 +12,14 @@
#include "esp_err.h"
#include "esp_log.h"
#include "esp_console.h"
#include "esp_vfs_cdcacm.h"
#include "driver/usb_serial_jtag_vfs.h"
#include "freertos/FreeRTOS.h"
#include "freertos/task.h"
#include "driver/uart.h"
#include "driver/uart_vfs.h"
#include "driver/usb_serial_jtag.h"
#include "driver/usb_serial_jtag_vfs.h"
#include "esp_private/usb_console.h"
#include "esp_vfs_cdcacm.h"
#include "console_private.h"
@@ -294,7 +295,26 @@ static esp_err_t esp_console_repl_uart_delete(esp_console_repl_t *repl)
ret = ESP_ERR_INVALID_STATE;
goto _exit;
}
// set the state to deinit to force the while loop in
// esp_console_repl_task to break
repl_com->state = CONSOLE_REPL_STATE_DEINIT;
const esp_err_t read_interrupted = esp_console_interrupt_reading();
if (read_interrupted != ESP_OK) {
return ESP_FAIL;
}
// wait for the task to notify that
// esp_console_repl_task returned
assert(repl_com->state_mux != NULL);
BaseType_t ret_val = xSemaphoreTake(repl_com->state_mux, portMAX_DELAY);
assert(ret_val == pdTRUE);
// delete the semaphore for the repl state
vSemaphoreDelete(repl_com->state_mux);
repl_com->state_mux = NULL;
esp_console_deinit();
uart_vfs_dev_use_nonblocking(uart_repl->uart_channel);
uart_driver_delete(uart_repl->uart_channel);
@@ -316,7 +336,26 @@ static esp_err_t esp_console_repl_usb_cdc_delete(esp_console_repl_t *repl)
ret = ESP_ERR_INVALID_STATE;
goto _exit;
}
// set the state to deinit to force the while loop in
// esp_console_repl_task to break
repl_com->state = CONSOLE_REPL_STATE_DEINIT;
const esp_err_t read_interrupted = esp_console_interrupt_reading();
if (read_interrupted != ESP_OK) {
return ESP_FAIL;
}
// wait for the task to notify that
// esp_console_repl_task returned
assert(repl_com->state_mux != NULL);
BaseType_t ret_val = xSemaphoreTake(repl_com->state_mux, portMAX_DELAY);
assert(ret_val == pdTRUE);
// delete the semaphore for the repl state
vSemaphoreDelete(repl_com->state_mux);
repl_com->state_mux = NULL;
esp_console_deinit();
free(cdc_repl);
_exit:
@@ -336,7 +375,26 @@ static esp_err_t esp_console_repl_usb_serial_jtag_delete(esp_console_repl_t *rep
ret = ESP_ERR_INVALID_STATE;
goto _exit;
}
// set the state to deinit to force the while loop in
// esp_console_repl_task to break
repl_com->state = CONSOLE_REPL_STATE_DEINIT;
const esp_err_t read_interrupted = esp_console_interrupt_reading();
if (read_interrupted != ESP_OK) {
return ESP_FAIL;
}
// wait for the task to notify that
// esp_console_repl_task returned
assert(repl_com->state_mux != NULL);
BaseType_t ret_val = xSemaphoreTake(repl_com->state_mux, portMAX_DELAY);
assert(ret_val == pdTRUE);
// delete the semaphore for the repl state
vSemaphoreDelete(repl_com->state_mux);
repl_com->state_mux = NULL;
esp_console_deinit();
usb_serial_jtag_vfs_use_nonblocking();
usb_serial_jtag_driver_uninstall();

View File

@@ -122,6 +122,7 @@
#include <sys/types.h>
#include <sys/fcntl.h>
#include <sys/time.h>
#include <sys/param.h>
#include <unistd.h>
#include <assert.h>
#include "linenoise.h"
@@ -144,6 +145,9 @@ static int history_len = 0;
static char **history = NULL;
static bool allow_empty = true;
static int intr_reading_fd = -1;
static uint64_t intr_reading_signal = -1;
/* The linenoiseState structure represents the state during line editing.
* We pass this state to functions implementing specific editing
* functionalities. */
@@ -239,6 +243,37 @@ static void flushWrite(void) {
fsync(fileno(stdout));
}
static int linenoiseReadBytes(int fd, void *buf, size_t max_bytes)
{
fd_set read_fds;
FD_ZERO(&read_fds);
FD_SET(fd, &read_fds);
if (intr_reading_fd !=-1) {
FD_SET(intr_reading_fd, &read_fds);
}
int maxFd = MAX(fd, intr_reading_fd);
/* call select to wait for either a read ready or an except to happen */
int nread = select(maxFd + 1, &read_fds, NULL, NULL, NULL);
if (nread < 0) {
return -1;
}
if(FD_ISSET(intr_reading_fd, &read_fds)) {
/* read termination request happened, return */
int buf[sizeof(intr_reading_signal)];
nread = read(intr_reading_fd, buf, sizeof(intr_reading_signal));
if ((nread == sizeof(intr_reading_signal)) && (buf[0] == intr_reading_signal)) {
return -1;
}
}
else if(FD_ISSET(fd, &read_fds)) {
/* a read ready triggered the select to return. call the
* read function with the number of bytes max_bytes */
nread = read(fd, buf, max_bytes);
}
return nread;
}
/* Use the ESC [6n escape sequence to query the horizontal cursor position
* and return it. On error -1 is returned, on success the position of the
* cursor. */
@@ -273,7 +308,7 @@ static int getCursorPosition(void) {
/* Keep using unistd's functions. Here, using `read` instead of `fgets`
* or `fgets` guarantees us that we we can read a byte regardless on
* whether the sender sent end of line character(s) (CR, CRLF, LF). */
if (read(in_fd, buf + i, 1) != 1 || buf[i] == 'R') {
if (linenoiseReadBytes(in_fd, buf + i, 1) != 1 || buf[i] == 'R') {
/* If we couldn't read a byte from STDIN or if 'R' was received,
* the transmission is finished. */
break;
@@ -413,7 +448,7 @@ static int completeLine(struct linenoiseState *ls) {
refreshLine(ls);
}
nread = read(in_fd, &c, 1);
nread = linenoiseReadBytes(in_fd, &c, 1);
if (nread <= 0) {
freeCompletions(&lc);
return -1;
@@ -456,6 +491,12 @@ void linenoiseSetHintsCallback(linenoiseHintsCallback *fn) {
hintsCallback = fn;
}
void linenoiseSetInterruptReadingData(int fd, uint64_t data)
{
intr_reading_fd = fd;
intr_reading_signal = data;
}
/* Register a function to free the hints returned by the hints callback
* registered with linenoiseSetHintsCallback(). */
void linenoiseSetFreeHintsCallback(linenoiseFreeHintsCallback *fn) {
@@ -890,8 +931,6 @@ static int linenoiseEdit(char *buf, size_t buflen, const char *prompt)
while(1) {
char c;
int nread;
char seq[3];
/**
* To determine whether the user is pasting data or typing itself, we
@@ -903,8 +942,10 @@ static int linenoiseEdit(char *buf, size_t buflen, const char *prompt)
* about 40ms (or even more)
*/
t1 = getMillis();
nread = read(in_fd, &c, 1);
if (nread <= 0) return l.len;
int nread = linenoiseReadBytes(in_fd, &c, 1);
if (nread <= 0) {
return l.len;
}
if ( (getMillis() - t1) < LINENOISE_PASTE_KEY_DELAY && c != ENTER) {
/* Pasting data, insert characters without formatting.
@@ -946,7 +987,7 @@ static int linenoiseEdit(char *buf, size_t buflen, const char *prompt)
errno = EAGAIN;
return -1;
case BACKSPACE: /* backspace */
case 8: /* ctrl-h */
case CTRL_H: /* ctrl-h */
linenoiseEditBackspace(&l);
break;
case CTRL_D: /* ctrl-d, remove char at right of cursor, or if the
@@ -980,18 +1021,42 @@ static int linenoiseEdit(char *buf, size_t buflen, const char *prompt)
case CTRL_N: /* ctrl-n */
linenoiseEditHistoryNext(&l, LINENOISE_HISTORY_NEXT);
break;
case ESC: /* escape sequence */
/* Read the next two bytes representing the escape sequence. */
if (read(in_fd, seq, 2) < 2) {
break;
}
case CTRL_U: /* Ctrl+u, delete the whole line. */
buf[0] = '\0';
l.pos = l.len = 0;
refreshLine(&l);
break;
case CTRL_K: /* Ctrl+k, delete from current to end of line. */
buf[l.pos] = '\0';
l.len = l.pos;
refreshLine(&l);
break;
case CTRL_A: /* Ctrl+a, go to the start of the line */
linenoiseEditMoveHome(&l);
break;
case CTRL_E: /* ctrl+e, go to the end of the line */
linenoiseEditMoveEnd(&l);
break;
case CTRL_L: /* ctrl+l, clear screen */
linenoiseClearScreen();
refreshLine(&l);
break;
case CTRL_W: /* ctrl+w, delete previous word */
linenoiseEditDeletePrevWord(&l);
break;
case ESC: { /* escape sequence */
/* ESC [ sequences. */
char seq[3];
int r = linenoiseReadBytes(in_fd, seq, 2);
if (r != 2) {
return -1;
}
if (seq[0] == '[') {
if (seq[1] >= '0' && seq[1] <= '9') {
/* Extended escape, read additional byte. */
if (read(in_fd, seq+2, 1) == -1) {
break;
r = linenoiseReadBytes(in_fd, seq + 2, 1);
if (r != 1) {
return -1;
}
if (seq[2] == '~') {
switch(seq[1]) {
@@ -1023,7 +1088,6 @@ static int linenoiseEdit(char *buf, size_t buflen, const char *prompt)
}
}
}
/* ESC O sequences. */
else if (seq[0] == 'O') {
switch(seq[1]) {
@@ -1036,32 +1100,10 @@ static int linenoiseEdit(char *buf, size_t buflen, const char *prompt)
}
}
break;
}
default:
if (linenoiseEditInsert(&l,c)) return -1;
break;
case CTRL_U: /* Ctrl+u, delete the whole line. */
buf[0] = '\0';
l.pos = l.len = 0;
refreshLine(&l);
break;
case CTRL_K: /* Ctrl+k, delete from current to end of line. */
buf[l.pos] = '\0';
l.len = l.pos;
refreshLine(&l);
break;
case CTRL_A: /* Ctrl+a, go to the start of the line */
linenoiseEditMoveHome(&l);
break;
case CTRL_E: /* ctrl+e, go to the end of the line */
linenoiseEditMoveEnd(&l);
break;
case CTRL_L: /* ctrl+l, clear screen */
linenoiseClearScreen();
refreshLine(&l);
break;
case CTRL_W: /* ctrl+w, delete previous word */
linenoiseEditDeletePrevWord(&l);
break;
}
flushWrite();
}
@@ -1103,12 +1145,7 @@ int linenoiseProbe(void) {
}
read_bytes += cb;
}
/* Restore old mode */
flags &= ~O_NONBLOCK;
res = fcntl(stdin_fileno, F_SETFL, flags);
if (res != 0) {
return -1;
}
if (read_bytes < 4) {
return -2;
}
@@ -1133,9 +1170,17 @@ static int linenoiseDumb(char* buf, size_t buflen, const char* prompt) {
/* dumb terminal, fall back to fgets */
fputs(prompt, stdout);
flushWrite();
size_t count = 0;
const int in_fd = fileno(stdin);
char c = 'c';
while (count < buflen) {
int c = fgetc(stdin);
int nread = linenoiseReadBytes(in_fd, &c, 1);
if (nread < 0) {
return nread;
}
if (c == '\n') {
break;
} else if (c == BACKSPACE || c == CTRL_H) {

View File

@@ -45,6 +45,7 @@ extern "C" {
#include <stdbool.h>
#include <stddef.h>
#include <stdint.h>
typedef struct linenoiseCompletions {
size_t len;
@@ -57,6 +58,7 @@ typedef void(linenoiseFreeHintsCallback)(void *);
void linenoiseSetCompletionCallback(linenoiseCompletionCallback *);
void linenoiseSetHintsCallback(linenoiseHintsCallback *);
void linenoiseSetFreeHintsCallback(linenoiseFreeHintsCallback *);
void linenoiseSetInterruptReadingData(int, uint64_t);
void linenoiseAddCompletion(linenoiseCompletions *, const char *);
int linenoiseProbe(void);

View File

@@ -11,6 +11,7 @@
#include "sdkconfig.h"
#include "freertos/FreeRTOS.h"
#include "freertos/task.h"
#include "freertos/semphr.h"
#include "esp_console.h"
#define CONSOLE_PROMPT_MAX_LEN (32)
@@ -32,6 +33,7 @@ typedef struct {
esp_console_repl_t repl_core; // base class
char prompt[CONSOLE_PROMPT_MAX_LEN]; // Prompt to be printed before each line
repl_state_t state;
SemaphoreHandle_t state_mux;
const char *history_save_path;
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.
@@ -49,3 +51,4 @@ esp_err_t esp_console_setup_prompt(const char *prompt, esp_console_repl_com_t *r
esp_err_t esp_console_setup_history(const char *history_path,
uint32_t max_history_len,
esp_console_repl_com_t *repl_com);
esp_err_t esp_console_interrupt_reading(void);

View File

@@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2022-2024 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2022-2025 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
@@ -127,8 +127,7 @@ TEST_CASE("esp console init/deinit with context test", "[console]")
static int do_cmd_quit(int argc, char **argv)
{
printf("ByeBye\r\n");
s_repl->del(s_repl);
TEST_ESP_OK(s_repl->del(s_repl));
linenoiseHistoryFree(); // Free up memory
return 0;
@@ -155,6 +154,27 @@ TEST_CASE("esp console repl test", "[console][ignore]")
vTaskDelay(pdMS_TO_TICKS(2000));
}
TEST_CASE("esp console repl deinit", "[console][ignore]")
{
esp_console_repl_config_t repl_config = ESP_CONSOLE_REPL_CONFIG_DEFAULT();
esp_console_dev_uart_config_t uart_config = ESP_CONSOLE_DEV_UART_CONFIG_DEFAULT();
TEST_ESP_OK(esp_console_new_repl_uart(&uart_config, &repl_config, &s_repl));
/* start the repl task */
TEST_ESP_OK(esp_console_start_repl(s_repl));
/* wait to make sure the task reaches linenoiseEdit function
* and gets stuck in the select */
vTaskDelay(pdMS_TO_TICKS(500));
/* call the delete function, this returns only when the repl task terminated */
const esp_err_t res = s_repl->del(s_repl);
/* if this point is reached, the repl environment has been deleted successfully */
TEST_ASSERT(res == ESP_OK);
printf("-------------- %p\n", s_repl);
}
static const esp_console_cmd_t cmd_a = {
.command = "aaa",
.help = "should appear first in help",

View File

@@ -12,6 +12,11 @@ def do_test_quit(dut: Dut) -> None:
dut.confirm_write('quit', expect_str='ByeBye')
def do_test_repl_deinit(dut: Dut) -> None:
dut.expect_exact('Press ENTER to see the list of tests')
dut.confirm_write('"esp console repl deinit"', expect_str='esp>')
def do_test_help_generic(dut: Dut, registration_order: str) -> None:
dut.expect_exact('Press ENTER to see the list of tests')
dut.confirm_write('"esp console help command - {} registration"'.format(registration_order), expect_str='esp>')