diff --git a/components/esp_modem/src/esp_modem_uart.cpp b/components/esp_modem/src/esp_modem_uart.cpp index ae1efe15e9..256669d889 100644 --- a/components/esp_modem/src/esp_modem_uart.cpp +++ b/components/esp_modem/src/esp_modem_uart.cpp @@ -13,6 +13,7 @@ #include "esp_modem_config.h" #include "exception_stub.hpp" #include "cxx_include/esp_modem_dte.hpp" +#include "cxx_include/esp_modem_primitives.hpp" #include "uart_resource.hpp" static const char *TAG = "uart_terminal"; @@ -64,6 +65,7 @@ class UartTerminal : public Terminal { void set_read_cb(std::function f) override { + Scoped lock(on_read_lock); on_read = std::move(f); } @@ -95,6 +97,10 @@ class UartTerminal : public Terminal { QueueHandle_t event_queue; uart_resource uart; SignalGroup signal; + /// Mutex to protect on_read callback from being updated while being called in \c task. + /// Must be recursive as the on_read callback may call set_read_cb to change the callback while being called. + /// Declared before task_handle so it outlives the UART task. + Lock on_read_lock; uart_task task_handle; }; @@ -121,8 +127,13 @@ void UartTerminal::task() switch (event.type) { case UART_DATA: uart_get_buffered_data_len(uart.port, &len); - if (len && on_read) { - on_read(nullptr, len); + { + // Lock on_read_lock while checking and calling on_read to + // prevent data-race between check and call + Scoped lock(on_read_lock); + if (len && on_read) { + on_read(nullptr, len); + } } break; case UART_FIFO_OVF: @@ -163,6 +174,8 @@ void UartTerminal::task() } } else { uart_get_buffered_data_len(uart.port, &len); + + Scoped lock(on_read_lock); if (len && on_read) { on_read(nullptr, len); }