diff --git a/esphome/components/socket/bsd_sockets_impl.cpp b/esphome/components/socket/bsd_sockets_impl.cpp index d55523ff77..baf030f2b1 100644 --- a/esphome/components/socket/bsd_sockets_impl.cpp +++ b/esphome/components/socket/bsd_sockets_impl.cpp @@ -15,33 +15,28 @@ BSDSocketImpl::BSDSocketImpl(int fd, bool monitor_loop) { return; #ifdef USE_LWIP_FAST_SELECT this->cached_sock_ = fast_select_hook_fd(this->fd_); - this->loop_monitored_ = this->cached_sock_ != nullptr; #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_) { -#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_); - } -#endif - int ret = ::close(this->fd_); - this->closed_ = true; - return ret; + if (this->fd_ < 0) { + // Already closed, or never opened. + return 0; } - return 0; +#ifndef USE_LWIP_FAST_SELECT + // On the fast-select path there is no per-socket unhook needed — all LwIP sockets + // share the same static event_callback. + 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) { diff --git a/esphome/components/socket/bsd_sockets_impl.h b/esphome/components/socket/bsd_sockets_impl.h index e520784702..64b6adf94a 100644 --- a/esphome/components/socket/bsd_sockets_impl.h +++ b/esphome/components/socket/bsd_sockets_impl.h @@ -119,12 +119,17 @@ 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 + // Non-null iff this socket is being monitored for read events. Replaces loop_monitored_ + // on the fast-select path: the pointer itself carries the "monitored" bit. struct lwip_sock *cached_sock_{nullptr}; // Cached for direct rcvevent read in ready() -#endif - bool closed_{false}; +#else bool loop_monitored_{false}; +#endif }; } // namespace esphome::socket diff --git a/esphome/components/socket/lwip_sockets_impl.cpp b/esphome/components/socket/lwip_sockets_impl.cpp index 475381548b..52c94bc7f6 100644 --- a/esphome/components/socket/lwip_sockets_impl.cpp +++ b/esphome/components/socket/lwip_sockets_impl.cpp @@ -15,33 +15,28 @@ LwIPSocketImpl::LwIPSocketImpl(int fd, bool monitor_loop) { return; #ifdef USE_LWIP_FAST_SELECT this->cached_sock_ = fast_select_hook_fd(this->fd_); - this->loop_monitored_ = this->cached_sock_ != nullptr; #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_) { -#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_); - } -#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; +#ifndef USE_LWIP_FAST_SELECT + // On the fast-select path there is no per-socket unhook needed — all LwIP sockets + // share the same static event_callback. + 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) { diff --git a/esphome/components/socket/lwip_sockets_impl.h b/esphome/components/socket/lwip_sockets_impl.h index 942d0ccf85..6e275eb6d1 100644 --- a/esphome/components/socket/lwip_sockets_impl.h +++ b/esphome/components/socket/lwip_sockets_impl.h @@ -85,12 +85,17 @@ 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 + // Non-null iff this socket is being monitored for read events. Replaces loop_monitored_ + // on the fast-select path: the pointer itself carries the "monitored" bit. struct lwip_sock *cached_sock_{nullptr}; // Cached for direct rcvevent read in ready() -#endif - bool closed_{false}; +#else bool loop_monitored_{false}; +#endif }; } // namespace esphome::socket diff --git a/esphome/components/socket/socket.h b/esphome/components/socket/socket.h index c33a5c42b6..739628ef43 100644 --- a/esphome/components/socket/socket.h +++ b/esphome/components/socket/socket.h @@ -42,8 +42,11 @@ 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 hook the netconn event callback so the main loop @@ -80,7 +83,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