Skip to content

call HardwareSerial::end() before begin() will deinit Serial1 and may cause dead lock #2793

@marshfolx

Description

@marshfolx

Reason

HardwareSerial::end() calls uart_deinit(serial_t *obj), and inside this function:

void uart_deinit(serial_t *obj)
{
  /* Reset UART and disable clock */
  switch (obj->index) {
#if defined(USART1_BASE)
    case UART1_INDEX:
      __HAL_RCC_USART1_FORCE_RESET();
      __HAL_RCC_USART1_RELEASE_RESET();
      __HAL_RCC_USART1_CLK_DISABLE();
      break;
#endif
#if defined(USART2_BASE)
    case UART2_INDEX:
      __HAL_RCC_USART2_FORCE_RESET();
      __HAL_RCC_USART2_RELEASE_RESET();
      __HAL_RCC_USART2_CLK_DISABLE();
      break;
#endif

...

  HAL_UART_DeInit(uart_handlers[obj->index]);

...


}

It finds corresponding uart peripheral by obj->index, which is always 0 before begin() is called. And UART1_INDEX == 0, so it always deinit Serial1 if HardSerial::end() is called before begin().

Consequence

When Serial1 is deinit-ed, tx interrupt of USART1 will not be enabled, and any data in tx buffer will not be sent, causing mcu dead locked in flush() or availableForWrite().

This problem is quite confusing. I think it is a bit hard to debug when someone encounters it. Maybe it' s better to make sure that calling end() does not cause any side effects.

By the way

My program needs to modifiy baudrate randomly after setup, so I wrote:

    void config_baudrate() {
        u16 b = _reg[REG_INDEX_BAUD];
        u32 baud;

        if (b > sizeof(BAUD_LIST) / sizeof(BAUD_LIST[0])) {
            baud = 9600;
        }
        else {
            baud = BAUD_LIST[b];
        }

        Serial485.end();
        Serial485.begin(baud);
    }

Serial485 is alias to Serial3. Everything was ok before I started to use Serial1. Any better approach?

Reproduce

These code should be able to reproduce the problem:

#include <Arduino.h>


HardwareSerial Serial1{PA10, PA9};
HardwareSerial Serial3{PB11, PB10};


void setup() {
    Serial1.begin(9600);

    Serial3.end();
    Serial3.begin(9600);

    Serial1.print("I'm stuck");
    Serial1.flush();
}


void loop() {}

Solution

Just leave uart_handlers[0] as a placeholder, so as uart_handlers[0] == nullptr. This solution only cost little RAM.

Metadata

Metadata

Assignees

No one assigned

    Labels

    duplicateThis issue or pull request already exists

    Type

    No type
    No fields configured for issues without a type.

    Projects

    Status

    Done

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions