Fix session memory leak during liveliness cleanup

eclipse-zenoh · Nov 21, 2024 · b534b48 · b534b48
1 parent 6691afc
commit b534b48
Show file tree

Hide file tree

Showing 4 changed files with 14 additions and 12 deletions.
diff --git a/src/api/liveliness.c b/src/api/liveliness.c
@@ -37,19 +37,17 @@ _z_liveliness_token_t _z_liveliness_token_null(void) {
 }
 
 void _z_liveliness_token_clear(_z_liveliness_token_t *token) {
-    if (!_z_liveliness_token_check(token)) {
+    if (_Z_RC_IS_NULL(&token->_zn)) {
         return;
     }
-    // TODO(sashacmc): implement proper check
-    if (token->_zn._val != NULL) {
-        _z_session_rc_t sess_rc = _z_session_weak_upgrade_if_open(&token->_zn);
-        if (!_Z_RC_IS_NULL(&sess_rc)) {
-            _z_undeclare_liveliness_token(token);
-            _z_session_rc_drop(&sess_rc);
-        }
-        _z_session_weak_drop(&token->_zn);
+    _z_session_rc_t sess_rc = _z_session_weak_upgrade_if_open(&token->_zn);
+    if (!_Z_RC_IS_NULL(&sess_rc)) {
+        _z_undeclare_liveliness_token(token);
+        _z_session_rc_drop(&sess_rc);
     }
+    _z_session_weak_drop(&token->_zn);
     _z_keyexpr_clear(&token->_key);
+    *token = _z_liveliness_token_null();
 }
 
 _Z_OWNED_FUNCTIONS_VALUE_NO_COPY_IMPL(_z_liveliness_token_t, liveliness_token, _z_liveliness_token_check,
@@ -70,7 +68,9 @@ z_result_t z_liveliness_declare_token(const z_loaned_session_t *zs, z_owned_live
 }
 
 z_result_t z_liveliness_undeclare_token(z_moved_liveliness_token_t *token) {
-    return _z_undeclare_liveliness_token(&token->_this._val);
+    z_result_t ret = _z_undeclare_liveliness_token(&token->_this._val);
+    _z_liveliness_token_clear(&token->_this._val);
+    return ret;
 }
 
 /**************** Liveliness Subscriber ****************/

diff --git a/src/net/liveliness.c b/src/net/liveliness.c
@@ -60,7 +60,6 @@ z_result_t _z_undeclare_liveliness_token(_z_liveliness_token_t *token) {
     ret = _z_send_n_msg(_Z_RC_IN_VAL(&token->_zn), &n_msg, Z_RELIABILITY_RELIABLE, Z_CONGESTION_CONTROL_BLOCK);
     _z_n_msg_clear(&n_msg);
 
-    *token = _z_liveliness_token_null();
     return ret;
 }
 

diff --git a/src/session/liveliness.c b/src/session/liveliness.c
@@ -107,7 +107,7 @@ z_result_t _z_liveliness_subscription_undeclare(_z_session_t *zn, uint32_t id, c
 
     if (key != NULL) {
         ret = _z_trigger_liveliness_subscriptions_undeclare(zn, *key, timestamp);
-        _z_keyexpr_clear(key);
+        _z_keyexpr_free(&key);
     }
 
     return ret;

diff --git a/tests/z_api_liveliness_test.c b/tests/z_api_liveliness_test.c
@@ -139,6 +139,9 @@ void test_liveliness_sub(bool multicast, bool history) {
     z_sleep_s(1);
     assert(context.token2_drop);
 
+    z_closure_sample_drop(z_closure_sample_move(&closure));
+    z_subscriber_drop(z_subscriber_move(&sub));
+
     assert_ok(zp_stop_read_task(z_loan_mut(s1)));
     assert_ok(zp_stop_read_task(z_loan_mut(s2)));
     assert_ok(zp_stop_lease_task(z_loan_mut(s1)));