From 5d2f56856507c30af5f9e2b0cd206630367d1105 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 29 Mar 2026 19:15:24 -1000 Subject: [PATCH 1/2] Switch to atomic uint8_t flag, fix race condition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace atomic counter with a simple flag to avoid underflow race where consumer could decrement before producer increments. Use uint8_t instead of bool to avoid GCC Xtensa indirect call issue. Clear flag before draining — if a new event arrives between clear and xQueueReceive, the flag is set again and caught next iteration. --- .../wifi/wifi_component_esp_idf.cpp | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/esphome/components/wifi/wifi_component_esp_idf.cpp b/esphome/components/wifi/wifi_component_esp_idf.cpp index cc8af38f75..22353bbd7f 100644 --- a/esphome/components/wifi/wifi_component_esp_idf.cpp +++ b/esphome/components/wifi/wifi_component_esp_idf.cpp @@ -49,10 +49,12 @@ static const char *const TAG = "wifi_esp32"; static EventGroupHandle_t s_wifi_event_group; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) static QueueHandle_t s_event_queue; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) -// Fast-path counter to skip xQueueReceive kernel call when queue is empty. -// Incremented from event handler (ISR-safe context), read from main loop. -static std::atomic s_event_count{0}; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) -static esp_netif_t *s_sta_netif = nullptr; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) +// Fast-path flag to skip xQueueReceive kernel call when queue is empty. +// Set from ESP-IDF event loop handler (task context), cleared from main loop. +// std::atomic is avoided because GCC on Xtensa generates indirect function +// calls for atomic bool operations instead of inlining them. +static std::atomic s_event_pending{0}; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) +static esp_netif_t *s_sta_netif = nullptr; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) #ifdef USE_WIFI_AP static esp_netif_t *s_ap_netif = nullptr; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) #endif // USE_WIFI_AP @@ -143,7 +145,7 @@ void event_handler(void *arg, esp_event_base_t event_base, int32_t event_id, voi if (xQueueSend(s_event_queue, &to_send, 0L) != pdPASS) { delete to_send; // NOLINT(cppcoreguidelines-owning-memory) } else { - s_event_count.fetch_add(1, std::memory_order_release); + s_event_pending.store(1, std::memory_order_release); } } @@ -732,16 +734,20 @@ const char *get_disconnect_reason_str(uint8_t reason) { void WiFiComponent::wifi_loop_() { // Fast path: skip xQueueReceive kernel call when no events are pending. // The atomic load is much cheaper than entering the FreeRTOS kernel. - if (s_event_count.load(std::memory_order_acquire) == 0) + if (s_event_pending.load(std::memory_order_acquire) == 0) return; + // Clear flag before draining. If a new event arrives between the clear + // and xQueueReceive, the flag will be set again and we'll catch it + // next loop iteration. This avoids any counter underflow issues. + s_event_pending.store(0, std::memory_order_relaxed); + while (true) { IDFWiFiEvent *data; if (xQueueReceive(s_event_queue, &data, 0L) != pdTRUE) { // no event ready break; } - s_event_count.fetch_sub(1, std::memory_order_relaxed); // process event wifi_process_event_(data); From ac2f837c7ff49d1dfdcaa4ce9259560ea9dadc79 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 29 Mar 2026 19:16:03 -1000 Subject: [PATCH 2/2] Use relaxed memory ordering for event pending flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The flag is just a hint — xQueueReceive with its own internal synchronization is the source of truth. Relaxed ordering avoids unnecessary fence cost on ESP targets. Worst case is missing an event for one loop iteration. --- esphome/components/wifi/wifi_component_esp_idf.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/esphome/components/wifi/wifi_component_esp_idf.cpp b/esphome/components/wifi/wifi_component_esp_idf.cpp index 22353bbd7f..3ad40709f0 100644 --- a/esphome/components/wifi/wifi_component_esp_idf.cpp +++ b/esphome/components/wifi/wifi_component_esp_idf.cpp @@ -145,7 +145,8 @@ void event_handler(void *arg, esp_event_base_t event_base, int32_t event_id, voi if (xQueueSend(s_event_queue, &to_send, 0L) != pdPASS) { delete to_send; // NOLINT(cppcoreguidelines-owning-memory) } else { - s_event_pending.store(1, std::memory_order_release); + // relaxed: this is just a hint; xQueueReceive is the source of truth + s_event_pending.store(1, std::memory_order_relaxed); } } @@ -734,7 +735,7 @@ const char *get_disconnect_reason_str(uint8_t reason) { void WiFiComponent::wifi_loop_() { // Fast path: skip xQueueReceive kernel call when no events are pending. // The atomic load is much cheaper than entering the FreeRTOS kernel. - if (s_event_pending.load(std::memory_order_acquire) == 0) + if (s_event_pending.load(std::memory_order_relaxed) == 0) return; // Clear flag before draining. If a new event arrives between the clear