Skip to content

Long timeouts, not topic, session breaks, with fix #23

Description

@chat-l18l

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.c passes Z_CONFIG_SOCKET_TIMEOUT into 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.c discards 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 a recv() 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_zenohd router, 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_ALIVE

Thirty 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 60000ms even 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_zbuf leaves the buffer position untouched,
and _z_unicast_client_read reports "nothing to read" — the ordinary
non-fatal path, which is what Z_CONFIG_SOCKET_TIMEOUT exists to produce.

The change

_z_tcp_esp32_open now honours its tout parameter, and
_z_tcp_esp32_accept sets Z_CONFIG_SOCKET_TIMEOUT the way tcp_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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions