Repository navigation
Long timeouts, not topic, session breaks, with fix #23
Description
Suggested PR title
fix(espidf): apply Z_CONFIG_SOCKET_TIMEOUT in the ESP-IDF TCP port
Suggested PR body
The ESP-IDF TCP link never applies the receive timeout it is given, so
recv()blocks indefinitely.
src/link/unicast/tcp.cpassesZ_CONFIG_SOCKET_TIMEOUTinto the platform
open function, and every other port applies it.tcp_lwip.c:tv.tv_sec = tout / (uint32_t)1000; tv.tv_usec = (tout % (uint32_t)1000) * (uint32_t)1000; setsockopt(..., SOL_SOCKET, SO_RCVTIMEO, (char *)&tv, sizeof(tv));
tcp_esp32.cdiscards it on the first line of the function:static z_result_t _z_tcp_esp32_open(..., uint32_t tout) { _ZP_UNUSED(tout);Why it matters beyond the read itself
Since 1.9.0 the read task runs as a future on a shared executor. A task parked
in arecv()that never returns holds that executor, so everything queued
behind it is delayed by however long the peer stays silent.Observed on an ESP32-P4 talking to a
rmw_zenohdrouter, with
ZENOH_DEBUG=3:02:36:11 _z_open_connect_locator Successfully opened connect locator [0] 02:36:11 _z_keep_alive_encode Encoding _Z_MID_T_KEEP_ALIVE 02:36:41 _z_keep_alive_decode Decoding _Z_MID_T_KEEP_ALIVEThirty seconds between a completed handshake and a usable session, spent
waiting for the peer's first byte because nothing else could run. With an idle
router that byte is its keep-alive, at lease/2.The same blockage starved the lease task, which both sends keep-alives and
performs the expiry check, so sessions were torn down once a minute with
Closing session because it has expired after 60000mseven though the router
was answering promptly.Why a timeout is safe here
By the library's existing design rather than by luck. A short read returns
SIZE_MAX,_z_link_socket_recv_zbufleaves the buffer position untouched,
and_z_unicast_client_readreports "nothing to read" — the ordinary
non-fatal path, which is whatZ_CONFIG_SOCKET_TIMEOUTexists to produce.The change
_z_tcp_esp32_opennow honours itstoutparameter, and
_z_tcp_esp32_acceptsetsZ_CONFIG_SOCKET_TIMEOUTthe waytcp_lwip.c's
accept path does. Both follow the ordering and error handling already used in
the surrounding code.Testing
ESP-IDF 5.5.5 on ESP32-P4, against ROS 2 Jazzy with
rmw_zenoh. With the
patch the session becomes usable immediately after the handshake, and the
minute-by-minute session drops stop. Verified against a build with the fix
--- a/src/link/transport/tcp/tcp_esp32.c
+++ b/src/link/transport/tcp/tcp_esp32.c
@@ -54,11 +54,19 @@
}
static z_result_t _z_tcp_esp32_open(_z_sys_net_socket_t *sock, const _z_sys_net_endpoint_t endpoint, uint32_t tout) {
-
_ZP_UNUSED(tout);
z_result_t ret = _Z_RES_OK;sock->_fd = socket(endpoint._iptcp->ai_family, endpoint._iptcp->ai_socktype, endpoint._iptcp->ai_protocol);
if (sock->_fd != -1) {
-
z_time_t tv; -
tv.tv_sec = tout / (uint32_t)1000; -
tv.tv_usec = (tout % (uint32_t)1000) * (uint32_t)1000; -
if ((ret == _Z_RES_OK) && -
(setsockopt(sock->_fd, SOL_SOCKET, SO_RCVTIMEO, (char *)&tv, sizeof(tv)) < 0)) { -
_Z_ERROR_LOG(_Z_ERR_GENERIC); -
ret = _Z_ERR_GENERIC; -
} -
int optflag = 1; if ((ret == _Z_RES_OK) && (setsockopt(sock->_fd, SOL_SOCKET, SO_KEEPALIVE, (void *)&optflag, sizeof(optflag)) < 0)) {
@@ -165,6 +173,14 @@
_Z_ERROR_RETURN(_Z_ERR_GENERIC);
}
- z_time_t tv;
- tv.tv_sec = Z_CONFIG_SOCKET_TIMEOUT / (uint32_t)1000;
- tv.tv_usec = (Z_CONFIG_SOCKET_TIMEOUT % (uint32_t)1000) * (uint32_t)1000;
- if (setsockopt(con_socket, SOL_SOCKET, SO_RCVTIMEO, (char *)&tv, sizeof(tv)) < 0) {
-
close(con_socket); -
_Z_ERROR_RETURN(_Z_ERR_GENERIC); - }
- int optflag = 1;
if (setsockopt(con_socket, SOL_SOCKET, SO_KEEPALIVE, (void *)&optflag, sizeof(optflag)) < 0) {
close(con_socket);
applied and no other workaround in place.
Activity