Merge pull request #262 from eclipse-zenoh/fix-259

fix memory leak in z_declare_subscriber

Author: p-avital

CMakeLists.txt

@@ -21,6 +21,11 @@ include(GNUInstallDirs)
 option(BUILD_SHARED_LIBS "Build shared libraries if ON, otherwise build static libraries" ON)
 option(ZENOH_DEBUG "Use this to set the ZENOH_DEBUG variable." 0)
 option(WITH_ZEPHYR "Build for Zephyr RTOS" OFF)
+set(CMAKE_EXPORT_COMPILE_COMMANDS ON CACHE INTERNAL "")
+if(CMAKE_EXPORT_COMPILE_COMMANDS)
+  set(CMAKE_CXX_STANDARD_INCLUDE_DIRECTORIES 
+      ${CMAKE_CXX_IMPLICIT_INCLUDE_DIRECTORIES})
+endif()
 
 if(CMAKE_SYSTEM_NAME MATCHES "Windows")
   set(BUILD_SHARED_LIBS "OFF")

include/zenoh-pico/session/session.h

@@ -90,6 +90,7 @@ typedef void (*_z_data_handler_t)(const _z_sample_t *sample, void *arg);
 
 typedef struct {
     _z_keyexpr_t _key;
+    uint16_t _key_id;
     uint32_t _id;
     _z_data_handler_t _callback;
     _z_drop_handler_t _dropper;

src/api/api.c

@@ -890,6 +890,7 @@ z_owned_subscriber_t z_declare_subscriber(z_session_t zs, z_keyexpr_t keyexpr, z
                                           const z_subscriber_options_t *options) {
     void *ctx = callback->context;
     callback->context = NULL;
+    char *suffix = NULL;
 
     z_keyexpr_t key = keyexpr;
     // TODO: Currently, if resource declarations are done over multicast transports, the current protocol definition
@@ -901,17 +902,24 @@ z_owned_subscriber_t z_declare_subscriber(z_session_t zs, z_keyexpr_t keyexpr, z
         _z_resource_t *r = _z_get_resource_by_key(zs._val, &keyexpr);
         if (r == NULL) {
             char *wild = strpbrk(keyexpr._suffix, "*$");
+            _Bool do_keydecl = true;
             if (wild != NULL && wild != keyexpr._suffix) {
                 wild -= 1;
                 size_t len = wild - keyexpr._suffix;
-                char *suffix = z_malloc(len + 1);
-                memcpy(suffix, keyexpr._suffix, len);
-                suffix[len] = 0;
-                keyexpr._suffix = suffix;
-                _z_keyexpr_set_owns_suffix(&keyexpr, true);
+                suffix = z_malloc(len + 1);
+                if (suffix != NULL) {
+                    memcpy(suffix, keyexpr._suffix, len);
+                    suffix[len] = 0;
+                    keyexpr._suffix = suffix;
+                    _z_keyexpr_set_owns_suffix(&keyexpr, false);
+                } else {
+                    do_keydecl = false;
+                }
+            }
+            if (do_keydecl) {
+                uint16_t id = _z_declare_resource(zs._val, keyexpr);
+                key = _z_rid_with_suffix(id, wild);
             }
-            uint16_t id = _z_declare_resource(zs._val, keyexpr);
-            key = _z_rid_with_suffix(id, wild);
         }
 #if Z_FEATURE_MULTICAST_TRANSPORT == 1
     }
@@ -921,9 +929,12 @@ z_owned_subscriber_t z_declare_subscriber(z_session_t zs, z_keyexpr_t keyexpr, z
     if (options != NULL) {
         subinfo.reliability = options->reliability;
     }
+    _z_subscriber_t *sub = _z_declare_subscriber(zs._val, key, subinfo, callback->call, callback->drop, ctx);
+    if (suffix != NULL) {
+        z_free(suffix);
+    }
 
-    return (z_owned_subscriber_t){
-        ._value = _z_declare_subscriber(zs._val, key, subinfo, callback->call, callback->drop, ctx)};
+    return (z_owned_subscriber_t){._value = sub};
 }
 
 z_owned_pull_subscriber_t z_declare_pull_subscriber(z_session_t zs, z_keyexpr_t keyexpr,

src/net/primitives.c

@@ -129,6 +129,7 @@ _z_subscriber_t *_z_declare_subscriber(_z_session_t *zn, _z_keyexpr_t keyexpr, _
                                        _z_data_handler_t callback, _z_drop_handler_t dropper, void *arg) {
     _z_subscription_t s;
     s._id = _z_get_entity_id(zn);
+    s._key_id = keyexpr._id;
     s._key = _z_get_expanded_key_from_key(zn, &keyexpr);
     s._info = sub_info;
     s._callback = callback;
@@ -175,6 +176,7 @@ int8_t _z_undeclare_subscriber(_z_subscriber_t *sub) {
             _z_network_message_t n_msg = _z_n_msg_make_declare(declaration);
             if (_z_send_n_msg(sub->_zn, &n_msg, Z_RELIABILITY_RELIABLE, Z_CONGESTION_CONTROL_BLOCK) == _Z_RES_OK) {
                 // Only if message is successfully send, local subscription state can be removed
+                _z_undeclare_resource(sub->_zn, s->ptr->_key_id);
                 _z_unregister_subscription(sub->_zn, _Z_RESOURCE_IS_LOCAL, s);
             } else {
                 ret = _Z_ERR_ENTITY_UNKNOWN;

zenohpico.pc

@@ -1,8 +1,8 @@
-prefix=/usr/local
+prefix=/var/empty/local
 
 Name: zenohpico
 Description: 
 URL: 
-Version: 0.11.20231012dev
+Version: 0.11.20231017dev
 Cflags: -I${prefix}/include
-Libs: -L${prefix}/lib -lzenohpico
+Libs: -L${prefix}/lib64 -lzenohpico