From 6154b673c2b6d25d05ec899731d3f4abd171f033 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 17 Mar 2026 12:18:31 -1000 Subject: [PATCH] [usb_uart] Fix EventPool/LockFreeQueue sizing off-by-one (#14895) --- esphome/components/usb_uart/usb_uart.cpp | 11 +++++------ esphome/components/usb_uart/usb_uart.h | 8 ++++++-- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/esphome/components/usb_uart/usb_uart.cpp b/esphome/components/usb_uart/usb_uart.cpp index 3d35f368fb..7c4358fdbd 100644 --- a/esphome/components/usb_uart/usb_uart.cpp +++ b/esphome/components/usb_uart/usb_uart.cpp @@ -160,11 +160,9 @@ void USBUartChannel::write_array(const uint8_t *data, size_t len) { size_t chunk_len = std::min(len, UsbOutputChunk::MAX_CHUNK_SIZE); memcpy(chunk->data, data, chunk_len); chunk->length = static_cast(chunk_len); - if (!this->output_queue_.push(chunk)) { - this->output_pool_.release(chunk); - ESP_LOGE(TAG, "Output queue full - lost %zu bytes", len); - break; - } + // Push always succeeds: pool is sized to queue capacity (SIZE-1), so if + // allocate() returned non-null, the queue cannot be full. + this->output_queue_.push(chunk); data += chunk_len; len -= chunk_len; } @@ -320,7 +318,8 @@ void USBUartComponent::start_input(USBUartChannel *channel) { chunk->channel = channel; // Push to lock-free queue for main loop processing - // Push always succeeds because pool size == queue size + // Push always succeeds: pool is sized to queue capacity (SIZE-1), so if + // allocate() returned non-null, the queue cannot be full. this->usb_data_queue_.push(chunk); // Re-enable component loop to process the queued data diff --git a/esphome/components/usb_uart/usb_uart.h b/esphome/components/usb_uart/usb_uart.h index 16469df7f6..7a06b04f11 100644 --- a/esphome/components/usb_uart/usb_uart.h +++ b/esphome/components/usb_uart/usb_uart.h @@ -158,7 +158,10 @@ class USBUartChannel : public uart::UARTComponent, public Parented output_queue_; - EventPool output_pool_; + // Pool sized to queue capacity (SIZE-1) because LockFreeQueue is a ring + // buffer that holds N-1 elements. This guarantees allocate() returns nullptr + // before push() can fail, preventing a pool slot leak. + EventPool output_pool_; std::function rx_callback_{}; CdcEps cdc_dev_{}; StringRef debug_prefix_{}; @@ -190,7 +193,8 @@ class USBUartComponent : public usb_host::USBClient { // Lock-free data transfer from USB task to main loop static constexpr int USB_DATA_QUEUE_SIZE = 32; LockFreeQueue usb_data_queue_; - EventPool chunk_pool_; + // Pool sized to queue capacity (SIZE-1) — see USBUartChannel::output_pool_ comment. + EventPool chunk_pool_; protected: std::vector channels_{};