diff --git a/esphome/components/socket/__init__.py b/esphome/components/socket/__init__.py index 06b0ba2bf7..4f6634bd7a 100644 --- a/esphome/components/socket/__init__.py +++ b/esphome/components/socket/__init__.py @@ -127,13 +127,12 @@ def require_wake_loop_threadsafe() -> None: Call this from components that need to wake the main event loop from background threads. - On ESP32: Uses FreeRTOS task notifications (<1 us, ISR-safe, no socket needed). + On ESP32: Uses FreeRTOS task notifications (<1 us, no socket needed). On other platforms: Uses a shared UDP loopback socket mechanism (~208 bytes RAM). This call is a no-op if networking is not enabled in the configuration. - On ESP32, this is safe to call from ISR context. - On other platforms, this is for background thread context only, NOT ISR context. + IMPORTANT: This is for background task context only, NOT ISR context. Example: from esphome.components import socket diff --git a/esphome/core/application.cpp b/esphome/core/application.cpp index 9de5aa1e9e..8950e6029e 100644 --- a/esphome/core/application.cpp +++ b/esphome/core/application.cpp @@ -634,16 +634,7 @@ void Application::unregister_socket_fd(int fd) { void Application::yield_with_select_(uint32_t delay_ms) { // Delay while monitoring sockets. When delay_ms is 0, always yield() to ensure other tasks run. #if defined(USE_SOCKET_SELECT_SUPPORT) && defined(USE_ESP32) - // ESP32 fast path: direct rcvevent reads (~858 ns for 4 sockets vs 133 us for lwip_select) - if (!this->socket_fds_.empty()) [[likely]] { - FD_ZERO(&this->read_fds_); - for (int fd : this->socket_fds_) { - if (esphome_lwip_socket_has_data(fd)) { - FD_SET(fd, &this->read_fds_); - } - } - } - + // ESP32 fast path: no fd_set needed — is_socket_ready_() reads rcvevent directly (~215 ns per socket) if (delay_ms == 0) [[unlikely]] { yield(); return; diff --git a/esphome/core/application.h b/esphome/core/application.h index fba2ae54c2..5b087f59fe 100644 --- a/esphome/core/application.h +++ b/esphome/core/application.h @@ -24,10 +24,14 @@ #endif #ifdef USE_SOCKET_SELECT_SUPPORT +#ifdef USE_ESP32 +#include "esphome/core/lwip_fast_select.h" +#else #include -#if defined(USE_WAKE_LOOP_THREADSAFE) && !defined(USE_ESP32) +#ifdef USE_WAKE_LOOP_THREADSAFE #include #endif +#endif #endif // USE_SOCKET_SELECT_SUPPORT #ifdef USE_BINARY_SENSOR @@ -497,10 +501,10 @@ class Application { bool is_socket_ready(int fd) const { return fd >= 0 && this->is_socket_ready_(fd); } #ifdef USE_WAKE_LOOP_THREADSAFE - /// Wake the main event loop from a FreeRTOS task or ISR. - /// Thread-safe, can be called from any context to immediately wake the main loop. - /// On ESP32: uses xTaskNotifyGive (<1 us, ISR-safe) - /// On other platforms: uses UDP loopback socket (NOT ISR-safe) + /// Wake the main event loop from another FreeRTOS task. + /// Thread-safe, but must only be called from task context (NOT ISR-safe). + /// On ESP32: uses xTaskNotifyGive (<1 us) + /// On other platforms: uses UDP loopback socket void wake_loop_threadsafe(); #endif #endif @@ -513,8 +517,13 @@ class Application { /// Fast path for Socket::ready() via friendship - skips negative fd check. /// Safe because: fd was validated in register_socket_fd() at registration time, /// and Socket::ready() only calls this when loop_monitored_ is true (registration succeeded). - /// FD_ISSET may include its own upper bounds check depending on platform. +#ifdef USE_ESP32 + /// ESP32: direct rcvevent read — always fresh, no fd_set snapshot needed (~215 ns) + bool is_socket_ready_(int fd) const { return esphome_lwip_socket_has_data(fd); } +#else + /// Other platforms: check fd_set populated by select() bool is_socket_ready_(int fd) const { return FD_ISSET(fd, &this->read_fds_); } +#endif #endif void register_component_(Component *comp); @@ -605,12 +614,10 @@ class Application { bool socket_fds_changed_{false}; // Flag to rebuild base_read_fds_ when socket_fds_ changes #endif -#ifdef USE_SOCKET_SELECT_SUPPORT - // Variable-sized members - fd_set read_fds_{}; // Working fd_set: populated by select() or direct rcvevent reads -#ifndef USE_ESP32 - fd_set base_read_fds_{}; // Cached fd_set rebuilt only when socket_fds_ changes (select() path) -#endif +#if defined(USE_SOCKET_SELECT_SUPPORT) && !defined(USE_ESP32) + // Variable-sized members (not needed on ESP32 — is_socket_ready_ reads rcvevent directly) + fd_set read_fds_{}; // Working fd_set: populated by select() + fd_set base_read_fds_{}; // Cached fd_set rebuilt only when socket_fds_ changes #endif // StaticVectors (largest members - contain actual array data inline) diff --git a/esphome/core/lwip_fast_select.c b/esphome/core/lwip_fast_select.c index 9a54b4ef04..8684909dc1 100644 --- a/esphome/core/lwip_fast_select.c +++ b/esphome/core/lwip_fast_select.c @@ -24,7 +24,7 @@ static TaskHandle_t s_main_loop_task = NULL; static netconn_callback s_original_callback = NULL; // Wrapper callback: calls original event_callback + notifies main loop task. -// Called from LwIP's TCP/IP thread when socket events occur. +// Called from LwIP's TCP/IP thread when socket events occur (task context, not ISR). static void esphome_socket_event_callback(struct netconn *conn, enum netconn_evt evt, u16_t len) { // Call original LwIP event_callback first — updates rcvevent/sendevent/errevent, // signals any select() waiters. This preserves all LwIP behavior. @@ -33,7 +33,7 @@ static void esphome_socket_event_callback(struct netconn *conn, enum netconn_evt } // Wake the main loop task if sleeping in ulTaskNotifyTake(). // Only notify on receive events to avoid spurious wakeups from send-ready events. - // xTaskNotifyGive is thread-safe and ISR-safe, costs <1 us. + // xTaskNotifyGive is safe from task context (LwIP TCP/IP thread). NOT ISR-safe. if (evt == NETCONN_EVT_RCVPLUS) { TaskHandle_t task = s_main_loop_task; if (task != NULL) { @@ -46,7 +46,13 @@ void esphome_lwip_fast_select_init(void) { s_main_loop_task = xTaskGetCurrentTas bool esphome_lwip_socket_has_data(int fd) { struct lwip_sock *sock = lwip_socket_dbg_get_socket(fd); - return sock != NULL && sock->conn != NULL && sock->rcvevent > 0; + if (sock == NULL || sock->conn == NULL) + return false; + // rcvevent is modified by LwIP's TCP/IP thread in event_callback. + // Use atomic load for C11 memory model correctness. On Xtensa/RISC-V (ESP32) + // aligned 16-bit reads are naturally atomic, so this compiles to a plain load + // but prevents compiler reordering. + return __atomic_load_n(&sock->rcvevent, __ATOMIC_RELAXED) > 0; } void esphome_lwip_hook_socket(int fd) { @@ -59,7 +65,10 @@ void esphome_lwip_hook_socket(int fd) { s_original_callback = sock->conn->callback; } - // Replace with our wrapper + // Replace with our wrapper. + // Thread safety: pointer writes are atomic on ESP32 (32-bit aligned). + // The TCP/IP thread may read conn->callback concurrently, but it will see + // either the old or new pointer — both are valid (our wrapper calls the original). sock->conn->callback = esphome_socket_event_callback; } @@ -68,7 +77,9 @@ void esphome_lwip_unhook_socket(int fd) { if (sock == NULL || sock->conn == NULL) return; - // Restore original callback + // Restore original callback. + // Thread safety: same as hook — pointer write is atomic on ESP32. + // TCP/IP thread sees either wrapper or original, both are safe. if (s_original_callback != NULL) { sock->conn->callback = s_original_callback; } diff --git a/esphome/core/lwip_fast_select.h b/esphome/core/lwip_fast_select.h index 8bd2a052af..4c47c13f20 100644 --- a/esphome/core/lwip_fast_select.h +++ b/esphome/core/lwip_fast_select.h @@ -2,7 +2,6 @@ // Fast socket monitoring for ESP32 (ESP-IDF LwIP) // Replaces lwip_select() with direct rcvevent reads and FreeRTOS task notifications. -// See fast_select.md for design rationale and benchmarks. #include @@ -28,8 +27,8 @@ void esphome_lwip_hook_socket(int fd); /// Must be called from the main loop before closing the socket. void esphome_lwip_unhook_socket(int fd); -/// Wake the main loop task from any thread or ISR — costs <1 us. -/// Replaces the UDP loopback socket wake mechanism. +/// Wake the main loop task from another FreeRTOS task — costs <1 us. +/// NOT ISR-safe — must only be called from task context. void esphome_lwip_wake_main_loop(void); #ifdef __cplusplus diff --git a/tests/components/socket/test_wake_loop_threadsafe.py b/tests/components/socket/test_wake_loop_threadsafe.py index a40b6068a8..45d354f7b3 100644 --- a/tests/components/socket/test_wake_loop_threadsafe.py +++ b/tests/components/socket/test_wake_loop_threadsafe.py @@ -1,9 +1,16 @@ from esphome.components import socket +from esphome.const import KEY_CORE, KEY_TARGET_PLATFORM, PLATFORM_ESP8266 from esphome.core import CORE +def _setup_non_esp32_platform() -> None: + """Set up CORE.data with a non-ESP32 platform for testing.""" + CORE.data[KEY_CORE] = {KEY_TARGET_PLATFORM: PLATFORM_ESP8266} + + def test_require_wake_loop_threadsafe__first_call() -> None: """Test that first call sets up define and consumes socket.""" + _setup_non_esp32_platform() CORE.config = {"wifi": True} socket.require_wake_loop_threadsafe() @@ -32,6 +39,7 @@ def test_require_wake_loop_threadsafe__idempotent() -> None: def test_require_wake_loop_threadsafe__multiple_calls() -> None: """Test that multiple calls only set up once.""" + _setup_non_esp32_platform() # Call three times CORE.config = {"openthread": True} socket.require_wake_loop_threadsafe()