From 0c2476900fb6a5901e62a0051083e9aafe3513e7 Mon Sep 17 00:00:00 2001 From: Pierre Avital Date: Thu, 12 Oct 2023 17:50:32 +0200 Subject: [PATCH 1/4] fix memory leak in z_declare_subscriber --- CMakeLists.txt | 5 +++++ include/zenoh-pico/session/session.h | 1 + src/api/api.c | 12 ++++++++---- src/net/primitives.c | 2 ++ zenohpico.pc | 8 ++++---- 5 files changed, 20 insertions(+), 8 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 076446581..81105e115 100644 --- a/CMakeLists.txt +++ b/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") diff --git a/include/zenoh-pico/session/session.h b/include/zenoh-pico/session/session.h index a6102da23..ee3f1653e 100644 --- a/include/zenoh-pico/session/session.h +++ b/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; diff --git a/src/api/api.c b/src/api/api.c index b673e3659..173c511db 100644 --- a/src/api/api.c +++ b/src/api/api.c @@ -810,6 +810,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 @@ -824,11 +825,11 @@ z_owned_subscriber_t z_declare_subscriber(z_session_t zs, z_keyexpr_t keyexpr, z if (wild != NULL && wild != keyexpr._suffix) { wild -= 1; size_t len = wild - keyexpr._suffix; - char *suffix = z_malloc(len + 1); + suffix = z_malloc(len + 1); memcpy(suffix, keyexpr._suffix, len); suffix[len] = 0; keyexpr._suffix = suffix; - _z_keyexpr_set_owns_suffix(&keyexpr, true); + _z_keyexpr_set_owns_suffix(&keyexpr, false); } uint16_t id = _z_declare_resource(zs._val, keyexpr); key = _z_rid_with_suffix(id, wild); @@ -841,9 +842,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, diff --git a/src/net/primitives.c b/src/net/primitives.c index d536f2e96..9d36cc1a8 100644 --- a/src/net/primitives.c +++ b/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; diff --git a/zenohpico.pc b/zenohpico.pc index 6325d30fe..ccbae71d3 100644 --- a/zenohpico.pc +++ b/zenohpico.pc @@ -1,8 +1,8 @@ -prefix=/usr/local +prefix=/var/empty/local Name: zenohpico Description: URL: -Version: 0.10.20230920dev -Cflags: -I${prefix}/ -Libs: -L${prefix}/ -lzenohpico +Version: 0.11.20231012dev +Cflags: -I${prefix}/include +Libs: -L${prefix}/lib64 -lzenohpico From d421206c0e44a1931cf5eb3d62a47007d41e0baf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Guimar=C3=A3es?= Date: Thu, 12 Oct 2023 21:53:36 +0200 Subject: [PATCH 2/4] Update zenohpico.pc --- zenohpico.pc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/zenohpico.pc b/zenohpico.pc index c0c342fea..6a0f039aa 100644 --- a/zenohpico.pc +++ b/zenohpico.pc @@ -1,8 +1,8 @@ -prefix=/var/local +prefix=/usr/local Name: zenohpico Description: URL: Version: 0.11.20231012dev Cflags: -I${prefix}/include -Libs: -L${prefix}/lib -lzenohpico \ No newline at end of file +Libs: -L${prefix}/lib -lzenohpico From 19e694fe68591ee14689cc0da87c7695068262a1 Mon Sep 17 00:00:00 2001 From: Pierre Avital Date: Fri, 13 Oct 2023 14:18:24 +0200 Subject: [PATCH 3/4] check the malloc --- src/api/api.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/api/api.c b/src/api/api.c index 173c511db..ba7bf18b6 100644 --- a/src/api/api.c +++ b/src/api/api.c @@ -826,10 +826,12 @@ z_owned_subscriber_t z_declare_subscriber(z_session_t zs, z_keyexpr_t keyexpr, z wild -= 1; size_t len = wild - keyexpr._suffix; suffix = z_malloc(len + 1); - memcpy(suffix, keyexpr._suffix, len); - suffix[len] = 0; - keyexpr._suffix = suffix; - _z_keyexpr_set_owns_suffix(&keyexpr, false); + if (suffix != NULL) { + memcpy(suffix, keyexpr._suffix, len); + suffix[len] = 0; + keyexpr._suffix = suffix; + _z_keyexpr_set_owns_suffix(&keyexpr, false); + } } uint16_t id = _z_declare_resource(zs._val, keyexpr); key = _z_rid_with_suffix(id, wild); From db2ac4cb91c990d668b3bdee3723b91b10ed45ee Mon Sep 17 00:00:00 2001 From: Pierre Avital Date: Tue, 17 Oct 2023 11:57:42 +0200 Subject: [PATCH 4/4] cancel prefix declaration if allocating suffix failed in subscriber declaration --- src/api/api.c | 9 +++++++-- zenohpico.pc | 6 +++--- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/api/api.c b/src/api/api.c index ba7bf18b6..d8b4cd8b4 100644 --- a/src/api/api.c +++ b/src/api/api.c @@ -822,6 +822,7 @@ 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; @@ -831,10 +832,14 @@ z_owned_subscriber_t z_declare_subscriber(z_session_t zs, z_keyexpr_t keyexpr, z suffix[len] = 0; keyexpr._suffix = suffix; _z_keyexpr_set_owns_suffix(&keyexpr, false); + } else { + do_keydecl = false; } } - uint16_t id = _z_declare_resource(zs._val, keyexpr); - key = _z_rid_with_suffix(id, wild); + if (do_keydecl) { + uint16_t id = _z_declare_resource(zs._val, keyexpr); + key = _z_rid_with_suffix(id, wild); + } } #if Z_FEATURE_MULTICAST_TRANSPORT == 1 } diff --git a/zenohpico.pc b/zenohpico.pc index 6a0f039aa..5282169f0 100644 --- a/zenohpico.pc +++ b/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