fix: memory leak on unix udp multicast (#347)

eclipse-zenoh · Feb 15, 2024 · 16e9eb3 · 16e9eb3
1 parent 9bb6f8c
commit 16e9eb3
Show file tree

Hide file tree

Showing 8 changed files with 20 additions and 18 deletions.
diff --git a/include/zenoh-pico/system/link/udp.h b/include/zenoh-pico/system/link/udp.h
@@ -47,7 +47,7 @@ int8_t _z_open_udp_multicast(_z_sys_net_socket_t *sock, const _z_sys_net_endpoin
 int8_t _z_listen_udp_multicast(_z_sys_net_socket_t *sock, const _z_sys_net_endpoint_t rep, uint32_t tout,
                                const char *iface, const char *join);
 void _z_close_udp_multicast(_z_sys_net_socket_t *sockrecv, _z_sys_net_socket_t *socksend,
-                            const _z_sys_net_endpoint_t rep);
+                            const _z_sys_net_endpoint_t rep, const _z_sys_net_endpoint_t lep);
 size_t _z_read_exact_udp_multicast(const _z_sys_net_socket_t sock, uint8_t *ptr, size_t len,
                                    const _z_sys_net_endpoint_t lep, _z_bytes_t *ep);
 size_t _z_read_udp_multicast(const _z_sys_net_socket_t sock, uint8_t *ptr, size_t len, const _z_sys_net_endpoint_t lep,

diff --git a/src/link/multicast/udp.c b/src/link/multicast/udp.c
@@ -139,7 +139,8 @@ int8_t _z_f_link_listen_udp_multicast(_z_link_t *self) {
 }
 
 void _z_f_link_close_udp_multicast(_z_link_t *self) {
-    _z_close_udp_multicast(&self->_socket._udp._sock, &self->_socket._udp._msock, self->_socket._udp._rep);
+    _z_close_udp_multicast(&self->_socket._udp._sock, &self->_socket._udp._msock, self->_socket._udp._rep,
+                           self->_socket._udp._lep);
 }
 
 void _z_f_link_free_udp_multicast(_z_link_t *self) {

diff --git a/src/system/arduino/esp32/network.cpp b/src/system/arduino/esp32/network.cpp
@@ -447,7 +447,8 @@ int8_t _z_listen_udp_multicast(_z_sys_net_socket_t *sock, const _z_sys_net_endpo
 }
 
 void _z_close_udp_multicast(_z_sys_net_socket_t *sockrecv, _z_sys_net_socket_t *socksend,
-                            const _z_sys_net_endpoint_t rep) {
+                            const _z_sys_net_endpoint_t rep, const _z_sys_net_endpoint_t lep) {
+    _ZP_UNUSED(lep);
     if (rep._iptcp->ai_family == AF_INET) {
         struct ip_mreq mreq;
         (void)memset(&mreq, 0, sizeof(mreq));

diff --git a/src/system/arduino/opencr/network.cpp b/src/system/arduino/opencr/network.cpp
@@ -269,8 +269,9 @@ int8_t _z_listen_udp_multicast(_z_sys_net_socket_t *sock, const _z_sys_net_endpo
 }
 
 void _z_close_udp_multicast(_z_sys_net_socket_t *sockrecv, _z_sys_net_socket_t *socksend,
-                            const _z_sys_net_endpoint_t rep) {
-    (void)(rep);
+                            const _z_sys_net_endpoint_t rep, const _z_sys_net_endpoint_t lep) {
+    _ZP_UNUSED(rep);
+    _ZP_UNUSED(lep);
 
     sockrecv->_udp->stop();
     delete sockrecv->_udp;

diff --git a/src/system/espidf/network.c b/src/system/espidf/network.c
@@ -443,7 +443,8 @@ int8_t _z_listen_udp_multicast(_z_sys_net_socket_t *sock, const _z_sys_net_endpo
 }
 
 void _z_close_udp_multicast(_z_sys_net_socket_t *sockrecv, _z_sys_net_socket_t *socksend,
-                            const _z_sys_net_endpoint_t rep) {
+                            const _z_sys_net_endpoint_t rep, const _z_sys_net_endpoint_t lep) {
+    _ZP_UNUSED(lep);
     if (rep._iptcp->ai_family == AF_INET) {
         struct ip_mreq mreq;
         (void)memset(&mreq, 0, sizeof(mreq));

diff --git a/src/system/mbed/network.cpp b/src/system/mbed/network.cpp
@@ -251,7 +251,8 @@ int8_t _z_listen_udp_multicast(_z_sys_net_socket_t *sock, const _z_sys_net_endpo
 }
 
 void _z_close_udp_multicast(_z_sys_net_socket_t *sockrecv, _z_sys_net_socket_t *socksend,
-                            const _z_sys_net_endpoint_t rep) {
+                            const _z_sys_net_endpoint_t rep, const _z_sys_net_endpoint_t lep) {
+    _ZP_UNUSED(lep);
     sockrecv->_udp->leave_multicast_group(*rep._iptcp);
     sockrecv->_udp->close();
     delete sockrecv->_udp;

diff --git a/src/system/unix/network.c b/src/system/unix/network.c
@@ -347,14 +347,6 @@ int8_t _z_open_udp_multicast(_z_sys_net_socket_t *sock, const _z_sys_net_endpoin
                     laddr->ai_canonname = NULL;
                     laddr->ai_next = NULL;
                     lep->_iptcp = laddr;
-
-                    // This is leaking 16 bytes according to valgrind, but it is a know problem on some libc6
-                    // implementations:
-                    //    https://lists.debian.org/debian-glibc/2016/03/msg00241.html
-                    // To avoid a fix to break zenoh-pico, we are let it leak for the moment.
-                    // #if defined(ZENOH_LINUX)
-                    //    zp_free(lsockaddr);
-                    // #endif
                 } else {
                     ret = _Z_ERR_GENERIC;
                 }
@@ -489,7 +481,7 @@ int8_t _z_listen_udp_multicast(_z_sys_net_socket_t *sock, const _z_sys_net_endpo
 }
 
 void _z_close_udp_multicast(_z_sys_net_socket_t *sockrecv, _z_sys_net_socket_t *socksend,
-                            const _z_sys_net_endpoint_t rep) {
+                            const _z_sys_net_endpoint_t rep, const _z_sys_net_endpoint_t lep) {
     if (rep._iptcp->ai_family == AF_INET) {
         struct ip_mreq mreq;
         (void)memset(&mreq, 0, sizeof(mreq));
@@ -507,7 +499,11 @@ void _z_close_udp_multicast(_z_sys_net_socket_t *sockrecv, _z_sys_net_socket_t *
         // Do nothing. It must never not enter here.
         // Required to be compliant with MISRA 15.7 rule
     }
-
+#if defined(ZENOH_LINUX)
+    zp_free(lep._iptcp->ai_addr);
+#else
+    _ZP_UNUSED(lep);
+#endif
     close(sockrecv->_fd);
     close(socksend->_fd);
 }

diff --git a/src/system/zephyr/network.c b/src/system/zephyr/network.c
@@ -440,7 +440,8 @@ int8_t _z_listen_udp_multicast(_z_sys_net_socket_t *sock, const _z_sys_net_endpo
 }
 
 void _z_close_udp_multicast(_z_sys_net_socket_t *sockrecv, _z_sys_net_socket_t *socksend,
-                            const _z_sys_net_endpoint_t rep) {
+                            const _z_sys_net_endpoint_t rep, const _z_sys_net_endpoint_t lep) {
+    _ZP_UNUSED(lep);
     // FIXME: iface passed into the locator is being ignored
     //        default if used instead
     struct net_if *ifa = NULL;