From 20857b30c6396c9da176bcdf5bf16e269a3c5269 Mon Sep 17 00:00:00 2001 From: Jean-Roland Date: Fri, 8 Mar 2024 14:47:02 +0100 Subject: [PATCH] refactor: go through session_lock function --- src/session/query.c | 49 +++++++++----------------------- src/session/queryable.c | 44 ++++++++--------------------- src/session/resource.c | 57 ++++++++++---------------------------- src/session/subscription.c | 45 ++++++++---------------------- src/session/utils.c | 1 + 5 files changed, 52 insertions(+), 144 deletions(-) diff --git a/src/session/query.c b/src/session/query.c index 491be698d..90d300621 100644 --- a/src/session/query.c +++ b/src/session/query.c @@ -20,6 +20,7 @@ #include "zenoh-pico/net/memory.h" #include "zenoh-pico/protocol/keyexpr.h" #include "zenoh-pico/session/resource.h" +#include "zenoh-pico/session/utils.h" #include "zenoh-pico/utils/logging.h" #if Z_FEATURE_QUERY == 1 @@ -103,15 +104,11 @@ _z_pending_query_t *__unsafe__z_get_pending_query_by_id(_z_session_t *zn, const } _z_pending_query_t *_z_get_pending_query_by_id(_z_session_t *zn, const _z_zint_t id) { -#if Z_FEATURE_MULTI_THREAD == 1 - zp_mutex_lock(&zn->_mutex_inner); -#endif // Z_FEATURE_MULTI_THREAD == 1 + _zp_session_lock_mutex(zn); _z_pending_query_t *pql = __unsafe__z_get_pending_query_by_id(zn, id); -#if Z_FEATURE_MULTI_THREAD == 1 - zp_mutex_unlock(&zn->_mutex_inner); -#endif // Z_FEATURE_MULTI_THREAD == 1 + _zp_session_unlock_mutex(zn); return pql; } @@ -121,9 +118,7 @@ int8_t _z_register_pending_query(_z_session_t *zn, _z_pending_query_t *pen_qry) _Z_DEBUG(">>> Allocating query for (%ju:%s,%s)", (uintmax_t)pen_qry->_key._id, pen_qry->_key._suffix, pen_qry->_parameters); -#if Z_FEATURE_MULTI_THREAD == 1 - zp_mutex_lock(&zn->_mutex_inner); -#endif // Z_FEATURE_MULTI_THREAD == 1 + _zp_session_lock_mutex(zn); _z_pending_query_t *pql = __unsafe__z_get_pending_query_by_id(zn, pen_qry->_id); if (pql == NULL) { // Register query only if a pending one with the same ID does not exist @@ -132,9 +127,7 @@ int8_t _z_register_pending_query(_z_session_t *zn, _z_pending_query_t *pen_qry) ret = _Z_ERR_ENTITY_DECLARATION_FAILED; } -#if Z_FEATURE_MULTI_THREAD == 1 - zp_mutex_unlock(&zn->_mutex_inner); -#endif // Z_FEATURE_MULTI_THREAD == 1 + _zp_session_unlock_mutex(zn); return ret; } @@ -144,9 +137,7 @@ int8_t _z_trigger_query_reply_partial(_z_session_t *zn, const _z_zint_t id, cons const _z_timestamp_t timestamp) { int8_t ret = _Z_RES_OK; -#if Z_FEATURE_MULTI_THREAD == 1 - zp_mutex_lock(&zn->_mutex_inner); -#endif // Z_FEATURE_MULTI_THREAD == 1 + _zp_session_lock_mutex(zn); _z_pending_query_t *pen_qry = __unsafe__z_get_pending_query_by_id(zn, id); if ((ret == _Z_RES_OK) && (pen_qry == NULL)) { @@ -216,9 +207,7 @@ int8_t _z_trigger_query_reply_partial(_z_session_t *zn, const _z_zint_t id, cons } } -#if Z_FEATURE_MULTI_THREAD == 1 - zp_mutex_unlock(&zn->_mutex_inner); -#endif // Z_FEATURE_MULTI_THREAD == 1 + _zp_session_unlock_mutex(zn); // Trigger the user callback if ((ret == _Z_RES_OK) && (pen_qry->_consolidation != Z_CONSOLIDATION_MODE_LATEST)) { @@ -235,9 +224,7 @@ int8_t _z_trigger_query_reply_partial(_z_session_t *zn, const _z_zint_t id, cons int8_t _z_trigger_query_reply_final(_z_session_t *zn, _z_zint_t id) { int8_t ret = _Z_RES_OK; -#if Z_FEATURE_MULTI_THREAD == 1 - zp_mutex_lock(&zn->_mutex_inner); -#endif // Z_FEATURE_MULTI_THREAD == 1 + _zp_session_lock_mutex(zn); // Final reply received for unknown query id _z_pending_query_t *pen_qry = __unsafe__z_get_pending_query_by_id(zn, id); @@ -261,34 +248,24 @@ int8_t _z_trigger_query_reply_final(_z_session_t *zn, _z_zint_t id) { zn->_pending_queries = _z_pending_query_list_drop_filter(zn->_pending_queries, _z_pending_query_eq, pen_qry); } -#if Z_FEATURE_MULTI_THREAD == 1 - zp_mutex_unlock(&zn->_mutex_inner); -#endif // Z_FEATURE_MULTI_THREAD == 1 + _zp_session_unlock_mutex(zn); return ret; } void _z_unregister_pending_query(_z_session_t *zn, _z_pending_query_t *pen_qry) { -#if Z_FEATURE_MULTI_THREAD == 1 - zp_mutex_lock(&zn->_mutex_inner); -#endif // Z_FEATURE_MULTI_THREAD == 1 + _zp_session_lock_mutex(zn); zn->_pending_queries = _z_pending_query_list_drop_filter(zn->_pending_queries, _z_pending_query_eq, pen_qry); -#if Z_FEATURE_MULTI_THREAD == 1 - zp_mutex_unlock(&zn->_mutex_inner); -#endif // Z_FEATURE_MULTI_THREAD == 1 + _zp_session_unlock_mutex(zn); } void _z_flush_pending_queries(_z_session_t *zn) { -#if Z_FEATURE_MULTI_THREAD == 1 - zp_mutex_lock(&zn->_mutex_inner); -#endif // Z_FEATURE_MULTI_THREAD == 1 + _zp_session_lock_mutex(zn); _z_pending_query_list_free(&zn->_pending_queries); -#if Z_FEATURE_MULTI_THREAD == 1 - zp_mutex_unlock(&zn->_mutex_inner); -#endif // Z_FEATURE_MULTI_THREAD == 1 + _zp_session_unlock_mutex(zn); } #endif diff --git a/src/session/queryable.c b/src/session/queryable.c index ce4ba8b19..b669b8b3f 100644 --- a/src/session/queryable.c +++ b/src/session/queryable.c @@ -95,30 +95,22 @@ _z_session_queryable_rc_list_t *__unsafe_z_get_session_queryable_by_key(_z_sessi } _z_session_queryable_rc_t *_z_get_session_queryable_by_id(_z_session_t *zn, const _z_zint_t id) { -#if Z_FEATURE_MULTI_THREAD == 1 - zp_mutex_lock(&zn->_mutex_inner); -#endif // Z_FEATURE_MULTI_THREAD == 1 + _zp_session_lock_mutex(zn); _z_session_queryable_rc_t *qle = __unsafe_z_get_session_queryable_by_id(zn, id); -#if Z_FEATURE_MULTI_THREAD == 1 - zp_mutex_unlock(&zn->_mutex_inner); -#endif // Z_FEATURE_MULTI_THREAD == 1 + _zp_session_unlock_mutex(zn); return qle; } _z_session_queryable_rc_list_t *_z_get_session_queryable_by_key(_z_session_t *zn, const _z_keyexpr_t *keyexpr) { -#if Z_FEATURE_MULTI_THREAD == 1 - zp_mutex_lock(&zn->_mutex_inner); -#endif // Z_FEATURE_MULTI_THREAD == 1 + _zp_session_lock_mutex(zn); _z_keyexpr_t key = __unsafe_z_get_expanded_key_from_key(zn, keyexpr); _z_session_queryable_rc_list_t *qles = __unsafe_z_get_session_queryable_by_key(zn, key); -#if Z_FEATURE_MULTI_THREAD == 1 - zp_mutex_unlock(&zn->_mutex_inner); -#endif // Z_FEATURE_MULTI_THREAD == 1 + _zp_session_unlock_mutex(zn); return qles; } @@ -127,9 +119,7 @@ _z_session_queryable_rc_t *_z_register_session_queryable(_z_session_t *zn, _z_se _Z_DEBUG(">>> Allocating queryable for (%ju:%s)", (uintmax_t)q->_key._id, q->_key._suffix); _z_session_queryable_rc_t *ret = NULL; -#if Z_FEATURE_MULTI_THREAD == 1 - zp_mutex_lock(&zn->_mutex_inner); -#endif // Z_FEATURE_MULTI_THREAD == 1 + _zp_session_lock_mutex(zn); ret = (_z_session_queryable_rc_t *)zp_malloc(sizeof(_z_session_queryable_rc_t)); if (ret != NULL) { @@ -137,9 +127,7 @@ _z_session_queryable_rc_t *_z_register_session_queryable(_z_session_t *zn, _z_se zn->_local_queryable = _z_session_queryable_rc_list_push(zn->_local_queryable, ret); } -#if Z_FEATURE_MULTI_THREAD == 1 - zp_mutex_unlock(&zn->_mutex_inner); -#endif // Z_FEATURE_MULTI_THREAD == 1 + _zp_session_unlock_mutex(zn); return ret; } @@ -147,9 +135,7 @@ _z_session_queryable_rc_t *_z_register_session_queryable(_z_session_t *zn, _z_se int8_t _z_trigger_queryables(_z_session_t *zn, const _z_msg_query_t *msgq, const _z_keyexpr_t q_key, uint32_t qid) { int8_t ret = _Z_RES_OK; -#if Z_FEATURE_MULTI_THREAD == 1 - zp_mutex_lock(&zn->_mutex_inner); -#endif // Z_FEATURE_MULTI_THREAD == 1 + _zp_session_lock_mutex(zn); _z_keyexpr_t key = __unsafe_z_get_expanded_key_from_key(zn, &q_key); if (key._suffix != NULL) { @@ -185,28 +171,20 @@ int8_t _z_trigger_queryables(_z_session_t *zn, const _z_msg_query_t *msgq, const } void _z_unregister_session_queryable(_z_session_t *zn, _z_session_queryable_rc_t *qle) { -#if Z_FEATURE_MULTI_THREAD == 1 - zp_mutex_lock(&zn->_mutex_inner); -#endif // Z_FEATURE_MULTI_THREAD == 1 + _zp_session_lock_mutex(zn); zn->_local_queryable = _z_session_queryable_rc_list_drop_filter(zn->_local_queryable, _z_session_queryable_rc_eq, qle); -#if Z_FEATURE_MULTI_THREAD == 1 - zp_mutex_unlock(&zn->_mutex_inner); -#endif // Z_FEATURE_MULTI_THREAD == 1 + _zp_session_unlock_mutex(zn); } void _z_flush_session_queryable(_z_session_t *zn) { -#if Z_FEATURE_MULTI_THREAD == 1 - zp_mutex_lock(&zn->_mutex_inner); -#endif // Z_FEATURE_MULTI_THREAD == 1 + _zp_session_lock_mutex(zn); _z_session_queryable_rc_list_free(&zn->_local_queryable); -#if Z_FEATURE_MULTI_THREAD == 1 - zp_mutex_unlock(&zn->_mutex_inner); -#endif // Z_FEATURE_MULTI_THREAD == 1 + _zp_session_unlock_mutex(zn); } #endif diff --git a/src/session/resource.c b/src/session/resource.c index f2e1d796e..477ff2789 100644 --- a/src/session/resource.c +++ b/src/session/resource.c @@ -22,6 +22,7 @@ #include "zenoh-pico/protocol/core.h" #include "zenoh-pico/protocol/keyexpr.h" #include "zenoh-pico/session/session.h" +#include "zenoh-pico/session/utils.h" #include "zenoh-pico/system/platform.h" #include "zenoh-pico/utils/logging.h" @@ -180,15 +181,11 @@ _z_keyexpr_t __unsafe_z_get_expanded_key_from_key(_z_session_t *zn, const _z_key } _z_resource_t *_z_get_resource_by_id(_z_session_t *zn, uint16_t mapping, _z_zint_t rid) { -#if Z_FEATURE_MULTI_THREAD == 1 - zp_mutex_lock(&zn->_mutex_inner); -#endif // Z_FEATURE_MULTI_THREAD == 1 + _zp_session_lock_mutex(zn); _z_resource_t *res = __unsafe_z_get_resource_by_id(zn, mapping, rid); -#if Z_FEATURE_MULTI_THREAD == 1 - zp_mutex_unlock(&zn->_mutex_inner); -#endif // Z_FEATURE_MULTI_THREAD == 1 + _zp_session_unlock_mutex(zn); return res; } @@ -197,28 +194,20 @@ _z_resource_t *_z_get_resource_by_key(_z_session_t *zn, const _z_keyexpr_t *keye if (keyexpr->_suffix == NULL) { return _z_get_resource_by_id(zn, _z_keyexpr_mapping_id(keyexpr), keyexpr->_id); } -#if Z_FEATURE_MULTI_THREAD == 1 - zp_mutex_lock(&zn->_mutex_inner); -#endif // Z_FEATURE_MULTI_THREAD == 1 + _zp_session_lock_mutex(zn); _z_resource_t *res = __unsafe_z_get_resource_by_key(zn, keyexpr); -#if Z_FEATURE_MULTI_THREAD == 1 - zp_mutex_unlock(&zn->_mutex_inner); -#endif // Z_FEATURE_MULTI_THREAD == 1 + _zp_session_unlock_mutex(zn); return res; } _z_keyexpr_t _z_get_expanded_key_from_key(_z_session_t *zn, const _z_keyexpr_t *keyexpr) { -#if Z_FEATURE_MULTI_THREAD == 1 - zp_mutex_lock(&zn->_mutex_inner); -#endif // Z_FEATURE_MULTI_THREAD == 1 + _zp_session_lock_mutex(zn); _z_keyexpr_t res = __unsafe_z_get_expanded_key_from_key(zn, keyexpr); -#if Z_FEATURE_MULTI_THREAD == 1 - zp_mutex_unlock(&zn->_mutex_inner); -#endif // Z_FEATURE_MULTI_THREAD == 1 + _zp_session_unlock_mutex(zn); return res; } @@ -230,9 +219,7 @@ int16_t _z_register_resource(_z_session_t *zn, _z_keyexpr_t key, uint16_t id, ui uint16_t mapping = register_to_mapping; uint16_t parent_mapping = _z_keyexpr_mapping_id(&key); -#if Z_FEATURE_MULTI_THREAD == 1 - zp_mutex_lock(&zn->_mutex_inner); -#endif // Z_FEATURE_MULTI_THREAD == 1 + _zp_session_lock_mutex(zn); if (key._id != Z_RESOURCE_ID_NONE) { if (parent_mapping == mapping) { @@ -261,9 +248,7 @@ int16_t _z_register_resource(_z_session_t *zn, _z_keyexpr_t key, uint16_t id, ui } } -#if Z_FEATURE_MULTI_THREAD == 1 - zp_mutex_unlock(&zn->_mutex_inner); -#endif // Z_FEATURE_MULTI_THREAD == 1 + _zp_session_unlock_mutex(zn); return ret; } @@ -271,9 +256,7 @@ int16_t _z_register_resource(_z_session_t *zn, _z_keyexpr_t key, uint16_t id, ui void _z_unregister_resource(_z_session_t *zn, uint16_t id, uint16_t mapping) { _Bool is_local = mapping == _Z_KEYEXPR_MAPPING_LOCAL; _Z_DEBUG("unregistering: id %d, mapping: %d", id, mapping); -#if Z_FEATURE_MULTI_THREAD == 1 - zp_mutex_lock(&zn->_mutex_inner); -#endif // Z_FEATURE_MULTI_THREAD == 1 + _zp_session_lock_mutex(zn); _z_resource_list_t **parent_mut = is_local ? &zn->_local_resources : &zn->_remote_resources; while (id != 0) { _z_resource_list_t *parent = *parent_mut; @@ -295,9 +278,7 @@ void _z_unregister_resource(_z_session_t *zn, uint16_t id, uint16_t mapping) { parent = *parent_mut; } } -#if Z_FEATURE_MULTI_THREAD == 1 - zp_mutex_unlock(&zn->_mutex_inner); -#endif // Z_FEATURE_MULTI_THREAD == 1 + _zp_session_unlock_mutex(zn); } _Bool _z_unregister_resource_for_peer_filter(const _z_resource_t *candidate, const _z_resource_t *ctx) { @@ -305,27 +286,19 @@ _Bool _z_unregister_resource_for_peer_filter(const _z_resource_t *candidate, con return _z_keyexpr_mapping_id(&candidate->_key) == mapping; } void _z_unregister_resources_for_peer(_z_session_t *zn, uint16_t mapping) { -#if Z_FEATURE_MULTI_THREAD == 1 - zp_mutex_lock(&zn->_mutex_inner); -#endif // Z_FEATURE_MULTI_THREAD == 1 + _zp_session_lock_mutex(zn); _z_resource_t ctx = {._id = mapping, ._refcount = 0, ._key = {0}}; zn->_remote_resources = _z_resource_list_drop_filter(zn->_remote_resources, _z_unregister_resource_for_peer_filter, &ctx); -#if Z_FEATURE_MULTI_THREAD == 1 - zp_mutex_unlock(&zn->_mutex_inner); -#endif // Z_FEATURE_MULTI_THREAD == 1 + _zp_session_unlock_mutex(zn); } void _z_flush_resources(_z_session_t *zn) { -#if Z_FEATURE_MULTI_THREAD == 1 - zp_mutex_lock(&zn->_mutex_inner); -#endif // Z_FEATURE_MULTI_THREAD == 1 + _zp_session_lock_mutex(zn); _z_resource_list_free(&zn->_local_resources); _z_resource_list_free(&zn->_remote_resources); -#if Z_FEATURE_MULTI_THREAD == 1 - zp_mutex_unlock(&zn->_mutex_inner); -#endif // Z_FEATURE_MULTI_THREAD == 1 + _zp_session_unlock_mutex(zn); } diff --git a/src/session/subscription.c b/src/session/subscription.c index e85ae2420..68e0cb300 100644 --- a/src/session/subscription.c +++ b/src/session/subscription.c @@ -22,6 +22,7 @@ #include "zenoh-pico/protocol/keyexpr.h" #include "zenoh-pico/session/resource.h" #include "zenoh-pico/session/session.h" +#include "zenoh-pico/session/utils.h" #include "zenoh-pico/utils/logging.h" #if Z_FEATURE_SUBSCRIPTION == 1 @@ -97,29 +98,21 @@ _z_subscription_rc_list_t *__unsafe_z_get_subscriptions_by_key(_z_session_t *zn, } _z_subscription_rc_t *_z_get_subscription_by_id(_z_session_t *zn, uint8_t is_local, const _z_zint_t id) { -#if Z_FEATURE_MULTI_THREAD == 1 - zp_mutex_lock(&zn->_mutex_inner); -#endif // Z_FEATURE_MULTI_THREAD == 1 + _zp_session_lock_mutex(zn); _z_subscription_rc_t *sub = __unsafe_z_get_subscription_by_id(zn, is_local, id); -#if Z_FEATURE_MULTI_THREAD == 1 - zp_mutex_unlock(&zn->_mutex_inner); -#endif // Z_FEATURE_MULTI_THREAD == 1 + _zp_session_unlock_mutex(zn); return sub; } _z_subscription_rc_list_t *_z_get_subscriptions_by_key(_z_session_t *zn, uint8_t is_local, const _z_keyexpr_t *key) { -#if Z_FEATURE_MULTI_THREAD == 1 - zp_mutex_lock(&zn->_mutex_inner); -#endif // Z_FEATURE_MULTI_THREAD == 1 + _zp_session_lock_mutex(zn); _z_subscription_rc_list_t *subs = __unsafe_z_get_subscriptions_by_key(zn, is_local, *key); -#if Z_FEATURE_MULTI_THREAD == 1 - zp_mutex_unlock(&zn->_mutex_inner); -#endif // Z_FEATURE_MULTI_THREAD == 1 + _zp_session_unlock_mutex(zn); return subs; } @@ -128,9 +121,7 @@ _z_subscription_rc_t *_z_register_subscription(_z_session_t *zn, uint8_t is_loca _Z_DEBUG(">>> Allocating sub decl for (%ju:%s)", (uintmax_t)s->_key._id, s->_key._suffix); _z_subscription_rc_t *ret = NULL; -#if Z_FEATURE_MULTI_THREAD == 1 - zp_mutex_lock(&zn->_mutex_inner); -#endif // Z_FEATURE_MULTI_THREAD == 1 + _zp_session_lock_mutex(zn); ret = (_z_subscription_rc_t *)zp_malloc(sizeof(_z_subscription_rc_t)); if (ret != NULL) { @@ -142,9 +133,7 @@ _z_subscription_rc_t *_z_register_subscription(_z_session_t *zn, uint8_t is_loca } } -#if Z_FEATURE_MULTI_THREAD == 1 - zp_mutex_unlock(&zn->_mutex_inner); -#endif // Z_FEATURE_MULTI_THREAD == 1 + _zp_session_unlock_mutex(zn); return ret; } @@ -176,9 +165,7 @@ int8_t _z_trigger_subscriptions(_z_session_t *zn, const _z_keyexpr_t keyexpr, co ) { int8_t ret = _Z_RES_OK; -#if Z_FEATURE_MULTI_THREAD == 1 - zp_mutex_lock(&zn->_mutex_inner); -#endif // Z_FEATURE_MULTI_THREAD == 1 + _zp_session_lock_mutex(zn); _Z_DEBUG("Resolving %d - %s on mapping 0x%x", keyexpr._id, keyexpr._suffix, _z_keyexpr_mapping_id(&keyexpr)); _z_keyexpr_t key = __unsafe_z_get_expanded_key_from_key(zn, &keyexpr); @@ -221,9 +208,7 @@ int8_t _z_trigger_subscriptions(_z_session_t *zn, const _z_keyexpr_t keyexpr, co } void _z_unregister_subscription(_z_session_t *zn, uint8_t is_local, _z_subscription_rc_t *sub) { -#if Z_FEATURE_MULTI_THREAD == 1 - zp_mutex_lock(&zn->_mutex_inner); -#endif // Z_FEATURE_MULTI_THREAD == 1 + _zp_session_lock_mutex(zn); if (is_local == _Z_RESOURCE_IS_LOCAL) { zn->_local_subscriptions = @@ -233,22 +218,16 @@ void _z_unregister_subscription(_z_session_t *zn, uint8_t is_local, _z_subscript _z_subscription_rc_list_drop_filter(zn->_remote_subscriptions, _z_subscription_rc_eq, sub); } -#if Z_FEATURE_MULTI_THREAD == 1 - zp_mutex_unlock(&zn->_mutex_inner); -#endif // Z_FEATURE_MULTI_THREAD == 1 + _zp_session_unlock_mutex(zn); } void _z_flush_subscriptions(_z_session_t *zn) { -#if Z_FEATURE_MULTI_THREAD == 1 - zp_mutex_lock(&zn->_mutex_inner); -#endif // Z_FEATURE_MULTI_THREAD == 1 + _zp_session_lock_mutex(zn); _z_subscription_rc_list_free(&zn->_local_subscriptions); _z_subscription_rc_list_free(&zn->_remote_subscriptions); -#if Z_FEATURE_MULTI_THREAD == 1 - zp_mutex_unlock(&zn->_mutex_inner); -#endif // Z_FEATURE_MULTI_THREAD == 1 + _zp_session_unlock_mutex(zn); } #else // Z_FEATURE_SUBSCRIPTION == 0 diff --git a/src/session/utils.c b/src/session/utils.c index cfeee4b1e..0791f4e47 100644 --- a/src/session/utils.c +++ b/src/session/utils.c @@ -22,6 +22,7 @@ #include "zenoh-pico/session/queryable.h" #include "zenoh-pico/session/resource.h" #include "zenoh-pico/session/subscription.h" +#include "zenoh-pico/utils/logging.h" /*------------------ clone helpers ------------------*/ _z_timestamp_t _z_timestamp_duplicate(const _z_timestamp_t *tstamp) {