[core] Remove pre-sleep socket scan from fast select path (#15639)

esphome/components/socket/bsd_sockets_impl.cpp

@@ -14,38 +14,34 @@ BSDSocketImpl::BSDSocketImpl(int fd, bool monitor_loop) {
   if (!monitor_loop || this->fd_ < 0)
     return;
 #ifdef USE_LWIP_FAST_SELECT
-  // Cache lwip_sock pointer and register for monitoring (hooks callback internally)
-  this->cached_sock_ = esphome_lwip_get_sock(this->fd_);
-  this->loop_monitored_ = App.register_socket(this->cached_sock_);
+  this->cached_sock_ = hook_fd_for_fast_select(this->fd_);
 #else
   this->loop_monitored_ = App.register_socket_fd(this->fd_);
 #endif
 }
 
-BSDSocketImpl::~BSDSocketImpl() {
-  if (!this->closed_) {
-    this->close();
-  }
-}
+BSDSocketImpl::~BSDSocketImpl() { this->close(); }
 
 int BSDSocketImpl::close() {
-  if (!this->closed_) {
-    // Unregister before closing to avoid dangling pointer in monitored set
-#ifdef USE_LWIP_FAST_SELECT
-    if (this->loop_monitored_) {
-      App.unregister_socket(this->cached_sock_);
-      this->cached_sock_ = nullptr;
-    }
-#else
-    if (this->loop_monitored_) {
-      App.unregister_socket_fd(this->fd_);
-    }
-#endif
-    int ret = ::close(this->fd_);
-    this->closed_ = true;
-    return ret;
+  if (this->fd_ < 0) {
+    // Already closed, or never opened.
+    return 0;
   }
-  return 0;
+#ifdef USE_LWIP_FAST_SELECT
+  // Null the cached lwip_sock pointer before closing. The underlying lwip slot can be
+  // recycled for a new connection as soon as ::close() returns, so anything that might
+  // dereference cached_sock_ post-close (e.g. setsockopt(TCP_NODELAY)) would otherwise
+  // touch an unrelated socket's pcb. No per-socket callback unhook is needed —
+  // all LwIP sockets share the same static event_callback.
+  this->cached_sock_ = nullptr;
+#else
+  if (this->loop_monitored_) {
+    App.unregister_socket_fd(this->fd_);
+  }
+#endif
+  int ret = ::close(this->fd_);
+  this->fd_ = -1;  // Sentinel for "closed" — prevents double-close and makes use-after-close visible.
+  return ret;
 }
 
 int BSDSocketImpl::setblocking(bool blocking) {

esphome/components/socket/bsd_sockets_impl.h

@@ -119,12 +119,21 @@ class BSDSocketImpl {
   int get_fd() const { return this->fd_; }
 
  protected:
+  // fd_ < 0 means "not open" — used both pre-open (initial state) and post-close. This
+  // replaces a separate closed_ flag: close() sets fd_ = -1 after ::close(), and the
+  // destructor / double-close path just check fd_ < 0.
   int fd_{-1};
 #ifdef USE_LWIP_FAST_SELECT
-  struct lwip_sock *cached_sock_{nullptr};  // Cached for direct rcvevent read in ready()
-#endif
-  bool closed_{false};
+  // Cached lwip_sock pointer used for direct rcvevent reads in ready() on the
+  // fast-select path. Replaces loop_monitored_: null means this socket is not being
+  // monitored for read events — either monitoring was not requested, the fd was
+  // invalid, or esphome_lwip_get_sock() failed. Non-null means the netconn event
+  // callback was hooked and notifications are flowing. close() nulls this to prevent
+  // use-after-free via a recycled lwip slot.
+  struct lwip_sock *cached_sock_{nullptr};
+#else
   bool loop_monitored_{false};
+#endif
 };
 
 }  // namespace esphome::socket

esphome/components/socket/lwip_sockets_impl.cpp

@@ -14,38 +14,34 @@ LwIPSocketImpl::LwIPSocketImpl(int fd, bool monitor_loop) {
   if (!monitor_loop || this->fd_ < 0)
     return;
 #ifdef USE_LWIP_FAST_SELECT
-  // Cache lwip_sock pointer and register for monitoring (hooks callback internally)
-  this->cached_sock_ = esphome_lwip_get_sock(this->fd_);
-  this->loop_monitored_ = App.register_socket(this->cached_sock_);
+  this->cached_sock_ = hook_fd_for_fast_select(this->fd_);
 #else
   this->loop_monitored_ = App.register_socket_fd(this->fd_);
 #endif
 }
 
-LwIPSocketImpl::~LwIPSocketImpl() {
-  if (!this->closed_) {
-    this->close();
-  }
-}
+LwIPSocketImpl::~LwIPSocketImpl() { this->close(); }
 
 int LwIPSocketImpl::close() {
-  if (!this->closed_) {
-    // Unregister before closing to avoid dangling pointer in monitored set
-#ifdef USE_LWIP_FAST_SELECT
-    if (this->loop_monitored_) {
-      App.unregister_socket(this->cached_sock_);
-      this->cached_sock_ = nullptr;
-    }
-#else
-    if (this->loop_monitored_) {
-      App.unregister_socket_fd(this->fd_);
-    }
-#endif
-    int ret = lwip_close(this->fd_);
-    this->closed_ = true;
-    return ret;
+  if (this->fd_ < 0) {
+    // Already closed, or never opened.
+    return 0;
   }
-  return 0;
+#ifdef USE_LWIP_FAST_SELECT
+  // Null the cached lwip_sock pointer before closing. The underlying lwip slot can be
+  // recycled for a new connection as soon as lwip_close() returns, so anything that
+  // might dereference cached_sock_ post-close (e.g. setsockopt(TCP_NODELAY)) would
+  // otherwise touch an unrelated socket's pcb. No per-socket callback unhook is needed —
+  // all LwIP sockets share the same static event_callback.
+  this->cached_sock_ = nullptr;
+#else
+  if (this->loop_monitored_) {
+    App.unregister_socket_fd(this->fd_);
+  }
+#endif
+  int ret = lwip_close(this->fd_);
+  this->fd_ = -1;  // Sentinel for "closed" — prevents double-close and makes use-after-close visible.
+  return ret;
 }
 
 int LwIPSocketImpl::setblocking(bool blocking) {

esphome/components/socket/lwip_sockets_impl.h

@@ -85,12 +85,21 @@ class LwIPSocketImpl {
   int get_fd() const { return this->fd_; }
 
  protected:
+  // fd_ < 0 means "not open" — used both pre-open (initial state) and post-close. This
+  // replaces a separate closed_ flag: close() sets fd_ = -1 after lwip_close(), and the
+  // destructor / double-close path just check fd_ < 0.
   int fd_{-1};
 #ifdef USE_LWIP_FAST_SELECT
-  struct lwip_sock *cached_sock_{nullptr};  // Cached for direct rcvevent read in ready()
-#endif
-  bool closed_{false};
+  // Cached lwip_sock pointer used for direct rcvevent reads in ready() on the
+  // fast-select path. Replaces loop_monitored_: null means this socket is not being
+  // monitored for read events — either monitoring was not requested, the fd was
+  // invalid, or esphome_lwip_get_sock() failed. Non-null means the netconn event
+  // callback was hooked and notifications are flowing. close() nulls this to prevent
+  // use-after-free via a recycled lwip slot.
+  struct lwip_sock *cached_sock_{nullptr};
+#else
   bool loop_monitored_{false};
+#endif
 };
 
 }  // namespace esphome::socket

esphome/components/socket/socket.h

@@ -42,8 +42,23 @@ using ListenSocket = LWIPRawListenImpl;
 
 #ifdef USE_LWIP_FAST_SELECT
 /// Shared ready() helper using cached lwip_sock pointer for direct rcvevent read.
-inline bool socket_ready(struct lwip_sock *cached_sock, bool loop_monitored) {
-  return !loop_monitored || (cached_sock != nullptr && esphome_lwip_socket_has_data(cached_sock));
+/// cached_sock == nullptr means the socket is not monitored (monitor_loop was false, fd
+/// was invalid, or esphome_lwip_get_sock() failed) — in that case return true so the
+/// caller attempts the read and handles blocking itself.
+inline bool socket_ready(struct lwip_sock *cached_sock) {
+  return cached_sock == nullptr || esphome_lwip_socket_has_data(cached_sock);
+}
+
+/// Resolve an fd to its lwip_sock and install the netconn event-callback hook so the
+/// main loop is woken by FreeRTOS task notifications when data arrives. Shared between
+/// BSD and LwIP socket impls on the fast-select path. Returns the cached lwip_sock
+/// pointer (or nullptr if the fd does not map to a valid lwip_sock).
+inline struct lwip_sock *hook_fd_for_fast_select(int fd) {
+  struct lwip_sock *sock = esphome_lwip_get_sock(fd);
+  if (sock != nullptr) {
+    esphome_lwip_hook_socket(sock);
+  }
+  return sock;
 }
 #elif defined(USE_HOST)
 /// Shared ready() helper for fd-based socket implementations.
@@ -69,7 +84,7 @@ bool socket_ready_fd(int fd, bool loop_monitored);
 #if defined(USE_SOCKET_IMPL_BSD_SOCKETS) || defined(USE_SOCKET_IMPL_LWIP_SOCKETS)
 inline bool Socket::ready() const {
 #ifdef USE_LWIP_FAST_SELECT
-  return socket_ready(this->cached_sock_, this->loop_monitored_);
+  return socket_ready(this->cached_sock_);
 #else
   return socket_ready_fd(this->fd_, this->loop_monitored_);
 #endif

esphome/core/application.cpp

@@ -509,32 +509,7 @@ void Application::enable_pending_loops_() {
   }
 }
 
-#ifdef USE_LWIP_FAST_SELECT
-bool Application::register_socket(struct lwip_sock *sock) {
-  // It modifies monitored_sockets_ without locking — must only be called from the main loop.
-  if (sock == nullptr)
-    return false;
-  esphome_lwip_hook_socket(sock);
-  this->monitored_sockets_.push_back(sock);
-  return true;
-}
-
-void Application::unregister_socket(struct lwip_sock *sock) {
-  // It modifies monitored_sockets_ without locking — must only be called from the main loop.
-  for (size_t i = 0; i < this->monitored_sockets_.size(); i++) {
-    if (this->monitored_sockets_[i] != sock)
-      continue;
-
-    // Swap with last element and pop - O(1) removal since order doesn't matter.
-    // No need to unhook the netconn callback — all LwIP sockets share the same
-    // static event_callback, and the socket will be closed by the caller.
-    if (i < this->monitored_sockets_.size() - 1)
-      this->monitored_sockets_[i] = this->monitored_sockets_.back();
-    this->monitored_sockets_.pop_back();
-    return;
-  }
-}
-#elif defined(USE_HOST)
+#ifdef USE_HOST
 bool Application::register_socket_fd(int fd) {
   // WARNING: This function is NOT thread-safe and must only be called from the main loop
   // It modifies socket_fds_ and related variables without locking

esphome/core/application.h

@@ -345,16 +345,13 @@ class Application {
 
   Scheduler scheduler;
 
-  /// Register/unregister a socket to be monitored for read events.
-  /// WARNING: These functions are NOT thread-safe. They must only be called from the main loop.
-#ifdef USE_LWIP_FAST_SELECT
-  /// Fast select path: hooks netconn callback and registers for monitoring.
-  /// @return true if registration was successful, false if sock is null
-  bool register_socket(struct lwip_sock *sock);
-  void unregister_socket(struct lwip_sock *sock);
-#elif defined(USE_HOST)
-  /// Fallback select() path: monitors file descriptors.
+#ifdef USE_HOST
+  /// Register/unregister a socket file descriptor with the host select() fallback loop.
+  /// USE_LWIP_FAST_SELECT builds do not use this API — sockets hook the lwIP netconn
+  /// event_callback directly (see socket.h hook_fd_for_fast_select) and rely on FreeRTOS
+  /// task notifications for wake-up.
   /// NOTE: File descriptors >= FD_SETSIZE (typically 10 on ESP) will be rejected with an error.
+  /// WARNING: These functions are NOT thread-safe. They must only be called from the main loop.
   /// @return true if registration was successful, false if fd exceeds limits
   bool register_socket_fd(int fd);
   void unregister_socket_fd(int fd);
@@ -488,9 +485,7 @@ class Application {
   //   and active_end_ is incremented
   // - This eliminates branch mispredictions from flag checking in the hot loop
   FixedVector<Component *> looping_components_{};
-#ifdef USE_LWIP_FAST_SELECT
-  std::vector<struct lwip_sock *> monitored_sockets_;  // Cached lwip_sock pointers for direct rcvevent read
-#elif defined(USE_HOST)
+#ifdef USE_HOST
   std::vector<int> socket_fds_;  // Vector of all monitored socket file descriptors
 #endif
 #ifdef USE_HOST
@@ -705,26 +700,16 @@ inline void ESPHOME_ALWAYS_INLINE Application::loop() {
 #ifndef USE_HOST
 inline void ESPHOME_ALWAYS_INLINE Application::yield_with_select_(uint32_t delay_ms) {
 #ifdef USE_LWIP_FAST_SELECT
-  // Fast path (ESP32/LibreTiny): reads rcvevent directly from cached lwip_sock pointers.
-  // Safe because this runs on the main loop which owns socket lifetime (create, read, close).
+  // Fast path (ESP32/LibreTiny): FreeRTOS task notifications posted by the lwip
+  // event_callback wrapper (see lwip_fast_select.c) are the single source of truth for
+  // socket wake-ups. Every NETCONN_EVT_RCVPLUS posts an xTaskNotifyGive, so any notification
+  // that lands between wakes keeps the counter non-zero (next ulTaskNotifyTake returns
+  // immediately) or wakes a blocked Take directly. Additional wake sources:
+  // wake_loop_threadsafe() from background tasks, and the delay_ms timeout.
   if (delay_ms == 0) [[unlikely]] {
     yield();
     return;
   }
-
-  // Check if any socket already has pending data before sleeping.
-  // If a socket still has unread data (rcvevent > 0) but the task notification was already
-  // consumed, ulTaskNotifyTake would block until timeout — adding up to delay_ms latency.
-  // This scan preserves select() semantics: return immediately when any fd is ready.
-  for (struct lwip_sock *sock : this->monitored_sockets_) {
-    if (esphome_lwip_socket_has_data(sock)) {
-      yield();
-      return;
-    }
-  }
-
-  // Sleep with instant wake via FreeRTOS task notification.
-  // Woken by: callback wrapper (socket data), wake_loop_threadsafe() (background tasks), or timeout.
 #endif
   esphome::internal::wakeable_delay(delay_ms);
 }