Skip to content

Commit

Permalink
[Console] fix console deinit: add ability to unblock linenoise, to fi…
Browse files Browse the repository at this point in the history
…x deadlock
  • Loading branch information
chipweinberger committed Jan 20, 2023
1 parent 84be4db commit 02eb8d6
Show file tree
Hide file tree
Showing 8 changed files with 172 additions and 13 deletions.
35 changes: 35 additions & 0 deletions components/console/esp_console_repl.c
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ typedef enum {
CONSOLE_REPL_STATE_DEINIT,
CONSOLE_REPL_STATE_INIT,
CONSOLE_REPL_STATE_START,
CONSOLE_REPL_STATE_REPL_STOP_REQUESTED,
CONSOLE_REPL_STATE_REPL_TASK_ENDED,
} repl_state_t;

typedef struct {
Expand Down Expand Up @@ -409,13 +411,25 @@ static esp_err_t esp_console_repl_uart_delete(esp_console_repl_t *repl)
esp_err_t ret = ESP_OK;
esp_console_repl_com_t *repl_com = __containerof(repl, esp_console_repl_com_t, repl_core);
esp_console_repl_universal_t *uart_repl = __containerof(repl_com, esp_console_repl_universal_t, repl_com);

// check if already de-initialized
if (repl_com->state == CONSOLE_REPL_STATE_DEINIT) {
ESP_LOGE(TAG, "already de-initialized");
ret = ESP_ERR_INVALID_STATE;
goto _exit;
}

// wait for repl thread to stop, if it is running
if (repl_com->state == CONSOLE_REPL_STATE_START) {
repl_com->state = CONSOLE_REPL_STATE_REPL_STOP_REQUESTED;
while (repl_com->state != CONSOLE_REPL_STATE_REPL_TASK_ENDED) {
uart_unblock_reads();
vTaskDelay(1);
}
}

repl_com->state = CONSOLE_REPL_STATE_DEINIT;

esp_console_deinit();
esp_vfs_dev_uart_use_nonblocking(uart_repl->uart_channel);
uart_driver_delete(uart_repl->uart_channel);
Expand All @@ -429,13 +443,21 @@ static esp_err_t esp_console_repl_usb_cdc_delete(esp_console_repl_t *repl)
esp_err_t ret = ESP_OK;
esp_console_repl_com_t *repl_com = __containerof(repl, esp_console_repl_com_t, repl_core);
esp_console_repl_universal_t *cdc_repl = __containerof(repl_com, esp_console_repl_universal_t, repl_com);

// check if already de-initialized
if (repl_com->state == CONSOLE_REPL_STATE_DEINIT) {
ESP_LOGE(TAG, "already de-initialized");
ret = ESP_ERR_INVALID_STATE;
goto _exit;
}

// TODO: wait for repl thread to stop, if it is running.
// Need to implement a USB CDC driver, and a
// corresponding usb_cdc_unblock_reads() function.
// See other esp_console_repl_X_delete() functions for reference.

repl_com->state = CONSOLE_REPL_STATE_DEINIT;

esp_console_deinit();
free(cdc_repl);
_exit:
Expand All @@ -454,7 +476,18 @@ static esp_err_t esp_console_repl_usb_serial_jtag_delete(esp_console_repl_t *rep
ret = ESP_ERR_INVALID_STATE;
goto _exit;
}

// wait for repl thread to stop, if it is running
if (repl_com->state == CONSOLE_REPL_STATE_START) {
repl_com->state = CONSOLE_REPL_STATE_REPL_STOP_REQUESTED;
while (repl_com->state != CONSOLE_REPL_STATE_REPL_TASK_ENDED) {
usb_serial_jtag_unblock_reads();
vTaskDelay(1);
}
}

repl_com->state = CONSOLE_REPL_STATE_DEINIT;

esp_console_deinit();
esp_vfs_usb_serial_jtag_use_nonblocking();
usb_serial_jtag_driver_uninstall();
Expand Down Expand Up @@ -535,6 +568,8 @@ static void esp_console_repl_task(void *args)
/* linenoise allocates line buffer on the heap, so need to free it */
linenoiseFree(line);
}

repl_com->state = CONSOLE_REPL_STATE_REPL_TASK_ENDED;
ESP_LOGD(TAG, "The End");
vTaskDelete(NULL);
}
74 changes: 62 additions & 12 deletions components/console/linenoise/linenoise.c
Original file line number Diff line number Diff line change
Expand Up @@ -221,8 +221,8 @@ static void flushWrite(void) {
}

/* 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. */
* and return it. On read error -1 is returned, On cursor error, -2 is returned.
* On success the position of the cursor. */
static int getCursorPosition(void) {
char buf[LINENOISE_COMMAND_MAX_LEN] = { 0 };
int cols = 0;
Expand All @@ -248,10 +248,16 @@ static int getCursorPosition(void) {
* Stop right before the last character of the buffer, to be able to NULL
* terminate it. */
while (i < sizeof(buf)-1) {

int nread = read(in_fd, buf + i, 1);
if (nread == -1 && errno == EWOULDBLOCK) {
return -1; // read error
}

/* 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 (nread != 1 || buf[i] == 'R') {
/* If we couldn't read a byte from STDIN or if 'R' was received,
* the transmission is finished. */
break;
Expand All @@ -270,7 +276,8 @@ static int getCursorPosition(void) {

/* Parse the received data to get the position of the cursor. */
if (buf[0] != ESC || buf[1] != '[' || sscanf(buf+2,"%d;%d",&rows,&cols) != 2) {
return -1;
// cursor error
return -2;
}
return cols;
}
Expand All @@ -294,7 +301,10 @@ static int getColumns(void) {

/* Get the initial position so we can restore it later. */
start = getCursorPosition();
if (start == -1) {
if (start == -1 && errno == EWOULDBLOCK) {
return -1;
}
if (start == -2) {
goto failed;
}

Expand All @@ -308,7 +318,10 @@ static int getColumns(void) {
/* After sending this command, we can get the new position of the cursor,
* we'd get the size, in columns, of the opened TTY. */
cols = getCursorPosition();
if (cols == -1) {
if (cols == -1 && errno == EWOULDBLOCK) {
return -1;
}
if (cols == -2) {
goto failed;
}

Expand Down Expand Up @@ -819,6 +832,11 @@ static int linenoiseEdit(char *buf, size_t buflen, const char *prompt)
int out_fd = fileno(stdout);
int in_fd = fileno(stdin);

int cols = getColumns();
if (cols == -1 && errno == EWOULDBLOCK) {
return -1;
}

/* Populate the linenoise state that we pass to functions implementing
* specific editing functionalities. */
l.buf = buf;
Expand All @@ -827,7 +845,7 @@ static int linenoiseEdit(char *buf, size_t buflen, const char *prompt)
l.plen = strlen(prompt);
l.oldpos = l.pos = 0;
l.len = 0;
l.cols = getColumns();
l.cols = cols;
l.maxrows = 0;
l.history_index = 0;

Expand All @@ -840,11 +858,20 @@ static int linenoiseEdit(char *buf, size_t buflen, const char *prompt)
linenoiseHistoryAdd("");

int pos1 = getCursorPosition();
if (pos1 == -1 && errno == EWOULDBLOCK) {
return -1;
}

if (write(out_fd, prompt,l.plen) == -1) {
return -1;
}
flushWrite();

int pos2 = getCursorPosition();
if (pos2 == -1 && errno == EWOULDBLOCK) {
return -1;
}

if (pos1 >= 0 && pos2 >= 0) {
l.plen = pos2 - pos1;
}
Expand All @@ -864,7 +891,12 @@ static int linenoiseEdit(char *buf, size_t buflen, const char *prompt)
*/
t1 = getMillis();
nread = read(in_fd, &c, 1);
if (nread <= 0) return l.len;
if (nread == -1 && errno == EWOULDBLOCK) {
return -1;
}
if (nread <= 0) {
return l.len;
}

if ( (getMillis() - t1) < LINENOISE_PASTE_KEY_DELAY && c != ENTER) {
/* Pasting data, insert characters without formatting.
Expand Down Expand Up @@ -940,17 +972,25 @@ 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 */
case ESC: { /* escape sequence */
/* Read the next two bytes representing the escape sequence. */
if (read(in_fd, seq, 2) < 2) {
int nread = read(in_fd, seq, 2);
if (nread == -1 && errno == EWOULDBLOCK) {
return -1;
}
if (nread < 2) {
break;
}

/* ESC [ sequences. */
if (seq[0] == '[') {
if (seq[1] >= '0' && seq[1] <= '9') {
/* Extended escape, read additional byte. */
if (read(in_fd, seq+2, 1) == -1) {
int nread = read(in_fd, seq+2, 1);
if (nread == -1 && errno == EWOULDBLOCK) {
return -1;
}
if (nread == -1) {
break;
}
if (seq[2] == '~') {
Expand Down Expand Up @@ -996,6 +1036,7 @@ static int linenoiseEdit(char *buf, size_t buflen, const char *prompt)
}
}
break;
}
default:
if (linenoiseEditInsert(&l,c)) return -1;
break;
Expand Down Expand Up @@ -1054,6 +1095,8 @@ int linenoiseProbe(void) {
timeout_ms -= retry_ms;
char c;
int cb = read(stdin_fileno, &c, 1);
// Note! Due to the fcntl call above, this is read in non-blocking mode!
// So, nread == -1 && errno == EWOULDBLOCK are expected here!
if (cb < 0) {
continue;
}
Expand Down Expand Up @@ -1084,6 +1127,9 @@ static int linenoiseRaw(char *buf, size_t buflen, const char *prompt) {
}

count = linenoiseEdit(buf, buflen, prompt);
if (count == -1 && errno == EWOULDBLOCK) {
return -1;
}
fputc('\n', stdout);
flushWrite();
return count;
Expand All @@ -1095,9 +1141,13 @@ static int linenoiseDumb(char* buf, size_t buflen, const char* prompt) {
size_t count = 0;
while (count < buflen) {
int c = fgetc(stdin);
if (c == -1 && errno == EWOULDBLOCK) {
return -1;
}

if (c == '\n') {
break;
} else if (c >= 0x1c && c <= 0x1f){
} else if (c >= 0x1c && c <= 0x1f) {
continue; /* consume arrow keys */
} else if (c == BACKSPACE || c == 0x8) {
if (count > 0) {
Expand Down
11 changes: 11 additions & 0 deletions components/driver/include/driver/uart.h
Original file line number Diff line number Diff line change
Expand Up @@ -823,6 +823,17 @@ esp_err_t uart_wait_tx_idle_polling(uart_port_t uart_num);
*/
esp_err_t uart_set_loop_back(uart_port_t uart_num, bool loop_back_en);

/**
* @brief Unblocks uart_read_bytes() if it is currently waiting to receive bytes.
*
* This will cause uart_read_bytes() to return -1 with errno set to EWOULDBLOCK.
*
* @return
* - ESP_OK Success
* - ESP_INVALID_STATE If the Uart driver has not been installed
*/
esp_err_t uart_unblock_reads(uart_port_t uart_num);

/**
* @brief Configure behavior of UART RX timeout interrupt.
*
Expand Down
11 changes: 11 additions & 0 deletions components/driver/include/driver/usb_serial_jtag.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,17 @@ typedef struct {
*/
esp_err_t usb_serial_jtag_driver_install(usb_serial_jtag_driver_config_t *usb_serial_jtag_config);

/**
* @brief Unblocks usb_serial_jtag_read_bytes() if it is currently waiting to receive bytes.
*
* This will cause usb_serial_jtag_read_bytes() to return -1 with errno set to EWOULDBLOCK.
*
* @return
* - ESP_OK Success
* - ESP_INVALID_STATE If the Usb Serial JTAG driver has not been installed
*/
esp_err_t usb_serial_jtag_unblock_reads();

/**
* @brief USB_SERIAL_JTAG read bytes from USB_SERIAL_JTAG buffer
*
Expand Down
10 changes: 10 additions & 0 deletions components/driver/uart.c
Original file line number Diff line number Diff line change
Expand Up @@ -1797,6 +1797,16 @@ esp_err_t uart_set_loop_back(uart_port_t uart_num, bool loop_back_en)
return ESP_OK;
}

esp_err_t uart_unblock_reads(uart_port_t uart_num)
{
if (p_uart_obj[uart_num]->rx_ring_buf == NULL) {
return ESP_ERR_INVALID_STATE;
}

vRingbufferUnblockRx(pp_uart_obj[uart_num]->rx_ring_buf);
return ESP_OK;
}

void uart_set_always_rx_timeout(uart_port_t uart_num, bool always_rx_timeout)
{
uint16_t rx_tout = uart_hal_get_rx_tout_thr(&(uart_context[uart_num].hal));
Expand Down
10 changes: 10 additions & 0 deletions components/driver/usb_serial_jtag.c
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,16 @@ esp_err_t usb_serial_jtag_driver_install(usb_serial_jtag_driver_config_t *usb_se
return err;
}

esp_err_t usb_serial_jtag_unblock_reads()
{
if (p_usb_serial_jtag_obj->rx_ring_buf == NULL) {
return ESP_ERR_INVALID_STATE;
}

vRingbufferUnblockRx(p_usb_serial_jtag_obj->rx_ring_buf);
return ESP_OK;
}

int usb_serial_jtag_read_bytes(void* buf, uint32_t length, TickType_t ticks_to_wait)
{
uint8_t *data = NULL;
Expand Down
10 changes: 10 additions & 0 deletions components/esp_ringbuf/include/freertos/ringbuf.h
Original file line number Diff line number Diff line change
Expand Up @@ -496,6 +496,16 @@ void vRingbufferGetInfo(RingbufHandle_t xRingbuffer,
UBaseType_t *uxAcquire,
UBaseType_t *uxItemsWaiting);

/**
* @brief Unblock any read function that is currently waiting. example: xRingbufferReceiveUpTo()
*
* All read functions take a xTicksToWait argument, which can be set up to
* to infinity. This function will unblock any threads currently waiting.
*
* @param[in] xRingbuffer The ring buffer who's rx will be unblocked.
*/
void vRingbufferUnblockRx(RingbufHandle_t xRingbuffer);

/**
* @brief Debugging function to print the internal pointers in the ring buffer
*
Expand Down
Loading

0 comments on commit 02eb8d6

Please sign in to comment.