Restore the cached_sock_ = nullptr assignment inside close() on the
fast-select path. The lwip slot can be recycled for a new connection
as soon as the underlying close() returns, so any dereference of
cached_sock_ afterwards would touch an unrelated socket's pcb.
No current caller does this — setsockopt(TCP_NODELAY) and ready() are
the only consumers and neither is invoked post-close today — but
leaving the pointer dangling is a footgun for future changes. The
fd_ = -1 sentinel alone would catch the ready() path via closed
semantics, but setsockopt() reaches cached_sock_ directly and would
not be protected. Null the pointer so the protection is by
construction rather than by caller discipline.
Replace the separate closed_ bool with fd_ < 0 as the 'not open'
sentinel. close() now sets fd_ = -1 after the underlying close call,
so the destructor and double-close paths just check fd_ < 0. As a
side benefit, get_fd() on a closed socket now returns -1, making
use-after-close visible to callers instead of returning a stale
descriptor.
Drop loop_monitored_ on the USE_LWIP_FAST_SELECT path — the pointer
cached_sock_ already encodes monitoring state (non-null iff
monitored). On USE_HOST the bool is still needed because there is no
cached pointer to derive from.
Combined effect on the fast-select path:
Before: fd_(4) + cached_sock_(4) + closed_(1) + loop_monitored_(1)
+ pad(2) = 12 bytes per socket
After: fd_(4) + cached_sock_(4)
= 8 bytes per socket (aligned, no tail padding)
Saves 4 bytes per Socket instance on ESP32/LibreTiny. With typical
workloads running 5-10 sockets (API listen + clients + mDNS) that's
20-40 bytes of RAM.
Lift the lwip_sock resolve + event-callback hook sequence into
socket::fast_select_hook_fd() in socket.h so the USE_LWIP_FAST_SELECT
constructor blocks in lwip_sockets_impl.cpp and bsd_sockets_impl.cpp
stop drifting in lockstep. Both impls now collapse to a two-line call
site.
- Drop redundant cached_sock_ = nullptr assignment in close(). After
closed_ = true the socket is a corpse and no ready() or other member
access is valid, so the nulling is not load-bearing. The comment now
explains why on both impl variants.
- Reword the yield_with_select_ comment so the wake-source sentence
reads as a complete list rather than a trailing fragment.
The pre-sleep scan in Application::yield_with_select_() walks
monitored_sockets_ on every loop iteration, issuing a volatile
cross-thread read on each socket's lwip_sock::rcvevent to preserve
select() semantics when the FreeRTOS task notification counter had
been consumed but a socket still had unread data.
That scenario only existed because of a Socket::ready() contract
violation: callers could stop reading with rcvevent > 0, leaving
data behind with no pending notification. That contract is now
documented and enforced (#15590), and #15589 (the first failure
that reverted the earlier removal attempt #14475) has been fixed.
With the contract honoured, every rcvevent > 0 is paired with a
pending xTaskNotifyGive from the lwip event_callback wrapper (see
lwip_fast_select.c). ulTaskNotifyTake either returns immediately
(counter non-zero) or wakes the moment the notify lands — the scan
has nothing left to rescue.
Evidence: https://github.com/esphome/esphome/pull/15638 — an
instrumentation PR ran across 5 devices (ESP32 rev1/rev3.1/C3 on
Ethernet and WiFi, plus LibreTiny RTL8720CF) through Home Assistant
disconnect/reconnect cycles, multi-client API logger bursts, and
BLE GATT connect storms. Across ~275,000 scans and 4 observed
load-bearing candidates, every hit was in the 2–14µs range — the
instruction-level window between the lwip callback writing
rcvevent and calling xTaskNotifyGive a few instructions later.
Zero hits exceeded 100µs. No hit came anywhere near loop_interval
(16ms), which is the latency scale the scan was added to prevent.
In addition to being unused, the scan is actively harmful on the
hot path: N volatile 16-bit loads against cache-cold cross-thread
lwip_sock structures on every main-loop iteration, just to
reproduce a microsecond-scale ordering artifact the notification
path is already handling authoritatively.
This also removes the now-unused monitored_sockets_ vector and
Application::{register,unregister}_socket() on the fast-select
path. Socket implementations now call esphome_lwip_hook_socket()
directly to install the netconn event callback wrapper.
* Add push to talk voice assistant
* Refactor most code into voice_assistant
* Make voice_assistant the component and remove push_to_talk (can be done in yaml)
* Fix component setup
* Always AF_INET to match serverside
* Fix microphone and media player co-existence
* Format
* Update codeowners
* Update test file
* Fix endifs
* nullptr not NULL
* clang-tidy
* Format
* fixup: Add VA event data
* Generate proto
* Parse and log events
* Add default to switch
* Fix
* Add mic/va to test5
J. Nick Koston