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 bc01e0c
Show file tree
Hide file tree
Showing 7 changed files with 112 additions and 1 deletion.
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);
}
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
26 changes: 25 additions & 1 deletion components/esp_ringbuf/ringbuf.c
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#define rbBYTE_BUFFER_FLAG ( ( UBaseType_t ) 2 ) //The ring buffer is a byte buffer
#define rbBUFFER_FULL_FLAG ( ( UBaseType_t ) 4 ) //The ring buffer is currently full (write pointer == free pointer)
#define rbBUFFER_STATIC_FLAG ( ( UBaseType_t ) 8 ) //The ring buffer is statically allocated
#define rbBUFFER_UNBLOCK_RX_FLAG ( ( UBaseType_t ) 16) //A request has been made to unblock any pending reads

//Item flags
#define rbITEM_FREE_FLAG ( ( UBaseType_t ) 1 ) //Item has been retrieved and returned by application, free to overwrite
Expand Down Expand Up @@ -759,6 +760,8 @@ static size_t prvGetCurMaxSizeByteBuf(Ringbuffer_t *pxRingbuffer)
return xFreeSize;
}



static BaseType_t prvReceiveGeneric(Ringbuffer_t *pxRingbuffer,
void **pvItem1,
void **pvItem2,
Expand All @@ -772,12 +775,19 @@ static BaseType_t prvReceiveGeneric(Ringbuffer_t *pxRingbuffer,
TickType_t xTicksEnd = xTaskGetTickCount() + xTicksToWait;
TickType_t xTicksRemaining = xTicksToWait;
while (xTicksRemaining <= xTicksToWait) { //xTicksToWait will underflow once xTaskGetTickCount() > ticks_end
//Block until more free space becomes available or timeout
//Block until some bytes become available or timeout
if (xSemaphoreTake(rbGET_RX_SEM_HANDLE(pxRingbuffer), xTicksRemaining) != pdTRUE) {
xReturn = pdFALSE; //Timed out attempting to get semaphore
break;
}

// has a request been made to unblock?
if (pxRingbuffer->uxRingbufferFlags & rbBUFFER_UNBLOCK_RX_FLAG) {
pxRingbuffer->uxRingbufferFlags &= ~rbBUFFER_UNBLOCK_RX_FLAG; // clear flag
xReturn = pdFALSE;
break;
}

//Semaphore obtained, check if item can be retrieved
portENTER_CRITICAL(&pxRingbuffer->mux);
if (prvCheckItemAvail(pxRingbuffer) == pdTRUE) {
Expand Down Expand Up @@ -1421,6 +1431,18 @@ void vRingbufferGetInfo(RingbufHandle_t xRingbuffer,
portEXIT_CRITICAL(&pxRingbuffer->mux);
}

void vRingbufferUnblockRx(RingbufHandle_t xRingbuffer)
{
Ringbuffer_t *pxRingbuffer = (Ringbuffer_t *)xRingbuffer;
configASSERT(pxRingbuffer);

// is the semaphore taken?
if (uxSemaphoreGetCount(rbGET_RX_SEM_HANDLE(pxRingbuffer)) == 0) {
pxRingbuffer->uxRingbufferFlags |= rbBUFFER_UNBLOCK_RX_FLAG;
xSemaphoreGive(rbGET_RX_SEM_HANDLE(pxRingbuffer));
}
}

void xRingbufferPrintInfo(RingbufHandle_t xRingbuffer)
{
Ringbuffer_t *pxRingbuffer = (Ringbuffer_t *)xRingbuffer;
Expand All @@ -1432,3 +1454,5 @@ void xRingbufferPrintInfo(RingbufHandle_t xRingbuffer)
pxRingbuffer->pucWrite - pxRingbuffer->pucHead,
pxRingbuffer->pucAcquire - pxRingbuffer->pucHead);
}


0 comments on commit bc01e0c

Please sign in to comment.