From 4a9d3da962de2512bc0e7373145bd15e28075d04 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 10 Apr 2026 20:58:12 -1000 Subject: [PATCH] [core] fast_select scan removal: address review feedback - Drop redundant cached_sock_ = nullptr assignment in close(). After closed_ = true the socket is a corpse and no ready() or other member access is valid, so the nulling is not load-bearing. The comment now explains why on both impl variants. - Reword the yield_with_select_ comment so the wake-source sentence reads as a complete list rather than a trailing fragment. --- esphome/components/socket/bsd_sockets_impl.cpp | 9 ++++----- esphome/components/socket/lwip_sockets_impl.cpp | 9 ++++----- esphome/core/application.h | 4 ++-- 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/esphome/components/socket/bsd_sockets_impl.cpp b/esphome/components/socket/bsd_sockets_impl.cpp index 1db905daea..2d4e0ea7c2 100644 --- a/esphome/components/socket/bsd_sockets_impl.cpp +++ b/esphome/components/socket/bsd_sockets_impl.cpp @@ -34,11 +34,10 @@ BSDSocketImpl::~BSDSocketImpl() { int BSDSocketImpl::close() { if (!this->closed_) { -#ifdef USE_LWIP_FAST_SELECT - // All LwIP sockets share the same static event_callback, so there is no per-socket - // unhook needed — just drop the cached pointer before the socket is destroyed. - this->cached_sock_ = nullptr; -#else +#ifndef USE_LWIP_FAST_SELECT + // All LwIP sockets share the same static event_callback, so on the fast-select path + // there is no per-socket unhook needed. cached_sock_ is not cleared because closed_ + // makes the socket a corpse — no ready() or other member access is valid afterwards. if (this->loop_monitored_) { App.unregister_socket_fd(this->fd_); } diff --git a/esphome/components/socket/lwip_sockets_impl.cpp b/esphome/components/socket/lwip_sockets_impl.cpp index e628a22d8d..c65d3a227d 100644 --- a/esphome/components/socket/lwip_sockets_impl.cpp +++ b/esphome/components/socket/lwip_sockets_impl.cpp @@ -34,11 +34,10 @@ LwIPSocketImpl::~LwIPSocketImpl() { int LwIPSocketImpl::close() { if (!this->closed_) { -#ifdef USE_LWIP_FAST_SELECT - // All LwIP sockets share the same static event_callback, so there is no per-socket - // unhook needed — just drop the cached pointer before the socket is destroyed. - this->cached_sock_ = nullptr; -#else +#ifndef USE_LWIP_FAST_SELECT + // All LwIP sockets share the same static event_callback, so on the fast-select path + // there is no per-socket unhook needed. cached_sock_ is not cleared because closed_ + // makes the socket a corpse — no ready() or other member access is valid afterwards. if (this->loop_monitored_) { App.unregister_socket_fd(this->fd_); } diff --git a/esphome/core/application.h b/esphome/core/application.h index 122f9b39c7..47976fd57b 100644 --- a/esphome/core/application.h +++ b/esphome/core/application.h @@ -895,8 +895,8 @@ inline void ESPHOME_ALWAYS_INLINE Application::yield_with_select_(uint32_t delay // 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. Also woken by wake_loop_threadsafe() - // from background tasks, or timeout. + // 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;