From 56c82ae78f8f7fde2473174b6c0ab4c5186e3c4e Mon Sep 17 00:00:00 2001 From: Philip Oetinger Date: Wed, 15 Nov 2023 10:17:23 +0100 Subject: [PATCH 1/9] introduce the concept of locking in `ddsrt` --- src/ddsrt/include/dds/ddsrt/cdtors.h | 8 ++++++++ src/ddsrt/src/cdtors.c | 10 ++++++++++ 2 files changed, 18 insertions(+) diff --git a/src/ddsrt/include/dds/ddsrt/cdtors.h b/src/ddsrt/include/dds/ddsrt/cdtors.h index 996809a9dd..0989295212 100644 --- a/src/ddsrt/include/dds/ddsrt/cdtors.h +++ b/src/ddsrt/include/dds/ddsrt/cdtors.h @@ -42,6 +42,14 @@ void ddsrt_init(void); * (when the reference count is 1) actually finalizes it. */ void ddsrt_fini(void); +/// +/// @brief Lock the pooled memory allocator +/// +dds_return_t ddsrt_lock(void); + +/// @brief Unlock the pooled memory allocator +/// +dds_return_t ddsrt_unlock(void); /** * @brief Get a pointer to the global 'init_mutex' diff --git a/src/ddsrt/src/cdtors.c b/src/ddsrt/src/cdtors.c index 8cbf279463..f6a1ac442c 100644 --- a/src/ddsrt/src/cdtors.c +++ b/src/ddsrt/src/cdtors.c @@ -86,6 +86,16 @@ void ddsrt_fini (void) } } +dds_return_t ddsrt_lock (void) { + dds_return_t dds_result = DDS_RETCODE_OK; + return dds_result; +} + +dds_return_t ddsrt_unlock (void) { + dds_return_t dds_result = DDS_RETCODE_OK; + return dds_result; +} + ddsrt_mutex_t *ddsrt_get_singleton_mutex(void) { return &init_mutex; From edf2609b85127f063c42d203f5bae866ac26b499 Mon Sep 17 00:00:00 2001 From: Philip Oetinger Date: Wed, 15 Nov 2023 11:09:32 +0100 Subject: [PATCH 2/9] introduce `dds_domain_lifecycle` Represents the lifecycle state of the system. --- src/core/ddsc/include/dds/dds.h | 7 +++++++ src/core/ddsc/src/dds_domain.c | 1 + 2 files changed, 8 insertions(+) diff --git a/src/core/ddsc/include/dds/dds.h b/src/core/ddsc/include/dds/dds.h index 5a7484683a..f61a0d4b6d 100644 --- a/src/core/ddsc/include/dds/dds.h +++ b/src/core/ddsc/include/dds/dds.h @@ -1103,6 +1103,13 @@ dds_create_domain(const dds_domainid_t domain, const char *config); DDS_EXPORT dds_entity_t dds_create_domain_with_rawconfig(const dds_domainid_t domain, const struct ddsi_config *config); +/// @brief Supported lifecycle states +/// @ingroup domain +enum dds_domain_lifecycle{ + DDS_DOMAIN_LIFECYCLE_INITIALISATION, + DDS_DOMAIN_LIFECYCLE_OPERATIONAL +}; + /** * @brief Get entity parent. * @ingroup entity diff --git a/src/core/ddsc/src/dds_domain.c b/src/core/ddsc/src/dds_domain.c index 68a332553b..14ed7dc8f2 100644 --- a/src/core/ddsc/src/dds_domain.c +++ b/src/core/ddsc/src/dds_domain.c @@ -33,6 +33,7 @@ #include "dds__psmx.h" static dds_return_t dds_domain_free (dds_entity *vdomain); +static dds_domain_lifecycle LIFECYCLE_STATE = DDS_DOMAIN_LIFECYCLE_INITIALISATION; const struct dds_entity_deriver dds_entity_deriver_domain = { .interrupt = dds_entity_deriver_dummy_interrupt, From 47b510272665b3257b8d78eabe1fc3b9ee259bac Mon Sep 17 00:00:00 2001 From: Philip Oetinger Date: Wed, 15 Nov 2023 11:11:32 +0100 Subject: [PATCH 3/9] introduce the lifecycle state transition The only supported operation is transitioning from initialisation to operational. --- src/core/ddsc/include/dds/dds.h | 19 +++++++++++++++++++ src/core/ddsc/src/dds_domain.c | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/src/core/ddsc/include/dds/dds.h b/src/core/ddsc/include/dds/dds.h index f61a0d4b6d..d6380a68d5 100644 --- a/src/core/ddsc/include/dds/dds.h +++ b/src/core/ddsc/include/dds/dds.h @@ -1110,6 +1110,25 @@ enum dds_domain_lifecycle{ DDS_DOMAIN_LIFECYCLE_OPERATIONAL }; +/// @brief Function to indicate where we are in the domain's lifecycle +/// @note Domains starts out in the INITIALISATION +/// state. The only transition currently allowed is to set it OPERATIONAL. +/// The transition is currently only possible when applied to all domains via +/// the `DDS_CYCLONEDDS_HANDLE`. +/// +/// @param[in] domain The domain entity for which to set the lifecycle state. Only accepts `DDS_CYCLONEDDS_HANDLE`. +/// @param[in] state The new state +/// @return a dds_retcode_t indicating success or failure +/// @retval DDS_RETCODE_OK transition was successful +/// @retval DDS_RETCODE_ERROR Internal error prevented the transition. Should not be possible. +/// @retval DDS_RETCODE_ILLEGAL_OPERATION handle refers to a non-domain entity +/// @retval DDS_RETCODE_PRECONDITION_NOT_MET attempt at an disallowed transition +/// @retval DDS_RETCODE_PRECONDITION_NOT_MET Cyclone DDS has not been initialised yet +DDS_EXPORT +dds_return_t dds_set_domain_lifecycle ( + const dds_entity_t domain, + const enum dds_domain_lifecycle state); + /** * @brief Get entity parent. * @ingroup entity diff --git a/src/core/ddsc/src/dds_domain.c b/src/core/ddsc/src/dds_domain.c index 14ed7dc8f2..da37075ca9 100644 --- a/src/core/ddsc/src/dds_domain.c +++ b/src/core/ddsc/src/dds_domain.c @@ -491,4 +491,36 @@ dds_return_t dds_free_typeobj (dds_typeobj_t *type_obj) return DDS_RETCODE_UNSUPPORTED; } +dds_return_t dds_set_domain_lifecycle(const dds_entity_t domain, const enum dds_domain_lifecycle state) { + dds_return_t result = DDS_RETCODE_ERROR; + if (dds_init () < 0) { + result = DDS_RETCODE_PRECONDITION_NOT_MET; + } else { + if (DDS_CYCLONEDDS_HANDLE == domain) { + switch (state) { + case DDS_DOMAIN_LIFECYCLE_OPERATIONAL: + // Only initialisation->operational is valid + if (LIFECYCLE_STATE == DDS_DOMAIN_LIFECYCLE_INITIALISATION) { + result = ddsrt_lock(); + if (result == DDS_RETCODE_SUCCESS) { + LIFECYCLE_STATE = state; + } // else result is the error from ddsrt_lock() + } else { + result = DDS_RETCODE_PRECONDITION_NOT_MET; + } + break; + case DDS_DOMAIN_LIFECYCLE_INITIALISATION: + result = DDS_RETCODE_PRECONDITION_NOT_MET; + break; + default: + result = DDS_RETCODE_ERROR; + break; + } + } else { + result = DDS_RETCODE_ILLEGAL_OPEATION; + } + } + return result; +} + #endif /* DDS_HAS_TYPE_DISCOVERY */ From 03aa97a3f58e84d9988cd2d5ef12c863ffcb2149 Mon Sep 17 00:00:00 2001 From: Philip Oetinger Date: Wed, 15 Nov 2023 11:31:42 +0100 Subject: [PATCH 4/9] fix missing `enum` --- src/core/ddsc/src/dds_domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/ddsc/src/dds_domain.c b/src/core/ddsc/src/dds_domain.c index da37075ca9..2c55a424da 100644 --- a/src/core/ddsc/src/dds_domain.c +++ b/src/core/ddsc/src/dds_domain.c @@ -33,7 +33,7 @@ #include "dds__psmx.h" static dds_return_t dds_domain_free (dds_entity *vdomain); -static dds_domain_lifecycle LIFECYCLE_STATE = DDS_DOMAIN_LIFECYCLE_INITIALISATION; +static enum dds_domain_lifecycle LIFECYCLE_STATE = DDS_DOMAIN_LIFECYCLE_INITIALISATION; const struct dds_entity_deriver dds_entity_deriver_domain = { .interrupt = dds_entity_deriver_dummy_interrupt, From 2d5a31925d1db7dc65f81201fed35a58a6667fb7 Mon Sep 17 00:00:00 2001 From: Philip Oetinger Date: Wed, 15 Nov 2023 14:01:55 +0100 Subject: [PATCH 5/9] introduce dds_get_domain_lifecycle Includes a fix for accidental placement of implementation within an #ifdef causing havok. --- src/core/ddsc/include/dds/dds.h | 19 ++++++++ src/core/ddsc/src/dds_domain.c | 82 ++++++++++++++++++++------------- 2 files changed, 68 insertions(+), 33 deletions(-) diff --git a/src/core/ddsc/include/dds/dds.h b/src/core/ddsc/include/dds/dds.h index d6380a68d5..908ac71feb 100644 --- a/src/core/ddsc/include/dds/dds.h +++ b/src/core/ddsc/include/dds/dds.h @@ -1129,6 +1129,25 @@ dds_return_t dds_set_domain_lifecycle ( const dds_entity_t domain, const enum dds_domain_lifecycle state); +/// @brief Get the status of the domain lifecycle +/// +/// The domain life-cycle state represents the operational phase of the domain. Currently +/// the supported states are: +/// +/// * Initialisation +/// * Operational +/// +/// @param[in] domain The domain entity for which to get the life-cycle state for. Only accepts `DDS_CYCLONEDDS_HANDLE`. +/// @param[out] state The life-cycle state for the provided domain. +/// @return a dds_retcode_t indicating success or failure +/// @retval DDS_RETCODE_SUCCESS The state was successfully retrieved. +/// @retval DDS_RETCODE_BAD_PARAMETER An incorrect domain provided. Only supports `DDS_CYCLONEDDS_HANDLE` currently. +/// @retval DDS_RETCODE_PRECONDITION_NOT_MET An invalid pointer was provided for getting the state. +DDS_EXPORT +dds_return_t dds_get_domain_lifecycle( + const dds_entity_t domain, + enum dds_domain_lifecycle *state); + /** * @brief Get entity parent. * @ingroup entity diff --git a/src/core/ddsc/src/dds_domain.c b/src/core/ddsc/src/dds_domain.c index 2c55a424da..a4de758a7a 100644 --- a/src/core/ddsc/src/dds_domain.c +++ b/src/core/ddsc/src/dds_domain.c @@ -11,6 +11,7 @@ #include #include "dds/features.h" +#include "dds/ddsrt/cdtors.h" #include "dds/ddsrt/process.h" #include "dds/ddsrt/heap.h" #include "dds/ddsrt/hopscotch.h" @@ -440,6 +441,54 @@ void dds_write_set_batch (bool enable) dds_entity_unpin_and_drop_ref (&dds_global.m_entity); } +dds_return_t dds_set_domain_lifecycle(const dds_entity_t domain, const enum dds_domain_lifecycle state) { + dds_return_t result = DDS_RETCODE_ERROR; + if (dds_init () < 0) { + result = DDS_RETCODE_PRECONDITION_NOT_MET; + } else { + if (DDS_CYCLONEDDS_HANDLE == domain) { + switch (state) { + case DDS_DOMAIN_LIFECYCLE_OPERATIONAL: + // Only initialisation->operational is valid + if (LIFECYCLE_STATE == DDS_DOMAIN_LIFECYCLE_INITIALISATION) { + result = ddsrt_lock(); + if (result == DDS_RETCODE_OK) { + LIFECYCLE_STATE = state; + } // else result is the error from ddsrt_lock() + } else { + result = DDS_RETCODE_PRECONDITION_NOT_MET; + } + break; + case DDS_DOMAIN_LIFECYCLE_INITIALISATION: + result = DDS_RETCODE_PRECONDITION_NOT_MET; + break; + default: + result = DDS_RETCODE_ERROR; + break; + } + } else { + result = DDS_RETCODE_ILLEGAL_OPERATION; + } + } + return result; +} + +dds_return_t dds_get_domain_lifecycle(const dds_entity_t domain, enum dds_domain_lifecycle *state) { + dds_return_t result = DDS_RETCODE_OK; + if (NULL == state) { + result = DDS_RETCODE_PRECONDITION_NOT_MET; + } else { + if (DDS_CYCLONEDDS_HANDLE == domain) { + *state = LIFECYCLE_STATE; + } else { + result = DDS_RETCODE_BAD_PARAMETER; + } + } + return result; +} + + + #ifdef DDS_HAS_TYPE_DISCOVERY dds_return_t dds_get_typeobj (dds_entity_t entity, const dds_typeid_t *type_id, dds_duration_t timeout, dds_typeobj_t **type_obj) @@ -490,37 +539,4 @@ dds_return_t dds_free_typeobj (dds_typeobj_t *type_obj) (void) type_obj; return DDS_RETCODE_UNSUPPORTED; } - -dds_return_t dds_set_domain_lifecycle(const dds_entity_t domain, const enum dds_domain_lifecycle state) { - dds_return_t result = DDS_RETCODE_ERROR; - if (dds_init () < 0) { - result = DDS_RETCODE_PRECONDITION_NOT_MET; - } else { - if (DDS_CYCLONEDDS_HANDLE == domain) { - switch (state) { - case DDS_DOMAIN_LIFECYCLE_OPERATIONAL: - // Only initialisation->operational is valid - if (LIFECYCLE_STATE == DDS_DOMAIN_LIFECYCLE_INITIALISATION) { - result = ddsrt_lock(); - if (result == DDS_RETCODE_SUCCESS) { - LIFECYCLE_STATE = state; - } // else result is the error from ddsrt_lock() - } else { - result = DDS_RETCODE_PRECONDITION_NOT_MET; - } - break; - case DDS_DOMAIN_LIFECYCLE_INITIALISATION: - result = DDS_RETCODE_PRECONDITION_NOT_MET; - break; - default: - result = DDS_RETCODE_ERROR; - break; - } - } else { - result = DDS_RETCODE_ILLEGAL_OPEATION; - } - } - return result; -} - #endif /* DDS_HAS_TYPE_DISCOVERY */ From 9800bdcf96886b6f7d97426e90269f7b059d137b Mon Sep 17 00:00:00 2001 From: Philip Oetinger Date: Wed, 15 Nov 2023 14:10:35 +0100 Subject: [PATCH 6/9] add symbol export tests --- src/core/xtests/symbol_export/symbol_export.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/core/xtests/symbol_export/symbol_export.c b/src/core/xtests/symbol_export/symbol_export.c index 1a70c8c00c..2b7308a7e0 100644 --- a/src/core/xtests/symbol_export/symbol_export.c +++ b/src/core/xtests/symbol_export/symbol_export.c @@ -137,6 +137,8 @@ int main (int argc, char **argv) dds_create_participant (0, ptr, ptr); dds_create_domain (0, ptr); dds_create_domain_with_rawconfig (0, ptr); + dds_set_domain_lifecycle(0,0); + dds_get_domain_lifecycle(0,ptr); dds_get_parent (1); dds_get_participant (1); dds_get_children (1, ptr, 0); From 0a4488cf86082b181ea47b1d911c5c985ff01a75 Mon Sep 17 00:00:00 2001 From: Philip Oetinger Date: Mon, 20 Nov 2023 11:38:21 +0100 Subject: [PATCH 7/9] change return value for non-existent state --- src/core/ddsc/src/dds_domain.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/core/ddsc/src/dds_domain.c b/src/core/ddsc/src/dds_domain.c index a4de758a7a..9a856c252a 100644 --- a/src/core/ddsc/src/dds_domain.c +++ b/src/core/ddsc/src/dds_domain.c @@ -455,15 +455,15 @@ dds_return_t dds_set_domain_lifecycle(const dds_entity_t domain, const enum dds_ if (result == DDS_RETCODE_OK) { LIFECYCLE_STATE = state; } // else result is the error from ddsrt_lock() - } else { + } else { // only allow init->operational result = DDS_RETCODE_PRECONDITION_NOT_MET; } break; case DDS_DOMAIN_LIFECYCLE_INITIALISATION: result = DDS_RETCODE_PRECONDITION_NOT_MET; break; - default: - result = DDS_RETCODE_ERROR; + default: // for MISRA compliance, no fall through :( + result = DDS_RETCODE_PRECONDITION_NOT_MET; break; } } else { From cf880f0bd28b3e2e02af45fed46104664dcf8c30 Mon Sep 17 00:00:00 2001 From: Philip Oetinger Date: Mon, 20 Nov 2023 11:38:38 +0100 Subject: [PATCH 8/9] wrap enum in typedef --- src/core/ddsc/include/dds/dds.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/ddsc/include/dds/dds.h b/src/core/ddsc/include/dds/dds.h index 908ac71feb..27aeb79dba 100644 --- a/src/core/ddsc/include/dds/dds.h +++ b/src/core/ddsc/include/dds/dds.h @@ -1105,10 +1105,10 @@ dds_create_domain_with_rawconfig(const dds_domainid_t domain, const struct ddsi_ /// @brief Supported lifecycle states /// @ingroup domain -enum dds_domain_lifecycle{ +typedef enum dds_domain_lifecycle{ DDS_DOMAIN_LIFECYCLE_INITIALISATION, DDS_DOMAIN_LIFECYCLE_OPERATIONAL -}; +} dds_domain_lifecycle_t; /// @brief Function to indicate where we are in the domain's lifecycle /// @note Domains starts out in the INITIALISATION From 39b82b1a9fcaaa915e3db7df2f38a8d580c2dbf4 Mon Sep 17 00:00:00 2001 From: Philip Oetinger Date: Mon, 20 Nov 2023 11:39:07 +0100 Subject: [PATCH 9/9] add tests for get and set domain lifecycle --- src/core/ddsc/tests/domain.c | 44 ++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/src/core/ddsc/tests/domain.c b/src/core/ddsc/tests/domain.c index fd0af73070..a63f63025f 100644 --- a/src/core/ddsc/tests/domain.c +++ b/src/core/ddsc/tests/domain.c @@ -393,3 +393,47 @@ CU_Test(ddsc_domain_create, raw_config) ddsrt_free (arg_raw.buf); } +CU_Test(ddsc_domain_lifecycle, basic_transition) +{ + dds_domain_lifecycle_t lifecycle_state = -1; + dds_return_t retval = dds_get_domain_lifecycle(DDS_CYCLONEDDS_HANDLE, &lifecycle_state); + CU_ASSERT_EQUAL(retval, DDS_RETCODE_OK); + CU_ASSERT_EQUAL(lifecycle_state, DDS_DOMAIN_LIFECYCLE_INITIALISATION); + + retval = dds_set_domain_lifecycle(DDS_CYCLONEDDS_HANDLE, DDS_DOMAIN_LIFECYCLE_OPERATIONAL); + CU_ASSERT_EQUAL(retval, DDS_RETCODE_OK); + retval = dds_get_domain_lifecycle(DDS_CYCLONEDDS_HANDLE, &lifecycle_state); + CU_ASSERT_EQUAL(retval, DDS_RETCODE_OK); + CU_ASSERT_EQUAL(lifecycle_state, DDS_DOMAIN_LIFECYCLE_OPERATIONAL); +} + +CU_Test(ddsc_domain_lifecycle, invalid_transitions) +{ + dds_domain_lifecycle_t lifecycle_state = -1; + + // Null check, get + dds_return_t retval = dds_get_domain_lifecycle(DDS_CYCLONEDDS_HANDLE, NULL); + CU_ASSERT_EQUAL(retval, DDS_RETCODE_PRECONDITION_NOT_MET); + + // Invalid handle, get + retval = dds_get_domain_lifecycle(0, &lifecycle_state); + CU_ASSERT_EQUAL(retval, DDS_RETCODE_BAD_PARAMETER); + + // Invalid init->init transition + retval = dds_set_domain_lifecycle(DDS_CYCLONEDDS_HANDLE, DDS_DOMAIN_LIFECYCLE_INITIALISATION); + CU_ASSERT_EQUAL(retval, DDS_RETCODE_PRECONDITION_NOT_MET); + + // Invalid handle, set + retval = dds_set_domain_lifecycle(0, DDS_DOMAIN_LIFECYCLE_OPERATIONAL); + CU_ASSERT_EQUAL(retval, DDS_RETCODE_ILLEGAL_OPERATION); + + // Invalid operatational->init transition + retval = dds_set_domain_lifecycle(DDS_CYCLONEDDS_HANDLE, DDS_DOMAIN_LIFECYCLE_OPERATIONAL); + CU_ASSERT_EQUAL(retval, DDS_RETCODE_OK); + retval = dds_set_domain_lifecycle(DDS_CYCLONEDDS_HANDLE, DDS_DOMAIN_LIFECYCLE_INITIALISATION); + CU_ASSERT_EQUAL(retval, DDS_RETCODE_PRECONDITION_NOT_MET); + + // Nonexistent state + retval = dds_set_domain_lifecycle(DDS_CYCLONEDDS_HANDLE, 3); + CU_ASSERT_EQUAL(retval, DDS_RETCODE_PRECONDITION_NOT_MET); +}