From 8bbfadb59aa39b02156653dee670f6c2990cd506 Mon Sep 17 00:00:00 2001 From: Diorcet Yann Date: Tue, 17 Mar 2026 14:22:31 +0100 Subject: [PATCH 01/23] [core] Small improvements (#14884) --- esphome/components/bme68x_bsec2/__init__.py | 4 ++-- script/merge_component_configs.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/esphome/components/bme68x_bsec2/__init__.py b/esphome/components/bme68x_bsec2/__init__.py index 4200b2f0b8..5f0afa9c9f 100644 --- a/esphome/components/bme68x_bsec2/__init__.py +++ b/esphome/components/bme68x_bsec2/__init__.py @@ -186,8 +186,8 @@ async def to_code_base(config): cg.add_library("SPI", None) cg.add_library( "BME68x Sensor library", - "1.3.40408", - "https://github.com/boschsensortec/Bosch-BME68x-Library", + None, + "https://github.com/boschsensortec/Bosch-BME68x-Library#v1.3.40408", ) cg.add_library( "BSEC2 Software Library", diff --git a/script/merge_component_configs.py b/script/merge_component_configs.py index 5e98f1fef5..41bbafcd02 100755 --- a/script/merge_component_configs.py +++ b/script/merge_component_configs.py @@ -384,7 +384,7 @@ def merge_component_configs( # Write merged config output_file.parent.mkdir(parents=True, exist_ok=True) yaml_content = yaml_util.dump(merged_config_data) - output_file.write_text(yaml_content) + output_file.write_text(yaml_content, encoding="utf-8") print(f"Successfully merged {len(component_names)} components into {output_file}") From 37f9541f322d3ccee1c232df8b2a9a503165d41a Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 17 Mar 2026 08:14:36 -1000 Subject: [PATCH 02/23] [api] Fix ProtoMessage protected destructor compile error on host platform (#14882) --- esphome/components/api/proto.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/esphome/components/api/proto.h b/esphome/components/api/proto.h index d1c955b1fb..44d8f04585 100644 --- a/esphome/components/api/proto.h +++ b/esphome/components/api/proto.h @@ -442,8 +442,12 @@ class ProtoMessage { virtual const char *message_name() const { return "unknown"; } #endif +#ifndef USE_HOST protected: +#endif // Non-virtual destructor is protected to prevent polymorphic deletion. + // On host platform, made public to allow value-initialization of std::array + // members (e.g. DeviceInfoResponse::devices) without clang errors. ~ProtoMessage() = default; }; From c5d42b05696f0238929447fcbb3db6ba1018ae82 Mon Sep 17 00:00:00 2001 From: Jonathan Swoboda <154711427+swoboda1337@users.noreply.github.com> Date: Tue, 17 Mar 2026 15:18:49 -0400 Subject: [PATCH 03/23] [speaker] Fix media playlist using announcement delay (#14889) --- .../components/speaker/media_player/speaker_media_player.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/esphome/components/speaker/media_player/speaker_media_player.cpp b/esphome/components/speaker/media_player/speaker_media_player.cpp index 9f168f854d..930373c6fc 100644 --- a/esphome/components/speaker/media_player/speaker_media_player.cpp +++ b/esphome/components/speaker/media_player/speaker_media_player.cpp @@ -417,7 +417,7 @@ void SpeakerMediaPlayer::loop() { this->media_playlist_.pop_front(); } // Only delay starting playback if moving on the next playlist item or repeating the current item - timeout_ms = this->announcement_playlist_delay_ms_; + timeout_ms = this->media_playlist_delay_ms_; } if (!this->media_playlist_.empty()) { PlaylistItem playlist_item = this->media_playlist_.front(); From 4122fa5dddaea6b354592dd41e20ad6179223b76 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 17 Mar 2026 09:58:05 -1000 Subject: [PATCH 04/23] [core] Add back deprecated set_internal() for external projects (#14887) --- esphome/core/entity_base.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/esphome/core/entity_base.h b/esphome/core/entity_base.h index cccbafd2c3..723bf54584 100644 --- a/esphome/core/entity_base.h +++ b/esphome/core/entity_base.h @@ -100,6 +100,14 @@ class EntityBase { // Get whether this Entity should be hidden outside ESPHome bool is_internal() const { return this->flags_.internal; } + // Deprecated: Calling set_internal() at runtime is undefined behavior. Components and clients + // are NOT notified of the change, the flag may have already been read during setup, and there + // is NO guarantee any consumer will observe the new value. Use the 'internal:' YAML key instead. + ESPDEPRECATED("set_internal() is undefined behavior at runtime — components and Home Assistant are NOT " + "notified. Use the 'internal:' YAML key instead. Will be removed in 2027.3.0.", + "2026.3.0") + void set_internal(bool internal) { this->flags_.internal = internal; } + // Check if this object is declared to be disabled by default. // That means that when the device gets added to Home Assistant (or other clients) it should // not be added to the default view by default, and a user action is necessary to manually add it. From 1b70df2c1f8a9a7edbc3b389fe6095ddd7e9f2b3 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 17 Mar 2026 12:16:44 -1000 Subject: [PATCH 05/23] [espnow] Fix EventPool/LockFreeQueue sizing off-by-one (#14893) --- esphome/components/espnow/espnow_component.cpp | 6 ++++-- esphome/components/espnow/espnow_component.h | 8 ++++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/esphome/components/espnow/espnow_component.cpp b/esphome/components/espnow/espnow_component.cpp index 991803d870..78916891f4 100644 --- a/esphome/components/espnow/espnow_component.cpp +++ b/esphome/components/espnow/espnow_component.cpp @@ -87,7 +87,8 @@ void on_send_report(const uint8_t *mac_addr, esp_now_send_status_t status) // Push the packet to the queue global_esp_now->receive_packet_queue_.push(packet); - // Push always because we're the only producer and the pool ensures we never exceed queue size + // Push always succeeds: pool is sized to queue capacity (SIZE-1), so if + // allocate() returned non-null, the queue cannot be full. // Wake main loop immediately to process ESP-NOW send event instead of waiting for select() timeout #if defined(USE_SOCKET_SELECT_SUPPORT) && defined(USE_WAKE_LOOP_THREADSAFE) @@ -109,7 +110,8 @@ void on_data_received(const esp_now_recv_info_t *info, const uint8_t *data, int // Push the packet to the queue global_esp_now->receive_packet_queue_.push(packet); - // Push always because we're the only producer and the pool ensures we never exceed queue size + // Push always succeeds: pool is sized to queue capacity (SIZE-1), so if + // allocate() returned non-null, the queue cannot be full. // Wake main loop immediately to process ESP-NOW receive event instead of waiting for select() timeout #if defined(USE_SOCKET_SELECT_SUPPORT) && defined(USE_WAKE_LOOP_THREADSAFE) diff --git a/esphome/components/espnow/espnow_component.h b/esphome/components/espnow/espnow_component.h index 9941e97227..ee4adc1b4d 100644 --- a/esphome/components/espnow/espnow_component.h +++ b/esphome/components/espnow/espnow_component.h @@ -163,10 +163,14 @@ class ESPNowComponent : public Component { uint8_t own_address_[ESP_NOW_ETH_ALEN]{0}; LockFreeQueue receive_packet_queue_{}; - EventPool receive_packet_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 receive_packet_pool_{}; LockFreeQueue send_packet_queue_{}; - EventPool send_packet_pool_{}; + // Pool sized to queue capacity (SIZE-1) — see receive_packet_pool_ comment. + EventPool send_packet_pool_{}; ESPNowSendPacket *current_send_packet_{nullptr}; // Currently sending packet, nullptr if none uint8_t wifi_channel_{0}; From 8caa11dcf45aed498be5ec8ef83611be86b9a547 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 17 Mar 2026 12:17:43 -1000 Subject: [PATCH 06/23] [usb_cdc_acm] Fix EventPool/LockFreeQueue sizing off-by-one (#14894) --- .../components/usb_cdc_acm/usb_cdc_acm.cpp | 26 +++++++------------ esphome/components/usb_cdc_acm/usb_cdc_acm.h | 6 ++++- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/esphome/components/usb_cdc_acm/usb_cdc_acm.cpp b/esphome/components/usb_cdc_acm/usb_cdc_acm.cpp index a4c2e6c4a4..253626f0a3 100644 --- a/esphome/components/usb_cdc_acm/usb_cdc_acm.cpp +++ b/esphome/components/usb_cdc_acm/usb_cdc_acm.cpp @@ -26,16 +26,13 @@ void USBCDCACMInstance::queue_line_state_event(bool dtr, bool rts) { event->data.line_state.dtr = dtr; event->data.line_state.rts = rts; - if (!this->event_queue_.push(event)) { - ESP_LOGW(TAG, "Event queue full, line state event dropped (itf=%d)", this->itf_); - // Return event to pool since we couldn't queue it - this->event_pool_.release(event); - } else { - // Wake main loop immediately to process event + // Push always succeeds: pool is sized to queue capacity (SIZE-1), so if + // allocate() returned non-null, the queue cannot be full. + this->event_queue_.push(event); + #if defined(USE_SOCKET_SELECT_SUPPORT) && defined(USE_WAKE_LOOP_THREADSAFE) - App.wake_loop_threadsafe(); + App.wake_loop_threadsafe(); #endif - } } void USBCDCACMInstance::queue_line_coding_event(uint32_t bit_rate, uint8_t stop_bits, uint8_t parity, @@ -53,16 +50,13 @@ void USBCDCACMInstance::queue_line_coding_event(uint32_t bit_rate, uint8_t stop_ event->data.line_coding.parity = parity; event->data.line_coding.data_bits = data_bits; - if (!this->event_queue_.push(event)) { - ESP_LOGW(TAG, "Event queue full, line coding event dropped (itf=%d)", this->itf_); - // Return event to pool since we couldn't queue it - this->event_pool_.release(event); - } else { - // Wake main loop immediately to process event + // Push always succeeds: pool is sized to queue capacity (SIZE-1), so if + // allocate() returned non-null, the queue cannot be full. + this->event_queue_.push(event); + #if defined(USE_SOCKET_SELECT_SUPPORT) && defined(USE_WAKE_LOOP_THREADSAFE) - App.wake_loop_threadsafe(); + App.wake_loop_threadsafe(); #endif - } } void USBCDCACMInstance::process_events_() { diff --git a/esphome/components/usb_cdc_acm/usb_cdc_acm.h b/esphome/components/usb_cdc_acm/usb_cdc_acm.h index 624f41cf8c..90c673a89e 100644 --- a/esphome/components/usb_cdc_acm/usb_cdc_acm.h +++ b/esphome/components/usb_cdc_acm/usb_cdc_acm.h @@ -102,7 +102,11 @@ class USBCDCACMInstance : public uart::UARTComponent, public Parented event_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 both a pool slot leak and an SPSC + // violation on the pool's internal free list. + EventPool event_pool_; LockFreeQueue event_queue_; }; From 3bde7ec978f2d691586700ab107cff59a0f014c9 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 17 Mar 2026 12:17:59 -1000 Subject: [PATCH 07/23] [usb_host] Fix EventPool/LockFreeQueue sizing off-by-one (#14896) --- esphome/components/usb_host/usb_host.h | 5 ++++- esphome/components/usb_host/usb_host_client.cpp | 3 ++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/esphome/components/usb_host/usb_host.h b/esphome/components/usb_host/usb_host.h index 2eec0c9699..dcb76a3a3b 100644 --- a/esphome/components/usb_host/usb_host.h +++ b/esphome/components/usb_host/usb_host.h @@ -144,7 +144,10 @@ class USBClient : public Component { // Lock-free event queue and pool for USB task to main loop communication // Must be public for access from static callbacks LockFreeQueue event_queue; - EventPool event_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 event_pool; protected: // Process USB events from the queue. Returns true if any work was done. diff --git a/esphome/components/usb_host/usb_host_client.cpp b/esphome/components/usb_host/usb_host_client.cpp index 2a460d1a07..18d938344c 100644 --- a/esphome/components/usb_host/usb_host_client.cpp +++ b/esphome/components/usb_host/usb_host_client.cpp @@ -193,7 +193,8 @@ static void client_event_cb(const usb_host_client_event_msg_t *event_msg, void * return; } - // Push to lock-free queue (always succeeds since 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. client->event_queue.push(event); // Re-enable component loop to process the queued event From 6154b673c2b6d25d05ec899731d3f4abd171f033 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 17 Mar 2026 12:18:31 -1000 Subject: [PATCH 08/23] [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_{}; From ccf672d7ee37389dcd85f46f49ca0a5b5471f3d0 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 17 Mar 2026 12:24:02 -1000 Subject: [PATCH 09/23] [esp32_ble] Fix EventPool/LockFreeQueue sizing off-by-one (#14892) --- esphome/components/esp32_ble/ble.cpp | 3 ++- esphome/components/esp32_ble/ble.h | 8 +++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/esphome/components/esp32_ble/ble.cpp b/esphome/components/esp32_ble/ble.cpp index ff9d9bb15a..fee1c546be 100644 --- a/esphome/components/esp32_ble/ble.cpp +++ b/esphome/components/esp32_ble/ble.cpp @@ -575,8 +575,9 @@ template void enqueue_ble_event(Args... args) { load_ble_event(event, args...); // Push the event to the queue + // Push always succeeds: pool is sized to queue capacity (N-1), so if + // allocate() returned non-null, the queue is guaranteed to have room. global_ble->ble_events_.push(event); - // Push always succeeds because we're the only producer and the pool ensures we never exceed queue size } // Explicit template instantiations for the friend function diff --git a/esphome/components/esp32_ble/ble.h b/esphome/components/esp32_ble/ble.h index 04bec3f785..752ddc9d1f 100644 --- a/esphome/components/esp32_ble/ble.h +++ b/esphome/components/esp32_ble/ble.h @@ -221,7 +221,13 @@ class ESP32BLE : public Component { // Large objects (size depends on template parameters, but typically aligned to 4 bytes) esphome::LockFreeQueue ble_events_; - esphome::EventPool ble_event_pool_; + // Pool sized to queue capacity (SIZE-1) because LockFreeQueue is a ring + // buffer that holds N-1 elements (one slot distinguishes full from empty). + // This guarantees allocate() returns nullptr before push() can fail, which: + // 1. Prevents leaking a pool slot (the Nth allocate succeeds but push fails) + // 2. Avoids needing release() on the producer path after a failed push(), + // preserving the SPSC contract on the pool's internal free list + esphome::EventPool ble_event_pool_; // 4-byte aligned members #ifdef USE_ESP32_BLE_ADVERTISING From 80bd6489cf12c6eadafe02604facb7f841cc5600 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 17 Mar 2026 13:38:41 -1000 Subject: [PATCH 10/23] [esp32_ble_server] Remove vestigial semaphore from BLECharacteristic (#14900) --- .../components/esp32_ble_server/ble_characteristic.cpp | 10 +--------- .../components/esp32_ble_server/ble_characteristic.h | 4 ---- 2 files changed, 1 insertion(+), 13 deletions(-) diff --git a/esphome/components/esp32_ble_server/ble_characteristic.cpp b/esphome/components/esp32_ble_server/ble_characteristic.cpp index 1806354712..aa82b773ba 100644 --- a/esphome/components/esp32_ble_server/ble_characteristic.cpp +++ b/esphome/components/esp32_ble_server/ble_characteristic.cpp @@ -16,13 +16,9 @@ BLECharacteristic::~BLECharacteristic() { for (auto *descriptor : this->descriptors_) { delete descriptor; // NOLINT(cppcoreguidelines-owning-memory) } - vSemaphoreDelete(this->set_value_lock_); } BLECharacteristic::BLECharacteristic(const ESPBTUUID uuid, uint32_t properties) : uuid_(uuid) { - this->set_value_lock_ = xSemaphoreCreateBinary(); - xSemaphoreGive(this->set_value_lock_); - this->properties_ = (esp_gatt_char_prop_t) 0; this->set_broadcast_property((properties & PROPERTY_BROADCAST) != 0); @@ -35,11 +31,7 @@ BLECharacteristic::BLECharacteristic(const ESPBTUUID uuid, uint32_t properties) void BLECharacteristic::set_value(ByteBuffer buffer) { this->set_value(buffer.get_data()); } -void BLECharacteristic::set_value(std::vector &&buffer) { - xSemaphoreTake(this->set_value_lock_, 0L); - this->value_ = std::move(buffer); - xSemaphoreGive(this->set_value_lock_); -} +void BLECharacteristic::set_value(std::vector &&buffer) { this->value_ = std::move(buffer); } void BLECharacteristic::set_value(std::initializer_list data) { this->set_value(std::vector(data)); // Delegate to move overload diff --git a/esphome/components/esp32_ble_server/ble_characteristic.h b/esphome/components/esp32_ble_server/ble_characteristic.h index 72897d1dfb..062052cdf8 100644 --- a/esphome/components/esp32_ble_server/ble_characteristic.h +++ b/esphome/components/esp32_ble_server/ble_characteristic.h @@ -16,8 +16,6 @@ #include #include #include -#include -#include namespace esphome { namespace esp32_ble_server { @@ -84,8 +82,6 @@ class BLECharacteristic { uint16_t value_read_offset_{0}; std::vector value_; - SemaphoreHandle_t set_value_lock_; - std::vector descriptors_; struct ClientNotificationEntry { From be2e4a5278a83155fa3128e68c4b9824c311f1cf Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 17 Mar 2026 13:49:24 -1000 Subject: [PATCH 11/23] [mqtt] Fix data race on inbound event queue (#14891) Co-authored-by: Jesse Hills <3060199+jesserockz@users.noreply.github.com> --- .../components/mqtt/mqtt_backend_esp32.cpp | 34 ++++++--- esphome/components/mqtt/mqtt_backend_esp32.h | 69 +++++++++++-------- 2 files changed, 66 insertions(+), 37 deletions(-) diff --git a/esphome/components/mqtt/mqtt_backend_esp32.cpp b/esphome/components/mqtt/mqtt_backend_esp32.cpp index 5642fd5f7b..ab067c4418 100644 --- a/esphome/components/mqtt/mqtt_backend_esp32.cpp +++ b/esphome/components/mqtt/mqtt_backend_esp32.cpp @@ -82,10 +82,16 @@ bool MQTTBackendESP32::initialize_() { void MQTTBackendESP32::loop() { // process new events // handle only 1 message per loop iteration - if (!mqtt_events_.empty()) { - auto &event = mqtt_events_.front(); - mqtt_event_handler_(event); - mqtt_events_.pop(); + Event *event = this->mqtt_event_queue_.pop(); + if (event != nullptr) { + this->mqtt_event_handler_(*event); + this->mqtt_event_pool_.release(event); + } + + // Log dropped inbound events (check is cheap - single atomic load in common case) + uint16_t inbound_dropped = this->mqtt_event_queue_.get_and_reset_dropped_count(); + if (inbound_dropped > 0) { + ESP_LOGW(TAG, "Dropped %u inbound MQTT events", inbound_dropped); } #if defined(USE_MQTT_IDF_ENQUEUE) @@ -183,10 +189,18 @@ void MQTTBackendESP32::mqtt_event_handler_(const Event &event) { void MQTTBackendESP32::mqtt_event_handler(void *handler_args, esp_event_base_t base, int32_t event_id, void *event_data) { MQTTBackendESP32 *instance = static_cast(handler_args); - // queue event to decouple processing + // queue event to decouple processing from ESP-IDF MQTT task to main loop if (instance) { - auto event = *static_cast(event_data); - instance->mqtt_events_.emplace(event); + auto *event = instance->mqtt_event_pool_.allocate(); + if (event == nullptr) { + // Pool exhausted, drop event (counted via queue's dropped counter) + instance->mqtt_event_queue_.increment_dropped_count(); + return; + } + event->populate(*static_cast(event_data)); + // Push always succeeds: pool is sized to queue capacity (SIZE-1), so if + // allocate() returned non-null, the queue cannot be full. + instance->mqtt_event_queue_.push(event); // Wake main loop immediately to process MQTT event instead of waiting for select() timeout #if defined(USE_SOCKET_SELECT_SUPPORT) && defined(USE_WAKE_LOOP_THREADSAFE) @@ -226,14 +240,14 @@ void MQTTBackendESP32::esphome_mqtt_task(void *params) { break; } } - this_mqtt->mqtt_event_pool_.release(elem); + this_mqtt->mqtt_outbound_pool_.release(elem); } } } bool MQTTBackendESP32::enqueue_(MqttQueueTypeT type, const char *topic, int qos, bool retain, const char *payload, size_t len) { - auto *elem = this->mqtt_event_pool_.allocate(); + auto *elem = this->mqtt_outbound_pool_.allocate(); if (!elem) { // Queue is full - increment counter but don't log immediately. @@ -253,7 +267,7 @@ bool MQTTBackendESP32::enqueue_(MqttQueueTypeT type, const char *topic, int qos, // Use the helper to allocate and copy data if (!elem->set_data(topic, payload, len)) { // Allocation failed, return elem to pool - this->mqtt_event_pool_.release(elem); + this->mqtt_outbound_pool_.release(elem); // Increment counter without logging to avoid cascade effect during memory pressure this->mqtt_queue_.increment_dropped_count(); return false; diff --git a/esphome/components/mqtt/mqtt_backend_esp32.h b/esphome/components/mqtt/mqtt_backend_esp32.h index 5c4dc413bd..58d1b29b32 100644 --- a/esphome/components/mqtt/mqtt_backend_esp32.h +++ b/esphome/components/mqtt/mqtt_backend_esp32.h @@ -5,7 +5,6 @@ #ifdef USE_ESP32 #include -#include #include #include #include @@ -18,32 +17,39 @@ namespace esphome::mqtt { struct Event { - esp_mqtt_event_id_t event_id; + esp_mqtt_event_id_t event_id{}; std::vector data; - int total_data_len; - int current_data_offset; + int total_data_len{0}; + int current_data_offset{0}; std::string topic; - int msg_id; - bool retain; - int qos; - bool dup; - bool session_present; - esp_mqtt_error_codes_t error_handle; + int msg_id{0}; + bool retain{false}; + int qos{0}; + bool dup{false}; + bool session_present{false}; + esp_mqtt_error_codes_t error_handle{}; - // Construct from esp_mqtt_event_t - // Any pointer values that are unsafe to keep are converted to safe copies - Event(const esp_mqtt_event_t &event) - : event_id(event.event_id), - data(event.data, event.data + event.data_len), - total_data_len(event.total_data_len), - current_data_offset(event.current_data_offset), - topic(event.topic, event.topic_len), - msg_id(event.msg_id), - retain(event.retain), - qos(event.qos), - dup(event.dup), - session_present(event.session_present), - error_handle(*event.error_handle) {} + // Populate from esp_mqtt_event_t + // Copies pointer-based data to owned storage for safe cross-thread transfer + void populate(const esp_mqtt_event_t &event) { + this->event_id = event.event_id; + this->data.assign(event.data, event.data + event.data_len); + this->total_data_len = event.total_data_len; + this->current_data_offset = event.current_data_offset; + this->topic.assign(event.topic, event.topic_len); + this->msg_id = event.msg_id; + this->retain = event.retain; + this->qos = event.qos; + this->dup = event.dup; + this->session_present = event.session_present; + this->error_handle = *event.error_handle; + } + + // Release owned resources for pool reuse (keeps allocated capacity for efficiency) + void release() { + this->data.clear(); + this->topic.clear(); + } }; enum MqttQueueTypeT : uint8_t { @@ -118,7 +124,8 @@ class MQTTBackendESP32 final : public MQTTBackend { static constexpr size_t TASK_STACK_SIZE = 3072; static constexpr size_t TASK_STACK_SIZE_TLS = 4096; // Larger stack for TLS operations static constexpr ssize_t TASK_PRIORITY = 5; - static constexpr uint8_t MQTT_QUEUE_LENGTH = 30; // 30*12 bytes = 360 + static constexpr uint8_t MQTT_QUEUE_LENGTH = 30; // 30*12 bytes = 360 + static constexpr uint8_t MQTT_EVENT_QUEUE_LENGTH = 32; // Inbound events from broker void set_keep_alive(uint16_t keep_alive) final { this->keep_alive_ = keep_alive; } void set_client_id(const char *client_id) final { this->client_id_ = client_id; } @@ -251,7 +258,8 @@ class MQTTBackendESP32 final : public MQTTBackend { bool skip_cert_cn_check_{false}; #if defined(USE_MQTT_IDF_ENQUEUE) static void esphome_mqtt_task(void *params); - EventPool mqtt_event_pool_; + // Pool sized to queue capacity (SIZE-1) — see mqtt_event_pool_ comment. + EventPool mqtt_outbound_pool_; NotifyingLockFreeQueue mqtt_queue_; TaskHandle_t task_handle_{nullptr}; bool enqueue_(MqttQueueTypeT type, const char *topic, int qos = 0, bool retain = false, const char *payload = NULL, @@ -266,7 +274,14 @@ class MQTTBackendESP32 final : public MQTTBackend { CallbackManager on_message_; CallbackManager on_publish_; std::string cached_topic_; - std::queue mqtt_events_; + // Pool sized to queue capacity (SIZE-1) because LockFreeQueue is a ring + // buffer that holds N-1 elements (one slot distinguishes full from empty). + // This guarantees allocate() returns nullptr before push() can fail, which: + // 1. Prevents leaking a pool slot (the Nth allocate succeeds but push fails) + // 2. Avoids needing release() on the producer path after a failed push(), + // preserving the SPSC contract on the pool's internal free list + EventPool mqtt_event_pool_; + LockFreeQueue mqtt_event_queue_; #if defined(USE_MQTT_IDF_ENQUEUE) uint32_t last_dropped_log_time_{0}; From 0fa96b6e1ea76b46a19d8cc4f847dcea3606452a Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 17 Mar 2026 14:19:31 -1000 Subject: [PATCH 12/23] [scheduler] Fix UB in cross-thread counter/vector reads, add atomic fast-path (#14880) Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com> --- esphome/core/scheduler.cpp | 48 ++++++----- esphome/core/scheduler.h | 159 ++++++++++++++++++++++++++++++++++--- 2 files changed, 177 insertions(+), 30 deletions(-) diff --git a/esphome/core/scheduler.cpp b/esphome/core/scheduler.cpp index 63e1006b03..8c4ff0ddb5 100644 --- a/esphome/core/scheduler.cpp +++ b/esphome/core/scheduler.cpp @@ -211,6 +211,14 @@ void HOT Scheduler::set_timer_common_(Component *component, SchedulerItem::Type this->cancel_item_locked_(component, name_type, static_name, hash_or_id, type); } target->push_back(item); + if (target == &this->to_add_) { + this->to_add_count_increment_(); + } +#ifndef ESPHOME_THREAD_SINGLE + else { + this->defer_count_increment_(); + } +#endif } void HOT Scheduler::set_timeout(Component *component, const char *name, uint32_t timeout, @@ -387,7 +395,7 @@ optional HOT Scheduler::next_schedule_in(uint32_t now) { // safe when called from the main thread. Other threads must not call this method. // If no items, return empty optional - if (this->cleanup_() == 0) + if (!this->cleanup_()) return {}; SchedulerItem *item = this->items_[0]; @@ -421,7 +429,7 @@ void Scheduler::full_cleanup_removed_items_() { this->items_.erase(this->items_.begin() + write, this->items_.end()); // Rebuild the heap structure since items are no longer in heap order std::make_heap(this->items_.begin(), this->items_.end(), SchedulerItem::cmp); - this->to_remove_ = 0; + this->to_remove_clear_(); } #ifndef ESPHOME_THREAD_SINGLE @@ -502,7 +510,7 @@ void HOT Scheduler::call(uint32_t now) { // If we still have too many cancelled items, do a full cleanup // This only happens if cancelled items are stuck in the middle/bottom of the heap - if (this->to_remove_ >= MAX_LOGICALLY_DELETED_ITEMS) { + if (this->to_remove_count_() >= MAX_LOGICALLY_DELETED_ITEMS) { this->full_cleanup_removed_items_(); } while (!this->items_.empty()) { @@ -529,7 +537,7 @@ void HOT Scheduler::call(uint32_t now) { LockGuard guard{this->lock_}; if (is_item_removed_locked_(item)) { this->recycle_item_main_loop_(this->pop_raw_locked_()); - this->to_remove_--; + this->to_remove_decrement_(); continue; } } @@ -538,7 +546,7 @@ void HOT Scheduler::call(uint32_t now) { if (is_item_removed_(item)) { LockGuard guard{this->lock_}; this->recycle_item_main_loop_(this->pop_raw_locked_()); - this->to_remove_--; + this->to_remove_decrement_(); continue; } #endif @@ -566,7 +574,7 @@ void HOT Scheduler::call(uint32_t now) { if (this->is_item_removed_locked_(executed_item)) { // We were removed/cancelled in the function call, recycle and continue - this->to_remove_--; + this->to_remove_decrement_(); this->recycle_item_main_loop_(executed_item); continue; } @@ -576,6 +584,7 @@ void HOT Scheduler::call(uint32_t now) { // Add new item directly to to_add_ // since we have the lock held this->to_add_.push_back(executed_item); + this->to_add_count_increment_(); } else { // Timeout completed - recycle it this->recycle_item_main_loop_(executed_item); @@ -604,6 +613,10 @@ void HOT Scheduler::call(uint32_t now) { #endif } void HOT Scheduler::process_to_add() { + // Fast path: skip lock acquisition when nothing to add. + // Worst case is a one-loop-iteration delay before newly added items are processed. + if (this->to_add_empty_()) + return; LockGuard guard{this->lock_}; for (auto *&it : this->to_add_) { if (is_item_removed_locked_(it)) { @@ -617,17 +630,14 @@ void HOT Scheduler::process_to_add() { std::push_heap(this->items_.begin(), this->items_.end(), SchedulerItem::cmp); } this->to_add_.clear(); + this->to_add_count_clear_(); } -size_t HOT Scheduler::cleanup_() { - // Fast path: if nothing to remove, just return the current size - // Reading to_remove_ without lock is safe because: - // 1. We only call this from the main thread during call() - // 2. If it's 0, there's definitely nothing to cleanup - // 3. If it becomes non-zero after we check, cleanup will happen on the next loop iteration - // 4. Not all platforms support atomics, so we accept this race in favor of performance - // 5. The worst case is a one-loop-iteration delay in cleanup, which is harmless - if (this->to_remove_ == 0) - return this->items_.size(); +bool HOT Scheduler::cleanup_() { + // Fast path: if nothing to remove, just check if items exist. + // Uses atomic load on platforms with atomics, falls back to always taking the lock otherwise. + // Worst case is a one-loop-iteration delay in cleanup. + if (this->to_remove_empty_()) + return !this->items_.empty(); // We must hold the lock for the entire cleanup operation because: // 1. We're modifying items_ (via pop_raw_locked_) which requires exclusive access @@ -642,10 +652,10 @@ size_t HOT Scheduler::cleanup_() { SchedulerItem *item = this->items_[0]; if (!this->is_item_removed_locked_(item)) break; - this->to_remove_--; + this->to_remove_decrement_(); this->recycle_item_main_loop_(this->pop_raw_locked_()); } - return this->items_.size(); + return !this->items_.empty(); } Scheduler::SchedulerItem *HOT Scheduler::pop_raw_locked_() { std::pop_heap(this->items_.begin(), this->items_.end(), SchedulerItem::cmp); @@ -698,7 +708,7 @@ bool HOT Scheduler::cancel_item_locked_(Component *component, NameType name_type size_t heap_cancelled = this->mark_matching_items_removed_locked_(this->items_, component, name_type, static_name, hash_or_id, type, match_retry); total_cancelled += heap_cancelled; - this->to_remove_ += heap_cancelled; + this->to_remove_add_(heap_cancelled); } // Cancel items in to_add_ diff --git a/esphome/core/scheduler.h b/esphome/core/scheduler.h index 0476513bb9..e545055fca 100644 --- a/esphome/core/scheduler.h +++ b/esphome/core/scheduler.h @@ -284,9 +284,9 @@ class Scheduler { #endif } // Cleanup logically deleted items from the scheduler - // Returns the number of items remaining after cleanup + // Returns true if items remain after cleanup // IMPORTANT: This method should only be called from the main thread (loop task). - size_t cleanup_(); + bool cleanup_(); // Remove and return the front item from the heap as a raw pointer. // Caller takes ownership and must either recycle or delete the item. // IMPORTANT: Caller must hold the scheduler lock before calling this function. @@ -395,15 +395,9 @@ class Scheduler { // erase() on every pop, which would be O(n). The queue is processed once per loop - // any items added during processing are left for the next loop iteration. - // Snapshot the queue end point - only process items that existed at loop start - // Items added during processing (by callbacks or other threads) run next loop - // No lock needed: single consumer (main loop), stale read just means we process less this iteration - size_t defer_queue_end = this->defer_queue_.size(); - // Fast path: nothing to process, avoid lock entirely. - // Safe without lock: single consumer (main loop) reads front_, and a stale size() read - // from a concurrent push can only make us see fewer items — they'll be processed next loop. - if (this->defer_queue_front_ >= defer_queue_end) + // Worst case is a one-loop-iteration delay before newly deferred items are processed. + if (this->defer_empty_()) return; // Merge lock acquisitions: instead of separate locks for move-out and recycle (2N+1 total), @@ -412,6 +406,13 @@ class Scheduler { SchedulerItem *item; this->lock_.lock(); + // Reset counter and snapshot queue end under lock + this->defer_count_clear_(); + size_t defer_queue_end = this->defer_queue_.size(); + if (this->defer_queue_front_ >= defer_queue_end) { + this->lock_.unlock(); + return; + } while (this->defer_queue_front_ < defer_queue_end) { // Take ownership of the item, leaving nullptr in the vector slot. // This is safe because: @@ -527,14 +528,150 @@ class Scheduler { Mutex lock_; std::vector items_; std::vector to_add_; + +#ifndef ESPHOME_THREAD_SINGLE + // Fast-path counter for process_to_add() to skip taking the lock when there is + // nothing to add. Uses std::atomic on platforms that support it, plain uint32_t + // otherwise. On non-atomic platforms, callers must hold the scheduler lock when + // mutating this counter. Not needed on single-threaded platforms where we can + // check to_add_.empty() directly. +#ifdef ESPHOME_THREAD_MULTI_ATOMICS + std::atomic to_add_count_{0}; +#else + uint32_t to_add_count_{0}; +#endif +#endif /* ESPHOME_THREAD_SINGLE */ + + // Fast-path helper for process_to_add() to decide if it can try the lock-free path. + // - On ESPHOME_THREAD_SINGLE: direct container check is safe (no concurrent writers). + // - On ESPHOME_THREAD_MULTI_ATOMICS: performs a lock-free check via to_add_count_. + // - On ESPHOME_THREAD_MULTI_NO_ATOMICS: always returns false to force the caller + // down the locked path; this is NOT a lock-free emptiness check on that platform. + bool to_add_empty_() const { +#ifdef ESPHOME_THREAD_SINGLE + return this->to_add_.empty(); +#elif defined(ESPHOME_THREAD_MULTI_ATOMICS) + return this->to_add_count_.load(std::memory_order_relaxed) == 0; +#else + return false; +#endif + } + + // Increment to_add_count_ (no-op on single-threaded platforms) + void to_add_count_increment_() { +#ifdef ESPHOME_THREAD_SINGLE + // No counter needed — to_add_empty_() checks the vector directly +#elif defined(ESPHOME_THREAD_MULTI_ATOMICS) + this->to_add_count_.fetch_add(1, std::memory_order_relaxed); +#else + this->to_add_count_++; +#endif + } + + // Reset to_add_count_ (no-op on single-threaded platforms) + void to_add_count_clear_() { +#ifdef ESPHOME_THREAD_SINGLE + // No counter needed — to_add_empty_() checks the vector directly +#elif defined(ESPHOME_THREAD_MULTI_ATOMICS) + this->to_add_count_.store(0, std::memory_order_relaxed); +#else + this->to_add_count_ = 0; +#endif + } + #ifndef ESPHOME_THREAD_SINGLE // Single-core platforms don't need the defer queue and save ~32 bytes of RAM // Using std::vector instead of std::deque avoids 512-byte chunked allocations // Index tracking avoids O(n) erase() calls when draining the queue each loop std::vector defer_queue_; // FIFO queue for defer() calls size_t defer_queue_front_{0}; // Index of first valid item in defer_queue_ (tracks consumed items) -#endif /* ESPHOME_THREAD_SINGLE */ + + // Fast-path counter for process_defer_queue_() to skip lock when nothing to process. +#ifdef ESPHOME_THREAD_MULTI_ATOMICS + std::atomic defer_count_{0}; +#else + uint32_t defer_count_{0}; +#endif + + bool defer_empty_() const { + // defer_queue_ only exists on multi-threaded platforms, so no ESPHOME_THREAD_SINGLE path + // ESPHOME_THREAD_MULTI_NO_ATOMICS: always take the lock +#ifdef ESPHOME_THREAD_MULTI_ATOMICS + return this->defer_count_.load(std::memory_order_relaxed) == 0; +#else + return false; +#endif + } + + void defer_count_increment_() { +#ifdef ESPHOME_THREAD_MULTI_ATOMICS + this->defer_count_.fetch_add(1, std::memory_order_relaxed); +#else + this->defer_count_++; +#endif + } + + void defer_count_clear_() { +#ifdef ESPHOME_THREAD_MULTI_ATOMICS + this->defer_count_.store(0, std::memory_order_relaxed); +#else + this->defer_count_ = 0; +#endif + } + +#endif /* ESPHOME_THREAD_SINGLE */ + + // Counter for items marked for removal. Incremented cross-thread in cancel_item_locked_(). + // On ESPHOME_THREAD_MULTI_ATOMICS this is read without a lock in the cleanup_() fast path; + // on ESPHOME_THREAD_MULTI_NO_ATOMICS the fast path is disabled so cleanup_() always takes the lock. +#ifdef ESPHOME_THREAD_MULTI_ATOMICS + std::atomic to_remove_{0}; +#else uint32_t to_remove_{0}; +#endif + + // Lock-free check if there are items to remove (for fast-path in cleanup_) + bool to_remove_empty_() const { +#ifdef ESPHOME_THREAD_MULTI_ATOMICS + return this->to_remove_.load(std::memory_order_relaxed) == 0; +#elif defined(ESPHOME_THREAD_SINGLE) + return this->to_remove_ == 0; +#else + return false; // Always take the lock path +#endif + } + + void to_remove_add_(uint32_t count) { +#ifdef ESPHOME_THREAD_MULTI_ATOMICS + this->to_remove_.fetch_add(count, std::memory_order_relaxed); +#else + this->to_remove_ += count; +#endif + } + + void to_remove_decrement_() { +#ifdef ESPHOME_THREAD_MULTI_ATOMICS + this->to_remove_.fetch_sub(1, std::memory_order_relaxed); +#else + this->to_remove_--; +#endif + } + + void to_remove_clear_() { +#ifdef ESPHOME_THREAD_MULTI_ATOMICS + this->to_remove_.store(0, std::memory_order_relaxed); +#else + this->to_remove_ = 0; +#endif + } + + uint32_t to_remove_count_() const { +#ifdef ESPHOME_THREAD_MULTI_ATOMICS + return this->to_remove_.load(std::memory_order_relaxed); +#else + return this->to_remove_; +#endif + } // Memory pool for recycling SchedulerItem objects to reduce heap churn. // Design decisions: From 5cc03d9befb7326fe708e7368778416eddd47253 Mon Sep 17 00:00:00 2001 From: Jesse Hills <3060199+jesserockz@users.noreply.github.com> Date: Wed, 18 Mar 2026 13:35:21 +1300 Subject: [PATCH 13/23] Bump version to 2026.3.0b4 --- Doxyfile | 2 +- esphome/const.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Doxyfile b/Doxyfile index 96295b3fc8..ea34106f36 100644 --- a/Doxyfile +++ b/Doxyfile @@ -48,7 +48,7 @@ PROJECT_NAME = ESPHome # could be handy for archiving the generated documentation or if some version # control system is used. -PROJECT_NUMBER = 2026.3.0b3 +PROJECT_NUMBER = 2026.3.0b4 # Using the PROJECT_BRIEF tag one can provide an optional one line description # for a project that appears at the top of each page and should give viewer a diff --git a/esphome/const.py b/esphome/const.py index 561a27d228..4ba8b1a23f 100644 --- a/esphome/const.py +++ b/esphome/const.py @@ -4,7 +4,7 @@ from enum import Enum from esphome.enum import StrEnum -__version__ = "2026.3.0b3" +__version__ = "2026.3.0b4" ALLOWED_NAME_CHARS = "abcdefghijklmnopqrstuvwxyz0123456789-_" VALID_SUBSTITUTIONS_CHARACTERS = ( From 2531fb1a021cf30e238754de5194e77a05be1800 Mon Sep 17 00:00:00 2001 From: Jonathan Swoboda <154711427+swoboda1337@users.noreply.github.com> Date: Tue, 17 Mar 2026 23:12:13 -0400 Subject: [PATCH 14/23] [voice_assistant][micro_wake_word] Fix null deref and missing error return (#14906) --- esphome/components/micro_wake_word/streaming_model.cpp | 1 + esphome/components/voice_assistant/voice_assistant.cpp | 2 ++ 2 files changed, 3 insertions(+) diff --git a/esphome/components/micro_wake_word/streaming_model.cpp b/esphome/components/micro_wake_word/streaming_model.cpp index 47d2c70e13..0ab6cd3772 100644 --- a/esphome/components/micro_wake_word/streaming_model.cpp +++ b/esphome/components/micro_wake_word/streaming_model.cpp @@ -80,6 +80,7 @@ bool StreamingModel::load_model_() { TfLiteTensor *output = this->interpreter_->output(0); if ((output->dims->size != 2) || (output->dims->data[0] != 1) || (output->dims->data[1] != 1)) { ESP_LOGE(TAG, "Streaming model tensor output dimension is not 1x1."); + return false; } if (output->type != kTfLiteUInt8) { diff --git a/esphome/components/voice_assistant/voice_assistant.cpp b/esphome/components/voice_assistant/voice_assistant.cpp index 51d52a8af8..15124e422f 100644 --- a/esphome/components/voice_assistant/voice_assistant.cpp +++ b/esphome/components/voice_assistant/voice_assistant.cpp @@ -619,6 +619,8 @@ void VoiceAssistant::start_playback_timeout_() { this->cancel_timeout("speaker-timeout"); this->set_state_(State::RESPONSE_FINISHED, State::RESPONSE_FINISHED); + if (this->api_client_ == nullptr) + return; api::VoiceAssistantAnnounceFinished msg; msg.success = true; this->api_client_->send_message(msg); From a28a6f7d934a905c907763fda31098a9616c5eec Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 17 Mar 2026 17:29:34 -1000 Subject: [PATCH 15/23] [http_request] Fix data race on update_info_ strings in update task The update task runs in a FreeRTOS thread on ESP32 and was writing directly to update_info_ std::string fields while the main loop reads them via API (as StringRef pointers into the string buffer), MQTT, web server, and Prometheus. This is undefined behavior that can cause use-after-free crashes when a string reallocation invalidates a StringRef held by the API serialization path. Move all update_info_ and state_ writes into the existing defer() callback so they execute on the main loop. The task now accumulates results in a local UpdateInfo struct and moves it into update_info_ in the deferred callback, eliminating the cross-thread data race with zero steady-state memory overhead. --- .../update/http_request_update.cpp | 140 ++++++++++-------- .../http_request/update/http_request_update.h | 3 + 2 files changed, 85 insertions(+), 58 deletions(-) diff --git a/esphome/components/http_request/update/http_request_update.cpp b/esphome/components/http_request/update/http_request_update.cpp index c40590af95..dc80ebc8f0 100644 --- a/esphome/components/http_request/update/http_request_update.cpp +++ b/esphome/components/http_request/update/http_request_update.cpp @@ -74,42 +74,41 @@ void HttpRequestUpdate::update() { #endif } -void HttpRequestUpdate::update_task(void *params) { - HttpRequestUpdate *this_update = (HttpRequestUpdate *) params; - - auto container = this_update->request_parent_->get(this_update->source_url_); +// Fetch and parse the update manifest. Returns a heap-allocated UpdateInfo on success +// (caller takes ownership), or nullptr on failure (with error_str set to the error message). +// Separated from update_task so that all code paths converge at a single defer() call, +// ensuring the allocated UpdateInfo is always passed to apply_manifest_result_main_loop_ +// which deletes it — preventing leaks regardless of which error path is taken. +update::UpdateInfo *HttpRequestUpdate::fetch_manifest_(const LogString *&error_str) { + auto container = this->request_parent_->get(this->source_url_); if (container == nullptr || container->status_code != HTTP_STATUS_OK) { - ESP_LOGE(TAG, "Failed to fetch manifest from %s", this_update->source_url_.c_str()); - // Defer to main loop to avoid race condition on component_state_ read-modify-write - this_update->defer([this_update]() { this_update->status_set_error(LOG_STR("Failed to fetch manifest")); }); - UPDATE_RETURN; + ESP_LOGE(TAG, "Failed to fetch manifest from %s", this->source_url_.c_str()); + error_str = LOG_STR("Failed to fetch manifest"); + return nullptr; } RAMAllocator allocator; uint8_t *data = allocator.allocate(container->content_length); if (data == nullptr) { ESP_LOGE(TAG, "Failed to allocate %zu bytes for manifest", container->content_length); - // Defer to main loop to avoid race condition on component_state_ read-modify-write - this_update->defer( - [this_update]() { this_update->status_set_error(LOG_STR("Failed to allocate memory for manifest")); }); + error_str = LOG_STR("Failed to allocate memory for manifest"); container->end(); - UPDATE_RETURN; + return nullptr; } auto read_result = http_read_fully(container.get(), data, container->content_length, MAX_READ_SIZE, - this_update->request_parent_->get_timeout()); + this->request_parent_->get_timeout()); if (read_result.status != HttpReadStatus::OK) { if (read_result.status == HttpReadStatus::TIMEOUT) { ESP_LOGE(TAG, "Timeout reading manifest"); } else { ESP_LOGE(TAG, "Error reading manifest: %d", read_result.error_code); } - // Defer to main loop to avoid race condition on component_state_ read-modify-write - this_update->defer([this_update]() { this_update->status_set_error(LOG_STR("Failed to read manifest")); }); + error_str = LOG_STR("Failed to read manifest"); allocator.deallocate(data, container->content_length); container->end(); - UPDATE_RETURN; + return nullptr; } size_t read_index = container->get_bytes_read(); size_t content_length = container->content_length; @@ -117,16 +116,18 @@ void HttpRequestUpdate::update_task(void *params) { container->end(); container.reset(); // Release ownership of the container's shared_ptr + auto *info = new update::UpdateInfo(); + bool valid = false; { // Scope to ensure JsonDocument is destroyed before deallocating buffer - valid = json::parse_json(data, read_index, [this_update](JsonObject root) -> bool { + valid = json::parse_json(data, read_index, [info](JsonObject root) -> bool { if (!root[ESPHOME_F("name")].is() || !root[ESPHOME_F("version")].is() || !root[ESPHOME_F("builds")].is()) { ESP_LOGE(TAG, "Manifest does not contain required fields"); return false; } - this_update->update_info_.title = root[ESPHOME_F("name")].as(); - this_update->update_info_.latest_version = root[ESPHOME_F("version")].as(); + info->title = root[ESPHOME_F("name")].as(); + info->latest_version = root[ESPHOME_F("version")].as(); auto builds_array = root[ESPHOME_F("builds")].as(); for (auto build : builds_array) { @@ -144,13 +145,13 @@ void HttpRequestUpdate::update_task(void *params) { ESP_LOGE(TAG, "Manifest does not contain required fields"); return false; } - this_update->update_info_.firmware_url = ota[ESPHOME_F("path")].as(); - this_update->update_info_.md5 = ota[ESPHOME_F("md5")].as(); + info->firmware_url = ota[ESPHOME_F("path")].as(); + info->md5 = ota[ESPHOME_F("md5")].as(); if (ota[ESPHOME_F("summary")].is()) - this_update->update_info_.summary = ota[ESPHOME_F("summary")].as(); + info->summary = ota[ESPHOME_F("summary")].as(); if (ota[ESPHOME_F("release_url")].is()) - this_update->update_info_.release_url = ota[ESPHOME_F("release_url")].as(); + info->release_url = ota[ESPHOME_F("release_url")].as(); return true; } @@ -161,62 +162,85 @@ void HttpRequestUpdate::update_task(void *params) { allocator.deallocate(data, content_length); if (!valid) { - ESP_LOGE(TAG, "Failed to parse JSON from %s", this_update->source_url_.c_str()); - // Defer to main loop to avoid race condition on component_state_ read-modify-write - this_update->defer([this_update]() { this_update->status_set_error(LOG_STR("Failed to parse manifest JSON")); }); - UPDATE_RETURN; + delete info; + ESP_LOGE(TAG, "Failed to parse JSON from %s", this->source_url_.c_str()); + error_str = LOG_STR("Failed to parse manifest JSON"); + return nullptr; } - // Merge source_url_ and this_update->update_info_.firmware_url - if (this_update->update_info_.firmware_url.find("http") == std::string::npos) { - std::string path = this_update->update_info_.firmware_url; + // Merge source_url_ and firmware_url + if (info->firmware_url.find("http") == std::string::npos) { + std::string path = info->firmware_url; if (path[0] == '/') { - std::string domain = this_update->source_url_.substr(0, this_update->source_url_.find('/', 8)); - this_update->update_info_.firmware_url = domain + path; + std::string domain = this->source_url_.substr(0, this->source_url_.find('/', 8)); + info->firmware_url = domain + path; } else { - std::string domain = this_update->source_url_.substr(0, this_update->source_url_.rfind('/') + 1); - this_update->update_info_.firmware_url = domain + path; + std::string domain = this->source_url_.substr(0, this->source_url_.rfind('/') + 1); + info->firmware_url = domain + path; } } #ifdef ESPHOME_PROJECT_VERSION - this_update->update_info_.current_version = ESPHOME_PROJECT_VERSION; + info->current_version = ESPHOME_PROJECT_VERSION; #else - this_update->update_info_.current_version = ESPHOME_VERSION; + info->current_version = ESPHOME_VERSION; #endif + return info; +} + +void HttpRequestUpdate::update_task(void *params) { + HttpRequestUpdate *this_update = (HttpRequestUpdate *) params; + + const LogString *error_str = nullptr; + auto *info = this_update->fetch_manifest_(error_str); + + update::UpdateState new_state = update::UPDATE_STATE_UNKNOWN; bool trigger_update_available = false; - if (this_update->update_info_.latest_version.empty() || - this_update->update_info_.latest_version == this_update->update_info_.current_version) { - this_update->state_ = update::UPDATE_STATE_NO_UPDATE; - } else { - if (this_update->state_ != update::UPDATE_STATE_AVAILABLE) { - trigger_update_available = true; + if (info != nullptr) { + if (info->latest_version.empty() || info->latest_version == info->current_version) { + new_state = update::UPDATE_STATE_NO_UPDATE; + } else { + new_state = update::UPDATE_STATE_AVAILABLE; + if (this_update->state_ != update::UPDATE_STATE_AVAILABLE) { + trigger_update_available = true; + } } - this_update->state_ = update::UPDATE_STATE_AVAILABLE; } - // Defer to main loop to ensure thread-safe execution of: - // - status_clear_error() performs non-atomic read-modify-write on component_state_ - // - publish_state() triggers API callbacks that write to the shared protobuf buffer - // which can be corrupted if accessed concurrently from task and main loop threads - // - update_available trigger to ensure consistent state when the trigger fires - this_update->defer([this_update, trigger_update_available]() { - this_update->update_info_.has_progress = false; - this_update->update_info_.progress = 0.0f; - - this_update->status_clear_error(); - this_update->publish_state(); - - if (trigger_update_available) { - this_update->get_update_available_trigger()->trigger(this_update->update_info_); - } + // Defer to the main loop so all update_info_ and state_ writes happen on the + // same thread as readers (API, MQTT, web server). This is a single defer for + // both success and error paths to avoid multiple std::function instantiations. + // Ownership of info transfers to apply_manifest_result_main_loop_. + this_update->defer([this_update, trigger_update_available, new_state, error_str, info]() { + this_update->apply_manifest_result_main_loop_(info, error_str, new_state, trigger_update_available); }); UPDATE_RETURN; } +void HttpRequestUpdate::apply_manifest_result_main_loop_(update::UpdateInfo *info, const LogString *error_str, + update::UpdateState new_state, bool trigger_update_available) { + if (error_str != nullptr) { + delete info; + this->status_set_error(error_str); + return; + } + this->update_info_ = std::move(*info); + this->update_info_.has_progress = false; + this->update_info_.progress = 0.0f; + this->state_ = new_state; + delete info; + + this->status_clear_error(); + this->publish_state(); + + if (trigger_update_available) { + this->get_update_available_trigger()->trigger(this->update_info_); + } +} + void HttpRequestUpdate::perform(bool force) { if (this->state_ != update::UPDATE_STATE_AVAILABLE && !force) { return; diff --git a/esphome/components/http_request/update/http_request_update.h b/esphome/components/http_request/update/http_request_update.h index b8350346f9..8b6fb2443f 100644 --- a/esphome/components/http_request/update/http_request_update.h +++ b/esphome/components/http_request/update/http_request_update.h @@ -37,6 +37,9 @@ class HttpRequestUpdate final : public update::UpdateEntity, public PollingCompo std::string source_url_; static void update_task(void *params); + update::UpdateInfo *fetch_manifest_(const LogString *&error_str); + void apply_manifest_result_main_loop_(update::UpdateInfo *info, const LogString *error_str, + update::UpdateState new_state, bool trigger_update_available); #ifdef USE_ESP32 TaskHandle_t update_task_handle_{nullptr}; #endif From f496936b31cbdd2e5e6ba1b3d837cb933da65597 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 17 Mar 2026 18:23:51 -1000 Subject: [PATCH 16/23] [http_request] Reduce flash cost of update race fix - Move state computation to main loop callback (avoids 2 extra lambda captures) - Add error_str field to UpdateInfo so error info rides with the pointer - Allocate UpdateInfo once at top of fetch_manifest_ (single ownership path) - Lambda captures only 2 pointers (8 bytes), fits std::function SBO Reduces ESP8266 flash delta from +480 to ~+288 bytes. --- .../update/http_request_update.cpp | 75 +++++++++---------- .../http_request/update/http_request_update.h | 5 +- esphome/components/update/update_entity.h | 1 + 3 files changed, 37 insertions(+), 44 deletions(-) diff --git a/esphome/components/http_request/update/http_request_update.cpp b/esphome/components/http_request/update/http_request_update.cpp index dc80ebc8f0..98d389119e 100644 --- a/esphome/components/http_request/update/http_request_update.cpp +++ b/esphome/components/http_request/update/http_request_update.cpp @@ -74,27 +74,27 @@ void HttpRequestUpdate::update() { #endif } -// Fetch and parse the update manifest. Returns a heap-allocated UpdateInfo on success -// (caller takes ownership), or nullptr on failure (with error_str set to the error message). -// Separated from update_task so that all code paths converge at a single defer() call, -// ensuring the allocated UpdateInfo is always passed to apply_manifest_result_main_loop_ -// which deletes it — preventing leaks regardless of which error path is taken. -update::UpdateInfo *HttpRequestUpdate::fetch_manifest_(const LogString *&error_str) { +// Fetch and parse the update manifest. Always returns a heap-allocated UpdateInfo +// (caller takes ownership). On failure, error_str is set; on success it is nullptr. +// Single allocation at the top ensures simple ownership — every path returns info. +update::UpdateInfo *HttpRequestUpdate::fetch_manifest_() { + auto *info = new update::UpdateInfo(); + auto container = this->request_parent_->get(this->source_url_); if (container == nullptr || container->status_code != HTTP_STATUS_OK) { ESP_LOGE(TAG, "Failed to fetch manifest from %s", this->source_url_.c_str()); - error_str = LOG_STR("Failed to fetch manifest"); - return nullptr; + info->error_str = LOG_STR("Failed to fetch manifest"); + return info; } RAMAllocator allocator; uint8_t *data = allocator.allocate(container->content_length); if (data == nullptr) { ESP_LOGE(TAG, "Failed to allocate %zu bytes for manifest", container->content_length); - error_str = LOG_STR("Failed to allocate memory for manifest"); container->end(); - return nullptr; + info->error_str = LOG_STR("Failed to allocate memory for manifest"); + return info; } auto read_result = http_read_fully(container.get(), data, container->content_length, MAX_READ_SIZE, @@ -105,10 +105,10 @@ update::UpdateInfo *HttpRequestUpdate::fetch_manifest_(const LogString *&error_s } else { ESP_LOGE(TAG, "Error reading manifest: %d", read_result.error_code); } - error_str = LOG_STR("Failed to read manifest"); allocator.deallocate(data, container->content_length); container->end(); - return nullptr; + info->error_str = LOG_STR("Failed to read manifest"); + return info; } size_t read_index = container->get_bytes_read(); size_t content_length = container->content_length; @@ -116,8 +116,6 @@ update::UpdateInfo *HttpRequestUpdate::fetch_manifest_(const LogString *&error_s container->end(); container.reset(); // Release ownership of the container's shared_ptr - auto *info = new update::UpdateInfo(); - bool valid = false; { // Scope to ensure JsonDocument is destroyed before deallocating buffer valid = json::parse_json(data, read_index, [info](JsonObject root) -> bool { @@ -162,10 +160,9 @@ update::UpdateInfo *HttpRequestUpdate::fetch_manifest_(const LogString *&error_s allocator.deallocate(data, content_length); if (!valid) { - delete info; ESP_LOGE(TAG, "Failed to parse JSON from %s", this->source_url_.c_str()); - error_str = LOG_STR("Failed to parse manifest JSON"); - return nullptr; + info->error_str = LOG_STR("Failed to parse manifest JSON"); + return info; } // Merge source_url_ and firmware_url @@ -192,41 +189,37 @@ update::UpdateInfo *HttpRequestUpdate::fetch_manifest_(const LogString *&error_s void HttpRequestUpdate::update_task(void *params) { HttpRequestUpdate *this_update = (HttpRequestUpdate *) params; - const LogString *error_str = nullptr; - auto *info = this_update->fetch_manifest_(error_str); - - update::UpdateState new_state = update::UPDATE_STATE_UNKNOWN; - bool trigger_update_available = false; - - if (info != nullptr) { - if (info->latest_version.empty() || info->latest_version == info->current_version) { - new_state = update::UPDATE_STATE_NO_UPDATE; - } else { - new_state = update::UPDATE_STATE_AVAILABLE; - if (this_update->state_ != update::UPDATE_STATE_AVAILABLE) { - trigger_update_available = true; - } - } - } + auto *info = this_update->fetch_manifest_(); // Defer to the main loop so all update_info_ and state_ writes happen on the // same thread as readers (API, MQTT, web server). This is a single defer for // both success and error paths to avoid multiple std::function instantiations. - // Ownership of info transfers to apply_manifest_result_main_loop_. - this_update->defer([this_update, trigger_update_available, new_state, error_str, info]() { - this_update->apply_manifest_result_main_loop_(info, error_str, new_state, trigger_update_available); - }); + // info == nullptr signals an error (specific error already logged by fetch_manifest_). + // Lambda captures only 2 pointers (8 bytes) — fits in std::function SBO on all platforms. + this_update->defer([this_update, info]() { this_update->apply_manifest_result_main_loop_(info); }); UPDATE_RETURN; } -void HttpRequestUpdate::apply_manifest_result_main_loop_(update::UpdateInfo *info, const LogString *error_str, - update::UpdateState new_state, bool trigger_update_available) { - if (error_str != nullptr) { +void HttpRequestUpdate::apply_manifest_result_main_loop_(update::UpdateInfo *info) { + if (info->error_str != nullptr) { + this->status_set_error(info->error_str); delete info; - this->status_set_error(error_str); return; } + + // Determine new state on main loop (avoids extra lambda captures from task) + bool trigger_update_available = false; + update::UpdateState new_state; + if (info->latest_version.empty() || info->latest_version == info->current_version) { + new_state = update::UPDATE_STATE_NO_UPDATE; + } else { + new_state = update::UPDATE_STATE_AVAILABLE; + if (this->state_ != update::UPDATE_STATE_AVAILABLE) { + trigger_update_available = true; + } + } + this->update_info_ = std::move(*info); this->update_info_.has_progress = false; this->update_info_.progress = 0.0f; diff --git a/esphome/components/http_request/update/http_request_update.h b/esphome/components/http_request/update/http_request_update.h index 8b6fb2443f..dc0e8910b8 100644 --- a/esphome/components/http_request/update/http_request_update.h +++ b/esphome/components/http_request/update/http_request_update.h @@ -37,9 +37,8 @@ class HttpRequestUpdate final : public update::UpdateEntity, public PollingCompo std::string source_url_; static void update_task(void *params); - update::UpdateInfo *fetch_manifest_(const LogString *&error_str); - void apply_manifest_result_main_loop_(update::UpdateInfo *info, const LogString *error_str, - update::UpdateState new_state, bool trigger_update_available); + update::UpdateInfo *fetch_manifest_(); + void apply_manifest_result_main_loop_(update::UpdateInfo *info); #ifdef USE_ESP32 TaskHandle_t update_task_handle_{nullptr}; #endif diff --git a/esphome/components/update/update_entity.h b/esphome/components/update/update_entity.h index 82eaacaf76..3b6d4fa245 100644 --- a/esphome/components/update/update_entity.h +++ b/esphome/components/update/update_entity.h @@ -16,6 +16,7 @@ struct UpdateInfo { std::string release_url; std::string firmware_url; std::string md5; + const LogString *error_str{nullptr}; // Set on fetch failure, nullptr on success bool has_progress{false}; float progress; }; From 211273d46c43d3aac421f0bde9dedb9587077e78 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 17 Mar 2026 18:26:22 -1000 Subject: [PATCH 17/23] [http_request] Inline fetch_manifest_ back into update_task MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Eliminates separate function call overhead — compiler can optimize across the single function. Saves ~32 bytes flash on ESP8266. --- .../update/http_request_update.cpp | 223 +++++++++--------- .../http_request/update/http_request_update.h | 1 - 2 files changed, 109 insertions(+), 115 deletions(-) diff --git a/esphome/components/http_request/update/http_request_update.cpp b/esphome/components/http_request/update/http_request_update.cpp index 98d389119e..2f14efc3d0 100644 --- a/esphome/components/http_request/update/http_request_update.cpp +++ b/esphome/components/http_request/update/http_request_update.cpp @@ -74,127 +74,122 @@ void HttpRequestUpdate::update() { #endif } -// Fetch and parse the update manifest. Always returns a heap-allocated UpdateInfo -// (caller takes ownership). On failure, error_str is set; on success it is nullptr. -// Single allocation at the top ensures simple ownership — every path returns info. -update::UpdateInfo *HttpRequestUpdate::fetch_manifest_() { - auto *info = new update::UpdateInfo(); - - auto container = this->request_parent_->get(this->source_url_); - - if (container == nullptr || container->status_code != HTTP_STATUS_OK) { - ESP_LOGE(TAG, "Failed to fetch manifest from %s", this->source_url_.c_str()); - info->error_str = LOG_STR("Failed to fetch manifest"); - return info; - } - - RAMAllocator allocator; - uint8_t *data = allocator.allocate(container->content_length); - if (data == nullptr) { - ESP_LOGE(TAG, "Failed to allocate %zu bytes for manifest", container->content_length); - container->end(); - info->error_str = LOG_STR("Failed to allocate memory for manifest"); - return info; - } - - auto read_result = http_read_fully(container.get(), data, container->content_length, MAX_READ_SIZE, - this->request_parent_->get_timeout()); - if (read_result.status != HttpReadStatus::OK) { - if (read_result.status == HttpReadStatus::TIMEOUT) { - ESP_LOGE(TAG, "Timeout reading manifest"); - } else { - ESP_LOGE(TAG, "Error reading manifest: %d", read_result.error_code); - } - allocator.deallocate(data, container->content_length); - container->end(); - info->error_str = LOG_STR("Failed to read manifest"); - return info; - } - size_t read_index = container->get_bytes_read(); - size_t content_length = container->content_length; - - container->end(); - container.reset(); // Release ownership of the container's shared_ptr - - bool valid = false; - { // Scope to ensure JsonDocument is destroyed before deallocating buffer - valid = json::parse_json(data, read_index, [info](JsonObject root) -> bool { - if (!root[ESPHOME_F("name")].is() || !root[ESPHOME_F("version")].is() || - !root[ESPHOME_F("builds")].is()) { - ESP_LOGE(TAG, "Manifest does not contain required fields"); - return false; - } - info->title = root[ESPHOME_F("name")].as(); - info->latest_version = root[ESPHOME_F("version")].as(); - - auto builds_array = root[ESPHOME_F("builds")].as(); - for (auto build : builds_array) { - if (!build[ESPHOME_F("chipFamily")].is()) { - ESP_LOGE(TAG, "Manifest does not contain required fields"); - return false; - } - if (build[ESPHOME_F("chipFamily")] == ESPHOME_VARIANT) { - if (!build[ESPHOME_F("ota")].is()) { - ESP_LOGE(TAG, "Manifest does not contain required fields"); - return false; - } - JsonObject ota = build[ESPHOME_F("ota")].as(); - if (!ota[ESPHOME_F("path")].is() || !ota[ESPHOME_F("md5")].is()) { - ESP_LOGE(TAG, "Manifest does not contain required fields"); - return false; - } - info->firmware_url = ota[ESPHOME_F("path")].as(); - info->md5 = ota[ESPHOME_F("md5")].as(); - - if (ota[ESPHOME_F("summary")].is()) - info->summary = ota[ESPHOME_F("summary")].as(); - if (ota[ESPHOME_F("release_url")].is()) - info->release_url = ota[ESPHOME_F("release_url")].as(); - - return true; - } - } - return false; - }); - } - allocator.deallocate(data, content_length); - - if (!valid) { - ESP_LOGE(TAG, "Failed to parse JSON from %s", this->source_url_.c_str()); - info->error_str = LOG_STR("Failed to parse manifest JSON"); - return info; - } - - // Merge source_url_ and firmware_url - if (info->firmware_url.find("http") == std::string::npos) { - std::string path = info->firmware_url; - if (path[0] == '/') { - std::string domain = this->source_url_.substr(0, this->source_url_.find('/', 8)); - info->firmware_url = domain + path; - } else { - std::string domain = this->source_url_.substr(0, this->source_url_.rfind('/') + 1); - info->firmware_url = domain + path; - } - } - -#ifdef ESPHOME_PROJECT_VERSION - info->current_version = ESPHOME_PROJECT_VERSION; -#else - info->current_version = ESPHOME_VERSION; -#endif - - return info; -} - void HttpRequestUpdate::update_task(void *params) { HttpRequestUpdate *this_update = (HttpRequestUpdate *) params; - auto *info = this_update->fetch_manifest_(); + // Allocate once — every path below returns via the single defer at the end. + // On failure, error_str is set; on success it is nullptr. + auto *info = new update::UpdateInfo(); + auto container = this_update->request_parent_->get(this_update->source_url_); + + if (container == nullptr || container->status_code != HTTP_STATUS_OK) { + ESP_LOGE(TAG, "Failed to fetch manifest from %s", this_update->source_url_.c_str()); + info->error_str = LOG_STR("Failed to fetch manifest"); + goto defer; // NOLINT(cppcoreguidelines-avoid-goto) + } + + { + RAMAllocator allocator; + uint8_t *data = allocator.allocate(container->content_length); + if (data == nullptr) { + ESP_LOGE(TAG, "Failed to allocate %zu bytes for manifest", container->content_length); + container->end(); + info->error_str = LOG_STR("Failed to allocate memory for manifest"); + goto defer; // NOLINT(cppcoreguidelines-avoid-goto) + } + + auto read_result = http_read_fully(container.get(), data, container->content_length, MAX_READ_SIZE, + this_update->request_parent_->get_timeout()); + if (read_result.status != HttpReadStatus::OK) { + if (read_result.status == HttpReadStatus::TIMEOUT) { + ESP_LOGE(TAG, "Timeout reading manifest"); + } else { + ESP_LOGE(TAG, "Error reading manifest: %d", read_result.error_code); + } + allocator.deallocate(data, container->content_length); + container->end(); + info->error_str = LOG_STR("Failed to read manifest"); + goto defer; // NOLINT(cppcoreguidelines-avoid-goto) + } + size_t read_index = container->get_bytes_read(); + size_t content_length = container->content_length; + + container->end(); + container.reset(); // Release ownership of the container's shared_ptr + + bool valid = false; + { // Scope to ensure JsonDocument is destroyed before deallocating buffer + valid = json::parse_json(data, read_index, [info](JsonObject root) -> bool { + if (!root[ESPHOME_F("name")].is() || !root[ESPHOME_F("version")].is() || + !root[ESPHOME_F("builds")].is()) { + ESP_LOGE(TAG, "Manifest does not contain required fields"); + return false; + } + info->title = root[ESPHOME_F("name")].as(); + info->latest_version = root[ESPHOME_F("version")].as(); + + auto builds_array = root[ESPHOME_F("builds")].as(); + for (auto build : builds_array) { + if (!build[ESPHOME_F("chipFamily")].is()) { + ESP_LOGE(TAG, "Manifest does not contain required fields"); + return false; + } + if (build[ESPHOME_F("chipFamily")] == ESPHOME_VARIANT) { + if (!build[ESPHOME_F("ota")].is()) { + ESP_LOGE(TAG, "Manifest does not contain required fields"); + return false; + } + JsonObject ota = build[ESPHOME_F("ota")].as(); + if (!ota[ESPHOME_F("path")].is() || !ota[ESPHOME_F("md5")].is()) { + ESP_LOGE(TAG, "Manifest does not contain required fields"); + return false; + } + info->firmware_url = ota[ESPHOME_F("path")].as(); + info->md5 = ota[ESPHOME_F("md5")].as(); + + if (ota[ESPHOME_F("summary")].is()) + info->summary = ota[ESPHOME_F("summary")].as(); + if (ota[ESPHOME_F("release_url")].is()) + info->release_url = ota[ESPHOME_F("release_url")].as(); + + return true; + } + } + return false; + }); + } + allocator.deallocate(data, content_length); + + if (!valid) { + ESP_LOGE(TAG, "Failed to parse JSON from %s", this_update->source_url_.c_str()); + info->error_str = LOG_STR("Failed to parse manifest JSON"); + goto defer; // NOLINT(cppcoreguidelines-avoid-goto) + } + + // Merge source_url_ and firmware_url + if (info->firmware_url.find("http") == std::string::npos) { + std::string path = info->firmware_url; + if (path[0] == '/') { + std::string domain = this_update->source_url_.substr(0, this_update->source_url_.find('/', 8)); + info->firmware_url = domain + path; + } else { + std::string domain = this_update->source_url_.substr(0, this_update->source_url_.rfind('/') + 1); + info->firmware_url = domain + path; + } + } + +#ifdef ESPHOME_PROJECT_VERSION + info->current_version = ESPHOME_PROJECT_VERSION; +#else + info->current_version = ESPHOME_VERSION; +#endif + } + +defer: // Defer to the main loop so all update_info_ and state_ writes happen on the // same thread as readers (API, MQTT, web server). This is a single defer for // both success and error paths to avoid multiple std::function instantiations. - // info == nullptr signals an error (specific error already logged by fetch_manifest_). // Lambda captures only 2 pointers (8 bytes) — fits in std::function SBO on all platforms. this_update->defer([this_update, info]() { this_update->apply_manifest_result_main_loop_(info); }); diff --git a/esphome/components/http_request/update/http_request_update.h b/esphome/components/http_request/update/http_request_update.h index dc0e8910b8..87116e8282 100644 --- a/esphome/components/http_request/update/http_request_update.h +++ b/esphome/components/http_request/update/http_request_update.h @@ -37,7 +37,6 @@ class HttpRequestUpdate final : public update::UpdateEntity, public PollingCompo std::string source_url_; static void update_task(void *params); - update::UpdateInfo *fetch_manifest_(); void apply_manifest_result_main_loop_(update::UpdateInfo *info); #ifdef USE_ESP32 TaskHandle_t update_task_handle_{nullptr}; From e93e064304c2110c3e7f51f172e45301815209f9 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 17 Mar 2026 18:27:53 -1000 Subject: [PATCH 18/23] [http_request] Inline apply_manifest_result_main_loop_ into defer lambda MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One less method — the logic lives directly in the lambda body. No flash change (compiler was already inlining the call). --- .../update/http_request_update.cpp | 66 +++++++++---------- .../http_request/update/http_request_update.h | 1 - 2 files changed, 32 insertions(+), 35 deletions(-) diff --git a/esphome/components/http_request/update/http_request_update.cpp b/esphome/components/http_request/update/http_request_update.cpp index 2f14efc3d0..981f1adac1 100644 --- a/esphome/components/http_request/update/http_request_update.cpp +++ b/esphome/components/http_request/update/http_request_update.cpp @@ -191,44 +191,42 @@ defer: // same thread as readers (API, MQTT, web server). This is a single defer for // both success and error paths to avoid multiple std::function instantiations. // Lambda captures only 2 pointers (8 bytes) — fits in std::function SBO on all platforms. - this_update->defer([this_update, info]() { this_update->apply_manifest_result_main_loop_(info); }); + this_update->defer([this_update, info]() { + if (info->error_str != nullptr) { + this_update->status_set_error(info->error_str); + delete info; + return; + } + + // Determine new state on main loop (avoids extra lambda captures from task) + bool trigger_update_available = false; + update::UpdateState new_state; + if (info->latest_version.empty() || info->latest_version == info->current_version) { + new_state = update::UPDATE_STATE_NO_UPDATE; + } else { + new_state = update::UPDATE_STATE_AVAILABLE; + if (this_update->state_ != update::UPDATE_STATE_AVAILABLE) { + trigger_update_available = true; + } + } + + this_update->update_info_ = std::move(*info); + this_update->update_info_.has_progress = false; + this_update->update_info_.progress = 0.0f; + this_update->state_ = new_state; + delete info; + + this_update->status_clear_error(); + this_update->publish_state(); + + if (trigger_update_available) { + this_update->get_update_available_trigger()->trigger(this_update->update_info_); + } + }); UPDATE_RETURN; } -void HttpRequestUpdate::apply_manifest_result_main_loop_(update::UpdateInfo *info) { - if (info->error_str != nullptr) { - this->status_set_error(info->error_str); - delete info; - return; - } - - // Determine new state on main loop (avoids extra lambda captures from task) - bool trigger_update_available = false; - update::UpdateState new_state; - if (info->latest_version.empty() || info->latest_version == info->current_version) { - new_state = update::UPDATE_STATE_NO_UPDATE; - } else { - new_state = update::UPDATE_STATE_AVAILABLE; - if (this->state_ != update::UPDATE_STATE_AVAILABLE) { - trigger_update_available = true; - } - } - - this->update_info_ = std::move(*info); - this->update_info_.has_progress = false; - this->update_info_.progress = 0.0f; - this->state_ = new_state; - delete info; - - this->status_clear_error(); - this->publish_state(); - - if (trigger_update_available) { - this->get_update_available_trigger()->trigger(this->update_info_); - } -} - void HttpRequestUpdate::perform(bool force) { if (this->state_ != update::UPDATE_STATE_AVAILABLE && !force) { return; diff --git a/esphome/components/http_request/update/http_request_update.h b/esphome/components/http_request/update/http_request_update.h index 87116e8282..b8350346f9 100644 --- a/esphome/components/http_request/update/http_request_update.h +++ b/esphome/components/http_request/update/http_request_update.h @@ -37,7 +37,6 @@ class HttpRequestUpdate final : public update::UpdateEntity, public PollingCompo std::string source_url_; static void update_task(void *params); - void apply_manifest_result_main_loop_(update::UpdateInfo *info); #ifdef USE_ESP32 TaskHandle_t update_task_handle_{nullptr}; #endif From 564128127b6aaf9b5522e97f808b5fb95f26d580 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 17 Mar 2026 18:33:55 -1000 Subject: [PATCH 19/23] [http_request] Call container->end() on non-OK status, fix comment Address Copilot review feedback: - Call container->end() when status_code != HTTP_STATUS_OK (pre-existing bug, but easy to fix while we're here) - Update error_str comment to say "update check failure" not "fetch failure" --- esphome/components/http_request/update/http_request_update.cpp | 2 ++ esphome/components/update/update_entity.h | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/esphome/components/http_request/update/http_request_update.cpp b/esphome/components/http_request/update/http_request_update.cpp index 981f1adac1..4c0ae916c2 100644 --- a/esphome/components/http_request/update/http_request_update.cpp +++ b/esphome/components/http_request/update/http_request_update.cpp @@ -85,6 +85,8 @@ void HttpRequestUpdate::update_task(void *params) { if (container == nullptr || container->status_code != HTTP_STATUS_OK) { ESP_LOGE(TAG, "Failed to fetch manifest from %s", this_update->source_url_.c_str()); + if (container != nullptr) + container->end(); info->error_str = LOG_STR("Failed to fetch manifest"); goto defer; // NOLINT(cppcoreguidelines-avoid-goto) } diff --git a/esphome/components/update/update_entity.h b/esphome/components/update/update_entity.h index 3b6d4fa245..504efd5c6f 100644 --- a/esphome/components/update/update_entity.h +++ b/esphome/components/update/update_entity.h @@ -16,7 +16,7 @@ struct UpdateInfo { std::string release_url; std::string firmware_url; std::string md5; - const LogString *error_str{nullptr}; // Set on fetch failure, nullptr on success + const LogString *error_str{nullptr}; // Set on update check failure, nullptr on success bool has_progress{false}; float progress; }; From 17476934c858153fa0b05c58bf19a2d9d56a02eb Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 17 Mar 2026 18:34:24 -1000 Subject: [PATCH 20/23] [http_request] Guard against empty firmware_url before path merge Accessing path[0] on an empty string is undefined behavior. Add an empty check before the relative URL merge logic. --- esphome/components/http_request/update/http_request_update.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/esphome/components/http_request/update/http_request_update.cpp b/esphome/components/http_request/update/http_request_update.cpp index 4c0ae916c2..422b6367a4 100644 --- a/esphome/components/http_request/update/http_request_update.cpp +++ b/esphome/components/http_request/update/http_request_update.cpp @@ -170,7 +170,7 @@ void HttpRequestUpdate::update_task(void *params) { } // Merge source_url_ and firmware_url - if (info->firmware_url.find("http") == std::string::npos) { + if (!info->firmware_url.empty() && info->firmware_url.find("http") == std::string::npos) { std::string path = info->firmware_url; if (path[0] == '/') { std::string domain = this_update->source_url_.substr(0, this_update->source_url_.find('/', 8)); From fd7051744508b3e1b590ab96768983c8986782c8 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 17 Mar 2026 18:39:01 -1000 Subject: [PATCH 21/23] [http_request] Use file-local TaskResult instead of modifying UpdateInfo Address review feedback: - Move error_str out of public UpdateInfo into file-local TaskResult - Add container.reset() before vTaskDelete (doesn't call destructors) - Remove redundant has_progress/progress assignments after move - Add comment on delete after std::move --- .../update/http_request_update.cpp | 36 +++++++++++-------- esphome/components/update/update_entity.h | 1 - 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/esphome/components/http_request/update/http_request_update.cpp b/esphome/components/http_request/update/http_request_update.cpp index 422b6367a4..0c5fdb2699 100644 --- a/esphome/components/http_request/update/http_request_update.cpp +++ b/esphome/components/http_request/update/http_request_update.cpp @@ -23,6 +23,12 @@ namespace http_request { static const char *const TAG = "http_request.update"; +// Wraps UpdateInfo + error for the task→main-loop handoff. +struct TaskResult { + update::UpdateInfo info; + const LogString *error_str{nullptr}; +}; + static const size_t MAX_READ_SIZE = 256; static constexpr uint32_t INITIAL_CHECK_INTERVAL_ID = 0; static constexpr uint32_t INITIAL_CHECK_INTERVAL_MS = 10000; @@ -79,7 +85,8 @@ void HttpRequestUpdate::update_task(void *params) { // Allocate once — every path below returns via the single defer at the end. // On failure, error_str is set; on success it is nullptr. - auto *info = new update::UpdateInfo(); + auto *result = new TaskResult(); + auto *info = &result->info; auto container = this_update->request_parent_->get(this_update->source_url_); @@ -87,7 +94,7 @@ void HttpRequestUpdate::update_task(void *params) { ESP_LOGE(TAG, "Failed to fetch manifest from %s", this_update->source_url_.c_str()); if (container != nullptr) container->end(); - info->error_str = LOG_STR("Failed to fetch manifest"); + result->error_str = LOG_STR("Failed to fetch manifest"); goto defer; // NOLINT(cppcoreguidelines-avoid-goto) } @@ -97,7 +104,7 @@ void HttpRequestUpdate::update_task(void *params) { if (data == nullptr) { ESP_LOGE(TAG, "Failed to allocate %zu bytes for manifest", container->content_length); container->end(); - info->error_str = LOG_STR("Failed to allocate memory for manifest"); + result->error_str = LOG_STR("Failed to allocate memory for manifest"); goto defer; // NOLINT(cppcoreguidelines-avoid-goto) } @@ -111,7 +118,7 @@ void HttpRequestUpdate::update_task(void *params) { } allocator.deallocate(data, container->content_length); container->end(); - info->error_str = LOG_STR("Failed to read manifest"); + result->error_str = LOG_STR("Failed to read manifest"); goto defer; // NOLINT(cppcoreguidelines-avoid-goto) } size_t read_index = container->get_bytes_read(); @@ -165,7 +172,7 @@ void HttpRequestUpdate::update_task(void *params) { if (!valid) { ESP_LOGE(TAG, "Failed to parse JSON from %s", this_update->source_url_.c_str()); - info->error_str = LOG_STR("Failed to parse manifest JSON"); + result->error_str = LOG_STR("Failed to parse manifest JSON"); goto defer; // NOLINT(cppcoreguidelines-avoid-goto) } @@ -189,21 +196,24 @@ void HttpRequestUpdate::update_task(void *params) { } defer: + // Release container before vTaskDelete (which doesn't call destructors) + container.reset(); + // Defer to the main loop so all update_info_ and state_ writes happen on the // same thread as readers (API, MQTT, web server). This is a single defer for // both success and error paths to avoid multiple std::function instantiations. // Lambda captures only 2 pointers (8 bytes) — fits in std::function SBO on all platforms. - this_update->defer([this_update, info]() { - if (info->error_str != nullptr) { - this_update->status_set_error(info->error_str); - delete info; + this_update->defer([this_update, result]() { + if (result->error_str != nullptr) { + this_update->status_set_error(result->error_str); + delete result; return; } // Determine new state on main loop (avoids extra lambda captures from task) bool trigger_update_available = false; update::UpdateState new_state; - if (info->latest_version.empty() || info->latest_version == info->current_version) { + if (result->info.latest_version.empty() || result->info.latest_version == result->info.current_version) { new_state = update::UPDATE_STATE_NO_UPDATE; } else { new_state = update::UPDATE_STATE_AVAILABLE; @@ -212,11 +222,9 @@ defer: } } - this_update->update_info_ = std::move(*info); - this_update->update_info_.has_progress = false; - this_update->update_info_.progress = 0.0f; + this_update->update_info_ = std::move(result->info); this_update->state_ = new_state; - delete info; + delete result; // Safe: moved-from state is valid for destruction this_update->status_clear_error(); this_update->publish_state(); diff --git a/esphome/components/update/update_entity.h b/esphome/components/update/update_entity.h index 504efd5c6f..82eaacaf76 100644 --- a/esphome/components/update/update_entity.h +++ b/esphome/components/update/update_entity.h @@ -16,7 +16,6 @@ struct UpdateInfo { std::string release_url; std::string firmware_url; std::string md5; - const LogString *error_str{nullptr}; // Set on update check failure, nullptr on success bool has_progress{false}; float progress; }; From 7b04f4ea8be10c13fe56a80cc5701b4d0bd52a95 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 17 Mar 2026 18:51:12 -1000 Subject: [PATCH 22/23] [http_request] Soften SBO comment to say supported toolchains --- esphome/components/http_request/update/http_request_update.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/esphome/components/http_request/update/http_request_update.cpp b/esphome/components/http_request/update/http_request_update.cpp index 0c5fdb2699..a15dc61675 100644 --- a/esphome/components/http_request/update/http_request_update.cpp +++ b/esphome/components/http_request/update/http_request_update.cpp @@ -202,7 +202,7 @@ defer: // Defer to the main loop so all update_info_ and state_ writes happen on the // same thread as readers (API, MQTT, web server). This is a single defer for // both success and error paths to avoid multiple std::function instantiations. - // Lambda captures only 2 pointers (8 bytes) — fits in std::function SBO on all platforms. + // Lambda captures only 2 pointers (8 bytes) — fits in std::function SBO on supported toolchains. this_update->defer([this_update, result]() { if (result->error_str != nullptr) { this_update->status_set_error(result->error_str); From e436265d6c5d163ae0ca86d3e96e8ed6bb9649b6 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 17 Mar 2026 19:21:15 -1000 Subject: [PATCH 23/23] [http_request] Prevent double update task launch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Guard against launching a second FreeRTOS update task while one is already running. Without this, calling update() twice (e.g. from both the polling interval and the initial check interval) could spawn two concurrent tasks both writing to the same UpdateInfo. With the single-defer structure from the race fix PR, clearing the handle only needs to happen in one place — at the top of the deferred lambda, before any other work. --- .../components/http_request/update/http_request_update.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/esphome/components/http_request/update/http_request_update.cpp b/esphome/components/http_request/update/http_request_update.cpp index a15dc61675..1c52a28105 100644 --- a/esphome/components/http_request/update/http_request_update.cpp +++ b/esphome/components/http_request/update/http_request_update.cpp @@ -74,6 +74,10 @@ void HttpRequestUpdate::update() { } this->cancel_interval(INITIAL_CHECK_INTERVAL_ID); #ifdef USE_ESP32 + if (this->update_task_handle_ != nullptr) { + ESP_LOGW(TAG, "Update check already in progress"); + return; + } xTaskCreate(HttpRequestUpdate::update_task, "update_task", 8192, (void *) this, 1, &this->update_task_handle_); #else this->update_task(this); @@ -204,6 +208,9 @@ defer: // both success and error paths to avoid multiple std::function instantiations. // Lambda captures only 2 pointers (8 bytes) — fits in std::function SBO on supported toolchains. this_update->defer([this_update, result]() { +#ifdef USE_ESP32 + this_update->update_task_handle_ = nullptr; +#endif if (result->error_str != nullptr) { this_update->status_set_error(result->error_str); delete result;