From 2fa244715d21e30338702c0ea017884f338efd03 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 2 Mar 2026 08:54:54 -1000 Subject: [PATCH 1/2] [socket] Fix pre-existing bugs found during socket devirtualization review (#14404) --- esphome/components/socket/bsd_sockets_impl.h | 2 +- .../components/socket/lwip_raw_tcp_impl.cpp | 21 +++++++++++++++++-- esphome/components/socket/lwip_sockets_impl.h | 2 +- 3 files changed, 21 insertions(+), 4 deletions(-) diff --git a/esphome/components/socket/bsd_sockets_impl.h b/esphome/components/socket/bsd_sockets_impl.h index edee39f315..d9ed9dc567 100644 --- a/esphome/components/socket/bsd_sockets_impl.h +++ b/esphome/components/socket/bsd_sockets_impl.h @@ -83,7 +83,7 @@ class BSDSocketImpl { return ::write(this->fd_, buf, len); #endif } - ssize_t send(void *buf, size_t len, int flags) { return ::send(this->fd_, buf, len, flags); } + ssize_t send(const void *buf, size_t len, int flags) { return ::send(this->fd_, buf, len, flags); } ssize_t writev(const struct iovec *iov, int iovcnt) { #if defined(USE_ESP32) return ::lwip_writev(this->fd_, iov, iovcnt); diff --git a/esphome/components/socket/lwip_raw_tcp_impl.cpp b/esphome/components/socket/lwip_raw_tcp_impl.cpp index 6556979fe0..430356592f 100644 --- a/esphome/components/socket/lwip_raw_tcp_impl.cpp +++ b/esphome/components/socket/lwip_raw_tcp_impl.cpp @@ -68,7 +68,7 @@ int LWIPRawCommon::bind(const struct sockaddr *name, socklen_t addrlen) { } if (name == nullptr) { errno = EINVAL; - return 0; + return -1; } ip_addr_t ip; in_port_t port; @@ -319,6 +319,13 @@ int LWIPRawCommon::ip2sockaddr_(ip_addr_t *ip, uint16_t port, struct sockaddr *n // ---- LWIPRawImpl methods ---- LWIPRawImpl::~LWIPRawImpl() { + // Free any received pbufs that LWIP transferred ownership of via recv_fn. + // tcp_abort() in the base destructor won't free these since LWIP considers + // ownership transferred once the recv callback accepts them. + if (this->rx_buf_ != nullptr) { + pbuf_free(this->rx_buf_); + this->rx_buf_ = nullptr; + } // Base class destructor handles pcb_ cleanup via tcp_abort } @@ -545,7 +552,14 @@ ssize_t LWIPRawImpl::writev(const struct iovec *iov, int iovcnt) { // ---- LWIPRawListenImpl methods ---- LWIPRawListenImpl::~LWIPRawListenImpl() { - // Base class destructor handles pcb_ cleanup via tcp_abort + // Listen PCBs must use tcp_close(), not tcp_abort(). + // tcp_abandon() asserts pcb->state != LISTEN and would access + // fields that don't exist in the smaller tcp_pcb_listen struct. + // Close here and null pcb_ so the base destructor skips tcp_abort. + if (this->pcb_ != nullptr) { + tcp_close(this->pcb_); + this->pcb_ = nullptr; + } } void LWIPRawListenImpl::init() { @@ -609,6 +623,9 @@ int LWIPRawListenImpl::listen(int backlog) { LWIP_LOG("tcp_arg(%p)", this->pcb_); tcp_arg(this->pcb_, this); tcp_accept(this->pcb_, LWIPRawListenImpl::s_accept_fn); + // Note: tcp_err() is NOT re-registered here. tcp_listen_with_backlog() converts the + // full tcp_pcb to a smaller tcp_pcb_listen struct that lacks the errf field. + // Calling tcp_err() on a listen PCB writes past the struct boundary (undefined behavior). return 0; } diff --git a/esphome/components/socket/lwip_sockets_impl.h b/esphome/components/socket/lwip_sockets_impl.h index 2e319fcc4d..d6699aded2 100644 --- a/esphome/components/socket/lwip_sockets_impl.h +++ b/esphome/components/socket/lwip_sockets_impl.h @@ -57,7 +57,7 @@ class LwIPSocketImpl { } ssize_t readv(const struct iovec *iov, int iovcnt) { return lwip_readv(this->fd_, iov, iovcnt); } ssize_t write(const void *buf, size_t len) { return lwip_write(this->fd_, buf, len); } - ssize_t send(void *buf, size_t len, int flags) { return lwip_send(this->fd_, buf, len, flags); } + ssize_t send(const void *buf, size_t len, int flags) { return lwip_send(this->fd_, buf, len, flags); } ssize_t writev(const struct iovec *iov, int iovcnt) { return lwip_writev(this->fd_, iov, iovcnt); } ssize_t sendto(const void *buf, size_t len, int flags, const struct sockaddr *to, socklen_t tolen) { return lwip_sendto(this->fd_, buf, len, flags, to, tolen); From cb232d828879b365bfa116ff3b4a7546c1e6ce62 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 2 Mar 2026 09:11:47 -1000 Subject: [PATCH 2/2] [core] Fix compile-time loop() detection for multiple inheritance (#14411) --- esphome/core/application.h | 13 ++++++++++--- esphome/core/config.py | 3 ++- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/esphome/core/application.h b/esphome/core/application.h index 44e8de7ee9..13fd0180ab 100644 --- a/esphome/core/application.h +++ b/esphome/core/application.h @@ -118,6 +118,14 @@ void original_setup(); // NOLINT(readability-redundant-declaration) - used by c namespace esphome { +/// SFINAE helper: detects whether T overrides Component::loop(). +/// When &T::loop is ambiguous (multiple inheritance with separate loop() methods), +/// the ambiguity itself proves an override exists, so the true_type default is correct. +template struct HasLoopOverride : std::true_type {}; +template +struct HasLoopOverride> + : std::bool_constant> {}; + // Teardown timeout constant (in milliseconds) // For reboots, it's more important to shut down quickly than disconnect cleanly // since we're not entering deep sleep. The only consequence of not shutting down @@ -544,10 +552,9 @@ class Application { #endif /// Register a component, detecting loop() override at compile time. - /// The template resolves &T::loop vs &Component::loop as a constexpr bool - /// and forwards it to register_component_impl_ which stores it in component_state_. + /// Uses HasLoopOverride which handles ambiguous &T::loop from multiple inheritance. template void register_component_(T *comp) { - this->register_component_impl_(comp, !std::is_same_v); + this->register_component_impl_(comp, HasLoopOverride::value); } void register_component_impl_(Component *comp, bool has_loop); diff --git a/esphome/core/config.py b/esphome/core/config.py index 3835fd3875..9411949bb9 100644 --- a/esphome/core/config.py +++ b/esphome/core/config.py @@ -517,9 +517,10 @@ async def _add_looping_components() -> None: return # Build constexpr sum for the exact count, deduplicating by type + # Uses HasLoopOverride which handles ambiguous &T::loop from multiple inheritance type_counts = Counter(entries) terms = [ - f"({count} * !std::is_same_v)" + f"({count} * HasLoopOverride<{cpp_type}>::value)" for cpp_type, count in type_counts.items() ] constexpr_expr = " + \\\n ".join(terms)