From 96f59a11acd38565879033bd875da4676e332cfa Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 11 Mar 2026 21:19:51 -1000 Subject: [PATCH] [socket] Release LWIP lock during SO_RCVTIMEO wait and handle spurious wakes - Extract read_locked_() to avoid holding LWIP_LOCK during socket_delay(), which would block recv_fn() on RP2040 (needs async_context lock) - Loop around socket_delay() for remaining time on spurious wakes from other sockets, ensuring SO_RCVTIMEO semantics are correct - Fix readv() to use read_locked_() directly instead of calling read(), avoiding recursive locking and unintended socket_delay() waits --- .../components/socket/lwip_raw_tcp_impl.cpp | 61 ++++++++++++------- esphome/components/socket/lwip_raw_tcp_impl.h | 1 + 2 files changed, 41 insertions(+), 21 deletions(-) diff --git a/esphome/components/socket/lwip_raw_tcp_impl.cpp b/esphome/components/socket/lwip_raw_tcp_impl.cpp index b64e1ecdbf9..25dddbab9d4 100644 --- a/esphome/components/socket/lwip_raw_tcp_impl.cpp +++ b/esphome/components/socket/lwip_raw_tcp_impl.cpp @@ -516,36 +516,42 @@ err_t LWIPRawImpl::recv_fn(struct pbuf *pb, err_t err) { } ssize_t LWIPRawImpl::read(void *buf, size_t len) { + // If SO_RCVTIMEO is set and no data available, wait without holding lock. + // These reads are safe unlocked (atomic pointer/bool on ARM/Xtensa) — + // they're just hints; the authoritative check happens under LWIP_LOCK below. + // Lock must not be held during socket_delay() so recv_fn() can run on RP2040. + if (this->recv_timeout_cs_ > 0 && this->rx_buf_ == nullptr && !this->rx_closed_ && this->pcb_ != nullptr) { + // Loop until data arrives, connection closes, or the full timeout elapses. + // socket_delay() may return early due to other sockets waking the global + // socket_wake() flag, so we re-enter for the remaining time. + uint32_t timeout_ms = this->recv_timeout_cs_ * 10; + uint32_t start = millis(); + while (this->rx_buf_ == nullptr && !this->rx_closed_ && this->pcb_ != nullptr) { + uint32_t elapsed = millis() - start; + if (elapsed >= timeout_ms) + break; + socket_delay(timeout_ms - elapsed); + } + } + LWIP_LOCK(); if (this->pcb_ == nullptr) { errno = ECONNRESET; return -1; } - if (this->rx_closed_ && this->rx_buf_ == nullptr) { + if (this->rx_closed_ && this->rx_buf_ == nullptr) return 0; - } - if (len == 0) { + if (len == 0) return 0; - } if (this->rx_buf_ == nullptr) { - if (this->recv_timeout_cs_ > 0) { - // Wait efficiently for data — socket_delay() sleeps and wakes - // immediately when recv_fn() fires (data arrives via socket_wake()) - socket_delay(this->recv_timeout_cs_ * 10); - // Recheck after waking — data or close may have arrived - if (this->rx_closed_ && this->rx_buf_ == nullptr) - return 0; - if (this->rx_buf_ == nullptr) { - errno = EWOULDBLOCK; - return -1; - } - // Data arrived, fall through to copy - } else { - errno = EWOULDBLOCK; - return -1; - } + errno = EWOULDBLOCK; + return -1; } + return this->read_locked_(buf, len); +} +ssize_t LWIPRawImpl::read_locked_(void *buf, size_t len) { + // Caller must hold LWIP_LOCK and ensure rx_buf_ != nullptr size_t read = 0; uint8_t *buf8 = reinterpret_cast(buf); while (len && this->rx_buf_ != nullptr) { @@ -591,9 +597,22 @@ ssize_t LWIPRawImpl::read(void *buf, size_t len) { ssize_t LWIPRawImpl::readv(const struct iovec *iov, int iovcnt) { LWIP_LOCK(); // Hold for entire scatter-gather operation + if (this->pcb_ == nullptr) { + errno = ECONNRESET; + return -1; + } + if (this->rx_closed_ && this->rx_buf_ == nullptr) { + return 0; + } ssize_t ret = 0; for (int i = 0; i < iovcnt; i++) { - ssize_t err = this->read(reinterpret_cast(iov[i].iov_base), iov[i].iov_len); + if (this->rx_buf_ == nullptr) { + if (ret != 0) + break; + errno = EWOULDBLOCK; + return -1; + } + ssize_t err = this->read_locked_(reinterpret_cast(iov[i].iov_base), iov[i].iov_len); if (err == -1) { if (ret != 0) { // if we already read some don't return an error diff --git a/esphome/components/socket/lwip_raw_tcp_impl.h b/esphome/components/socket/lwip_raw_tcp_impl.h index 50009236ded..6e27049a7ba 100644 --- a/esphome/components/socket/lwip_raw_tcp_impl.h +++ b/esphome/components/socket/lwip_raw_tcp_impl.h @@ -120,6 +120,7 @@ class LWIPRawImpl : public LWIPRawCommon { static err_t s_recv_fn(void *arg, struct tcp_pcb *pcb, struct pbuf *pb, err_t err); protected: + ssize_t read_locked_(void *buf, size_t len); ssize_t internal_write_(const void *buf, size_t len); int internal_output_();