From ab422809f562620e3b21c9736f33a47f62176ba9 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 11 Mar 2026 21:25:20 -1000 Subject: [PATCH] [socket] Simplify SO_RCVTIMEO: extract wait_for_data_() for read/readv Replace read_locked_() approach with a simpler wait_for_data_() called at the top of both read() and readv(), keeping the original read/readv structure intact. --- .../components/socket/lwip_raw_tcp_impl.cpp | 64 +++++++++---------- esphome/components/socket/lwip_raw_tcp_impl.h | 2 +- 2 files changed, 30 insertions(+), 36 deletions(-) diff --git a/esphome/components/socket/lwip_raw_tcp_impl.cpp b/esphome/components/socket/lwip_raw_tcp_impl.cpp index 25dddbab9d4..91be20ffb6d 100644 --- a/esphome/components/socket/lwip_raw_tcp_impl.cpp +++ b/esphome/components/socket/lwip_raw_tcp_impl.cpp @@ -515,23 +515,28 @@ err_t LWIPRawImpl::recv_fn(struct pbuf *pb, err_t err) { return ERR_OK; } +void LWIPRawImpl::wait_for_data_() { + // Wait for data without holding LWIP_LOCK so recv_fn() can run on RP2040 + // (needs async_context lock). Unlocked reads of rx_buf_/rx_closed_/pcb_ are + // safe (atomic pointer/bool on ARM/Xtensa) — they're just hints to avoid + // unnecessary sleeping; the authoritative check happens under LWIP_LOCK + // in the caller after this returns. + // 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); + } +} + 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); - } + this->wait_for_data_(); } LWIP_LOCK(); @@ -539,19 +544,17 @@ ssize_t LWIPRawImpl::read(void *buf, size_t len) { 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) { 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) { @@ -596,23 +599,14 @@ ssize_t LWIPRawImpl::read_locked_(void *buf, size_t len) { } ssize_t LWIPRawImpl::readv(const struct iovec *iov, int iovcnt) { + if (this->recv_timeout_cs_ > 0 && this->rx_buf_ == nullptr && !this->rx_closed_ && this->pcb_ != nullptr) { + this->wait_for_data_(); + } + 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++) { - 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); + ssize_t err = this->read(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 6e27049a7ba..ec0b2504b39 100644 --- a/esphome/components/socket/lwip_raw_tcp_impl.h +++ b/esphome/components/socket/lwip_raw_tcp_impl.h @@ -120,7 +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); + void wait_for_data_(); ssize_t internal_write_(const void *buf, size_t len); int internal_output_();