[Console] fix console deinit: add ability to unblock linenoise, to fi…

…x deadlock

components/console/esp_console_repl.c

@@ -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 {
@@ -162,7 +164,7 @@ esp_err_t esp_console_new_repl_usb_serial_jtag(const esp_console_dev_usb_serial_
         goto _exit;
     }
 
-    /* Tell vfs to use usb-serial-jtag driver */
+    /* Tell vfs to use usb-serial-jtag driver*/
     esp_vfs_usb_serial_jtag_use_driver();
 
     // setup history
@@ -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(20 / portTICK_PERIOD_MS);
+        }
+    } 
+
     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);
@@ -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");
+        ESP_LOGE(TAG, "already de-initialized");s
         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:
@@ -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(20 / portTICK_PERIOD_MS);
+        }
+    } 
+
     repl_com->state = CONSOLE_REPL_STATE_DEINIT;
+
     esp_console_deinit();
     esp_vfs_usb_serial_jtag_use_nonblocking();
     usb_serial_jtag_driver_uninstall();
@@ -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);
 }

components/console/linenoise/linenoise.c

@@ -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;
@@ -248,10 +248,14 @@ 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;
@@ -270,7 +274,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;
 }
@@ -294,9 +299,8 @@ static int getColumns(void) {
 
     /* Get the initial position so we can restore it later. */
     start = getCursorPosition();
-    if (start == -1) {
-        goto failed;
-    }
+    if (start == -1 && errno == EWOULDBLOCK) {return -1;}
+    if (start == -2) {goto failed;}
 
     /* Send the command to go to right margin. Use `write` function instead of
      * `fwrite` for the same reasons explained in `getCursorPosition()` */
@@ -308,9 +312,8 @@ 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) {
-        goto failed;
-    }
+    if (cols == -1 && errno == EWOULDBLOCK) {return -1;}
+    if (cols == -2) {goto failed;}
 
     /* Restore the position of the cursor back. */
     if (cols > start) {
@@ -819,6 +822,9 @@ 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;
@@ -827,9 +833,9 @@ 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.maxrows = 0;
     l.history_index = 0;
+    l.cols = cols;
 
     /* Buffer starts empty. */
     l.buf[0] = '\0';
@@ -840,11 +846,16 @@ 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;
     }
@@ -864,7 +875,8 @@ 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.
@@ -940,19 +952,19 @@ 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) {
-                break;
-            }
+            int n = read(in_fd, seq, 2);
+            if (n == -1 && errno == EWOULDBLOCK) {return -1;}
+            if (n < 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) {
-                        break;
-                    }
+                    int n = read(in_fd, seq+2, 1);
+                    if (n == -1 && errno == EWOULDBLOCK) {return -1;}
+                    if (n == -1) {break;}
                     if (seq[2] == '~') {
                         switch(seq[1]) {
                         case '3': /* Delete key. */
@@ -996,6 +1008,7 @@ static int linenoiseEdit(char *buf, size_t buflen, const char *prompt)
                 }
             }
             break;
+        }
         default:
             if (linenoiseEditInsert(&l,c)) return -1;
             break;
@@ -1054,6 +1067,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;
         }
@@ -1084,8 +1099,10 @@ static int linenoiseRaw(char *buf, size_t buflen, const char *prompt) {
     }
 
     count = linenoiseEdit(buf, buflen, prompt);
-    fputc('\n', stdout);
-    flushWrite();
+    if (count != -1){
+        fputc('\n', stdout);
+        flushWrite();
+    }
     return count;
 }
 
@@ -1095,6 +1112,8 @@ 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){

components/driver/include/driver/uart.h

@@ -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.
   *

components/driver/include/driver/usb_serial_jtag.h

@@ -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
  *

components/driver/uart.c

@@ -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));

components/driver/usb_serial_jtag.c

@@ -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;

components/esp_ringbuf/include/freertos/ringbuf.h

@@ -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
  *