From ffbd0dcbfcdc13a655f8af890647a3303f49967f Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 10 Apr 2026 15:41:57 -1000 Subject: [PATCH] [esphome.ota] Set pending-enable flags before main-loop wake (fix race) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The wake-hook call (esphome_wake_ota_component_any_context / App.wake_ota_component_any_context) was placed AFTER xTaskNotifyGive()/wake_loop_any_context() in both the fast-select callback and the raw-TCP accept callback. That opened a race: the main task could wake, run a full iteration (draining has_pending_enable_loop_requests_), and finish before the flag-set ran — losing the pending-enable request until the next unrelated socket event happened to re-trigger the path. Swap the order so the pending-enable flags are set first, then the main task is woken. The main-loop iteration triggered by the wake is now guaranteed to see the pending request. Note: host's yield_with_select_ path already sets and consumes the flag on the main thread with no cross-task wake in between, so it has no race and is unchanged. --- esphome/components/socket/lwip_raw_tcp_impl.cpp | 12 ++++++------ esphome/core/application.cpp | 7 ++++--- esphome/core/lwip_fast_select.c | 13 +++++++------ 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/esphome/components/socket/lwip_raw_tcp_impl.cpp b/esphome/components/socket/lwip_raw_tcp_impl.cpp index 3d1679553e..cfd8cf0daa 100644 --- a/esphome/components/socket/lwip_raw_tcp_impl.cpp +++ b/esphome/components/socket/lwip_raw_tcp_impl.cpp @@ -857,16 +857,16 @@ err_t LWIPRawListenImpl::accept_fn_(struct tcp_pcb *newpcb, err_t err) { tcp_err(newpcb, LWIPRawListenImpl::s_queued_err_fn); tcp_recv(newpcb, LWIPRawListenImpl::s_queued_recv_fn); LWIP_LOG("Accepted connection, queue size: %d", this->accepted_socket_count_); - // Wake the main loop immediately so it can accept the new connection. - esphome::wake_loop_any_context(); #ifdef USE_OTA // Mark the OTA component loop to be re-enabled if it disabled itself while idle. - // This only sets pending-enable flags; the wake_loop_any_context() call above has - // already woken the main loop, which will process the pending enable on its next - // iteration. Safe to call from RP2040's low-priority user IRQ context — it only - // writes volatile bools, no heap or locks. + // This MUST happen before wake_loop_any_context() below — otherwise the main loop + // could wake, run a full iteration, and finish before we set the pending-enable + // flags, losing the wake event. Safe from RP2040's low-priority user IRQ context: + // it only writes volatile bools, no heap or locks. esphome::App.wake_ota_component_any_context(); #endif + // Wake the main loop immediately so it can accept the new connection. + esphome::wake_loop_any_context(); return ERR_OK; } diff --git a/esphome/core/application.cpp b/esphome/core/application.cpp index 9b7091f59a..9311b9fbf0 100644 --- a/esphome/core/application.cpp +++ b/esphome/core/application.cpp @@ -450,9 +450,10 @@ void Application::enable_pending_loops_() { } #if defined(USE_OTA) && defined(USE_LWIP_FAST_SELECT) -// Called from the LwIP TCP/IP task via esphome_socket_event_callback() on NETCONN_EVT_RCVPLUS. -// Only marks the OTA component as pending loop-enable; the fast-select callback itself has -// already woken the main task via xTaskNotifyGive(). +// Called from the LwIP TCP/IP task via esphome_socket_event_callback() on NETCONN_EVT_RCVPLUS, +// BEFORE the callback calls xTaskNotifyGive() — the flag-set must happen before the wake, +// otherwise the main task could wake, run a full iteration, and miss the pending-enable. +// Only marks the OTA component as pending loop-enable; does not itself wake the main task. extern "C" void esphome_wake_ota_component_any_context() { App.wake_ota_component_any_context(); } #endif diff --git a/esphome/core/lwip_fast_select.c b/esphome/core/lwip_fast_select.c index 32b79f288d..96eddf00ff 100644 --- a/esphome/core/lwip_fast_select.c +++ b/esphome/core/lwip_fast_select.c @@ -180,16 +180,17 @@ static void esphome_socket_event_callback(struct netconn *conn, enum netconn_evt // (rcvevent++ with a NULL pbuf or error in recvmbox), so error conditions // already wake the main loop through the RCVPLUS path. if (evt == NETCONN_EVT_RCVPLUS) { +#ifdef USE_OTA + // Mark the OTA component loop to be re-enabled if it disabled itself while idle. + // This MUST happen before xTaskNotifyGive below — otherwise the main task could + // wake, run a full iteration, and finish before we set the pending-enable flags, + // causing the wake event to be lost until the next unrelated socket activity. + esphome_wake_ota_component_any_context(); +#endif TaskHandle_t task = esphome_main_task_handle; if (task != NULL) { xTaskNotifyGive(task); } -#ifdef USE_OTA - // Mark the OTA component loop to be re-enabled if it disabled itself while idle. - // Only sets pending-enable flags — the xTaskNotifyGive above has already woken - // the main task, which will process the pending enable on its next iteration. - esphome_wake_ota_component_any_context(); -#endif } }