fix: missing logs & mutex peer locks

eclipse-zenoh · Mar 4, 2025 · f3c2799 · f3c2799
1 parent 415ea04
commit f3c2799
Show file tree

Hide file tree

Showing 4 changed files with 17 additions and 7 deletions.
diff --git a/src/session/resource.c b/src/session/resource.c
@@ -223,6 +223,8 @@ uint16_t _z_register_resource(_z_session_t *zn, const _z_keyexpr_t *key, uint16_
     uint16_t mapping = register_to_mapping;
     uint16_t parent_mapping = _z_keyexpr_mapping_id(key);
     _z_keyexpr_t full_ke = _z_keyexpr_alias(key);
+    _Z_DEBUG("registering: key: %.*s id: %d, mapping: %d", (int)_z_string_len(&key->_suffix),
+             _z_string_data(&key->_suffix), id, mapping);
 
     _z_session_mutex_lock(zn);
 
@@ -267,7 +269,7 @@ void _z_unregister_resource(_z_session_t *zn, uint16_t id, uint16_t mapping) {
         _z_resource_list_t *parent = *parent_mut;
         while (parent != NULL) {
             _z_resource_t *head = _z_resource_list_head(parent);
-            if (head && head->_id == id && _z_keyexpr_mapping_id(&head->_key) == mapping) {
+            if ((head != NULL) && (head->_id == id) && (_z_keyexpr_mapping_id(&head->_key) == mapping)) {
                 head->_refcount--;
                 if (head->_refcount == 0) {
                     *parent_mut = _z_resource_list_pop(parent, &head);

diff --git a/src/transport/unicast/accept.c b/src/transport/unicast/accept.c
@@ -28,7 +28,10 @@ static void *_zp_unicast_accept_task(void *ctx) {
     _z_sys_net_socket_t con_socket = {0};
 
     while (true) {
-        if (_z_transport_unicast_peer_list_len(ztu->_peers) < Z_LISTEN_MAX_CONNECTION_NB) {
+        _z_transport_peer_mutex_lock(&ztu->_common);
+        size_t peer_len = _z_transport_unicast_peer_list_len(ztu->_peers);
+        _z_transport_peer_mutex_unlock(&ztu->_common);
+        if (peer_len < Z_LISTEN_MAX_CONNECTION_NB) {
             // Accept connection
             if (_z_socket_accept(&listen_socket, &con_socket) != _Z_RES_OK) {
                 _Z_INFO("Connection accept failed");
@@ -44,7 +47,7 @@ static void *_zp_unicast_accept_task(void *ctx) {
                 _z_socket_close(&con_socket);
                 continue;
             }
-            // Set socket as non blocking (FIXME: activate when read tasks reworked)
+            // Set socket as non blocking
             if (_z_socket_set_non_blocking(&con_socket) != _Z_RES_OK) {
                 _Z_INFO("Failed to set socket non blocking");
                 _z_socket_close(&con_socket);

diff --git a/src/transport/unicast/read.c b/src/transport/unicast/read.c
@@ -128,17 +128,20 @@ static int _z_unicast_peer_read(_z_transport_unicast_t *ztu, _z_transport_unicas
         case Z_LINK_CAP_FLOW_STREAM:
             read_size = _z_link_socket_recv_zbuf(&ztu->_common._link, &ztu->_common._zbuf, peer->_socket);
             if (read_size == 0) {
+                _Z_DEBUG("Socket closed");
                 return -2;
             } else if (read_size == SIZE_MAX) {
                 return -1;
             }
             if (_z_zbuf_len(&ztu->_common._zbuf) < _Z_MSG_LEN_ENC_SIZE) {
+                _Z_DEBUG("Not enough bytes to read stream size");
                 return -1;
             }
             // Get stream size
             *to_read = _z_read_stream_size(&ztu->_common._zbuf);
             // Read data if needed
             if (_z_zbuf_len(&ztu->_common._zbuf) < *to_read) {
+                _Z_DEBUG("Not enough bytes to read packet");
                 return -1;
             }
             break;
@@ -181,13 +184,12 @@ void *_zp_unicast_read_task(void *ztu_arg) {
                 z_sleep_s(1);
                 continue;
             }
-            // Wait for events on sockets
+            // Wait for events on sockets (need mutex)
             if (_z_socket_wait_event(ztu->_peers) != _Z_RES_OK) {
-                _Z_ERROR("Wait event failed");
-                ztu->_common._read_task_running = false;
-                continue;
+                continue;  // Might need to process errors other than timeout
             }
             // Process events
+            _z_transport_peer_mutex_lock(&ztu->_common);
             _z_transport_unicast_peer_list_t *curr_list = ztu->_peers;
             _z_transport_unicast_peer_list_t *to_drop = NULL;
             _z_transport_unicast_peer_list_t *prev = NULL;
@@ -211,9 +213,11 @@ void *_zp_unicast_read_task(void *ztu_arg) {
                 prev = curr_list;
                 curr_list = _z_transport_unicast_peer_list_tail(curr_list);
                 if (drop_peer) {
+                    _Z_DEBUG("Dropping peer");
                     ztu->_peers = _z_transport_unicast_peer_list_drop_element(ztu->_peers, to_drop, prev);
                 }
             }
+            _z_transport_peer_mutex_unlock(&ztu->_common);
         } else {
             // Retrieve data
             if (!_z_unicast_client_read(ztu, curr_peer, &to_read)) {

diff --git a/src/transport/unicast/rx.c b/src/transport/unicast/rx.c
@@ -293,6 +293,7 @@ z_result_t _z_unicast_handle_transport_message(_z_transport_unicast_t *ztu, _z_t
 
         case _Z_MID_T_CLOSE: {
             _Z_INFO("Closing session as requested by the remote peer");
+            // Peer will be dropped thanks to the error
             ret = _Z_ERR_CONNECTION_CLOSED;
             _z_t_msg_close_clear(&t_msg->_body._close);
             break;