From dd07fba943f27a9105ede153217bb62d4910d58c Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 9 Apr 2026 05:48:18 -1000 Subject: [PATCH] [socket] Document ready() contract: callers must drain or track (#15590) --- esphome/components/api/api_frame_helper.h | 5 ++++- esphome/components/socket/bsd_sockets_impl.h | 2 ++ esphome/components/socket/lwip_raw_tcp_impl.h | 2 ++ esphome/components/socket/lwip_sockets_impl.h | 2 ++ esphome/components/socket/socket.h | 13 +++++++++++++ 5 files changed, 23 insertions(+), 1 deletion(-) diff --git a/esphome/components/api/api_frame_helper.h b/esphome/components/api/api_frame_helper.h index d1215388d2..f98eca8076 100644 --- a/esphome/components/api/api_frame_helper.h +++ b/esphome/components/api/api_frame_helper.h @@ -195,7 +195,10 @@ class APIFrameHelper { } // Get the frame footer size required by this protocol uint8_t frame_footer_size() const { return frame_footer_size_; } - // Check if socket has data ready to read + // Check if socket has buffered data ready to read. + // Contract: callers must read until it would block (EAGAIN/EWOULDBLOCK) + // or track that they stopped early and retry without this check. + // See Socket::ready() for details. bool is_socket_ready() const { return socket_ != nullptr && socket_->ready(); } // Release excess memory from internal buffers after initial sync void release_buffers() { diff --git a/esphome/components/socket/bsd_sockets_impl.h b/esphome/components/socket/bsd_sockets_impl.h index 339a699bc9..e520784702 100644 --- a/esphome/components/socket/bsd_sockets_impl.h +++ b/esphome/components/socket/bsd_sockets_impl.h @@ -112,6 +112,8 @@ class BSDSocketImpl { int setblocking(bool blocking); int loop() { return 0; } + /// Check if the socket has buffered data ready to read. + /// See the ready() contract in socket.h — callers must drain or track remaining data. bool ready() const; int get_fd() const { return this->fd_; } diff --git a/esphome/components/socket/lwip_raw_tcp_impl.h b/esphome/components/socket/lwip_raw_tcp_impl.h index e2dcb80d32..917b5b2f7a 100644 --- a/esphome/components/socket/lwip_raw_tcp_impl.h +++ b/esphome/components/socket/lwip_raw_tcp_impl.h @@ -96,6 +96,8 @@ class LWIPRawImpl : public LWIPRawCommon { errno = ENOSYS; return -1; } + // Check if the socket has buffered data ready to read. + // See the ready() contract in socket.h — callers must drain or track remaining data. // Intentionally unlocked — this is a polling check called every loop iteration. // A stale read at worst delays processing by one loop tick; the actual I/O in // read() holds the lwip lock and re-checks properly. See esphome#10681. diff --git a/esphome/components/socket/lwip_sockets_impl.h b/esphome/components/socket/lwip_sockets_impl.h index bfc4da9926..942d0ccf85 100644 --- a/esphome/components/socket/lwip_sockets_impl.h +++ b/esphome/components/socket/lwip_sockets_impl.h @@ -78,6 +78,8 @@ class LwIPSocketImpl { int setblocking(bool blocking); int loop() { return 0; } + /// Check if the socket has buffered data ready to read. + /// See the ready() contract in socket.h — callers must drain or track remaining data. bool ready() const; int get_fd() const { return this->fd_; } diff --git a/esphome/components/socket/socket.h b/esphome/components/socket/socket.h index 9ea71321e0..ad55e889e8 100644 --- a/esphome/components/socket/socket.h +++ b/esphome/components/socket/socket.h @@ -53,6 +53,19 @@ bool socket_ready_fd(int fd, bool loop_monitored); // Inline ready() — defined here because it depends on socket_ready/socket_ready_fd // declared above, while the impl headers are included before those declarations. +// +// Contract (applies to ALL socket implementations — each platform implements +// ready() differently, but this contract holds regardless of the mechanism): +// ready() checks if the socket has buffered data ready to read. When it returns +// true, the caller MUST read until it would block (EAGAIN/EWOULDBLOCK), or until +// read() returns 0 to indicate EOF / connection closed, or track that it stopped +// early and retry without calling ready(). The next call to ready() will only +// report new data correctly if all callers fulfill this contract. Failing to +// drain the socket may cause ready() to return false while data remains readable. +// +// In practice each socket is owned by a single component, so this contract is +// straightforward to fulfill — but the owning component must be aware of it, +// especially if it limits how many messages it processes per loop iteration. #if defined(USE_SOCKET_IMPL_BSD_SOCKETS) || defined(USE_SOCKET_IMPL_LWIP_SOCKETS) inline bool Socket::ready() const { #ifdef USE_LWIP_FAST_SELECT