From 32078e4a4b094d6a1a7bd56f69d26bf8d5981de8 Mon Sep 17 00:00:00 2001 From: jimwang118 Date: Wed, 2 Aug 2023 14:01:14 +0800 Subject: [PATCH 01/23] Fix macro definition in thread stl --- include/fastrtps/utils/TimedMutex.hpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/include/fastrtps/utils/TimedMutex.hpp b/include/fastrtps/utils/TimedMutex.hpp index e4ff36a7368..094156653b1 100644 --- a/include/fastrtps/utils/TimedMutex.hpp +++ b/include/fastrtps/utils/TimedMutex.hpp @@ -98,11 +98,11 @@ class TimedMutex nsecs -= secs; max_wait.tv_sec += secs.count(); max_wait.tv_nsec = (long)nsecs.count(); - return (_Thrd_success == _Mtx_timedlock(mutex_, (xtime*)&max_wait)); + return (_Thrd_result::_Success == _Mtx_timedlock(mutex_, (xtime*)&max_wait)); } else { - return (_Thrd_success == _Mtx_trylock(mutex_)); + return (_Thrd_result::_Success == _Mtx_trylock(mutex_)); } } @@ -147,7 +147,7 @@ class RecursiveTimedMutex bool try_lock() { - return (_Thrd_success == _Mtx_trylock(mutex_)); + return (_Thrd_result::_Success == _Mtx_trylock(mutex_)); } template @@ -173,11 +173,11 @@ class RecursiveTimedMutex nsecs -= secs; max_wait.tv_sec += secs.count(); max_wait.tv_nsec = (long)nsecs.count(); - return (_Thrd_success == _Mtx_timedlock(mutex_, (xtime*)&max_wait)); + return (_Thrd_result::_Success == _Mtx_timedlock(mutex_, (xtime*)&max_wait)); } else { - return (_Thrd_success == _Mtx_trylock(mutex_)); + return (_Thrd_result::_Success == _Mtx_trylock(mutex_)); } } From 5f57bdebb93a1d72d1b2cf804de2634e21884201 Mon Sep 17 00:00:00 2001 From: jimwang118 Date: Wed, 2 Aug 2023 14:01:14 +0800 Subject: [PATCH 02/23] Fix macro definition in thread stl Signed-off-by: jimwang118 --- include/fastrtps/utils/TimedMutex.hpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/include/fastrtps/utils/TimedMutex.hpp b/include/fastrtps/utils/TimedMutex.hpp index e4ff36a7368..094156653b1 100644 --- a/include/fastrtps/utils/TimedMutex.hpp +++ b/include/fastrtps/utils/TimedMutex.hpp @@ -98,11 +98,11 @@ class TimedMutex nsecs -= secs; max_wait.tv_sec += secs.count(); max_wait.tv_nsec = (long)nsecs.count(); - return (_Thrd_success == _Mtx_timedlock(mutex_, (xtime*)&max_wait)); + return (_Thrd_result::_Success == _Mtx_timedlock(mutex_, (xtime*)&max_wait)); } else { - return (_Thrd_success == _Mtx_trylock(mutex_)); + return (_Thrd_result::_Success == _Mtx_trylock(mutex_)); } } @@ -147,7 +147,7 @@ class RecursiveTimedMutex bool try_lock() { - return (_Thrd_success == _Mtx_trylock(mutex_)); + return (_Thrd_result::_Success == _Mtx_trylock(mutex_)); } template @@ -173,11 +173,11 @@ class RecursiveTimedMutex nsecs -= secs; max_wait.tv_sec += secs.count(); max_wait.tv_nsec = (long)nsecs.count(); - return (_Thrd_success == _Mtx_timedlock(mutex_, (xtime*)&max_wait)); + return (_Thrd_result::_Success == _Mtx_timedlock(mutex_, (xtime*)&max_wait)); } else { - return (_Thrd_success == _Mtx_trylock(mutex_)); + return (_Thrd_result::_Success == _Mtx_trylock(mutex_)); } } From 4d25905b89a46939d8c105feeff2fa9e8295a627 Mon Sep 17 00:00:00 2001 From: jimwang118 Date: Thu, 3 Aug 2023 16:45:13 +0800 Subject: [PATCH 03/23] fix macro in thread --- CMakeLists.txt | 12 +++++++++++- include/fastrtps/utils/TimedMutex.hpp | 10 +++++----- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 2d627832c54..1d2b13df262 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -41,7 +41,17 @@ message(STATUS "Version: ${PROJECT_VERSION}") # eProsima build options ############################################################################### option(EPROSIMA_BUILD "Activate internal building" OFF) - +############################################################################### +# Replace _Thrd_success with _Thrd_result::_Success. As a workround of the +# unreleased version of MSVC, it will be deleted after release. +############################################################################### +if(CMAKE_CXX_COMPILER_ID MATCHES "MSVC") + if(CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL "19.38.32926.95") + file(READ "${PROJECT_SOURCE_DIR}/include/fastrtps/utils/TimedMutex.hpp" _contents) + string(REPLACE "_Thrd_success" "_Thrd_result::_Success" _contents "${_contents}") + file(WRITE "${PROJECT_SOURCE_DIR}/include/fastrtps/utils/TimedMutex.hpp" "${_contents}") + endif() +endif() ############################################################################### # Warning level ############################################################################### diff --git a/include/fastrtps/utils/TimedMutex.hpp b/include/fastrtps/utils/TimedMutex.hpp index 094156653b1..e4ff36a7368 100644 --- a/include/fastrtps/utils/TimedMutex.hpp +++ b/include/fastrtps/utils/TimedMutex.hpp @@ -98,11 +98,11 @@ class TimedMutex nsecs -= secs; max_wait.tv_sec += secs.count(); max_wait.tv_nsec = (long)nsecs.count(); - return (_Thrd_result::_Success == _Mtx_timedlock(mutex_, (xtime*)&max_wait)); + return (_Thrd_success == _Mtx_timedlock(mutex_, (xtime*)&max_wait)); } else { - return (_Thrd_result::_Success == _Mtx_trylock(mutex_)); + return (_Thrd_success == _Mtx_trylock(mutex_)); } } @@ -147,7 +147,7 @@ class RecursiveTimedMutex bool try_lock() { - return (_Thrd_result::_Success == _Mtx_trylock(mutex_)); + return (_Thrd_success == _Mtx_trylock(mutex_)); } template @@ -173,11 +173,11 @@ class RecursiveTimedMutex nsecs -= secs; max_wait.tv_sec += secs.count(); max_wait.tv_nsec = (long)nsecs.count(); - return (_Thrd_result::_Success == _Mtx_timedlock(mutex_, (xtime*)&max_wait)); + return (_Thrd_success == _Mtx_timedlock(mutex_, (xtime*)&max_wait)); } else { - return (_Thrd_result::_Success == _Mtx_trylock(mutex_)); + return (_Thrd_success == _Mtx_trylock(mutex_)); } } From fb280965fabf013a35f98cc5f6b0da90f5bbc236 Mon Sep 17 00:00:00 2001 From: jimwang118 Date: Thu, 3 Aug 2023 17:02:42 +0800 Subject: [PATCH 04/23] fix macro in thread --- include/fastrtps/utils/TimedMutex.hpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/include/fastrtps/utils/TimedMutex.hpp b/include/fastrtps/utils/TimedMutex.hpp index 094156653b1..e4ff36a7368 100644 --- a/include/fastrtps/utils/TimedMutex.hpp +++ b/include/fastrtps/utils/TimedMutex.hpp @@ -98,11 +98,11 @@ class TimedMutex nsecs -= secs; max_wait.tv_sec += secs.count(); max_wait.tv_nsec = (long)nsecs.count(); - return (_Thrd_result::_Success == _Mtx_timedlock(mutex_, (xtime*)&max_wait)); + return (_Thrd_success == _Mtx_timedlock(mutex_, (xtime*)&max_wait)); } else { - return (_Thrd_result::_Success == _Mtx_trylock(mutex_)); + return (_Thrd_success == _Mtx_trylock(mutex_)); } } @@ -147,7 +147,7 @@ class RecursiveTimedMutex bool try_lock() { - return (_Thrd_result::_Success == _Mtx_trylock(mutex_)); + return (_Thrd_success == _Mtx_trylock(mutex_)); } template @@ -173,11 +173,11 @@ class RecursiveTimedMutex nsecs -= secs; max_wait.tv_sec += secs.count(); max_wait.tv_nsec = (long)nsecs.count(); - return (_Thrd_result::_Success == _Mtx_timedlock(mutex_, (xtime*)&max_wait)); + return (_Thrd_success == _Mtx_timedlock(mutex_, (xtime*)&max_wait)); } else { - return (_Thrd_result::_Success == _Mtx_trylock(mutex_)); + return (_Thrd_success == _Mtx_trylock(mutex_)); } } From 0c69f97b043b8e757553c5970cc071d5e8d3e1ac Mon Sep 17 00:00:00 2001 From: jimwang118 Date: Fri, 4 Aug 2023 14:51:30 +0800 Subject: [PATCH 05/23] fix macro on thread --- CMakeLists.txt | 11 ----------- include/fastrtps/utils/TimedMutex.hpp | 10 ++++++++-- 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 1d2b13df262..e26012e5cb4 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -42,17 +42,6 @@ message(STATUS "Version: ${PROJECT_VERSION}") ############################################################################### option(EPROSIMA_BUILD "Activate internal building" OFF) ############################################################################### -# Replace _Thrd_success with _Thrd_result::_Success. As a workround of the -# unreleased version of MSVC, it will be deleted after release. -############################################################################### -if(CMAKE_CXX_COMPILER_ID MATCHES "MSVC") - if(CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL "19.38.32926.95") - file(READ "${PROJECT_SOURCE_DIR}/include/fastrtps/utils/TimedMutex.hpp" _contents) - string(REPLACE "_Thrd_success" "_Thrd_result::_Success" _contents "${_contents}") - file(WRITE "${PROJECT_SOURCE_DIR}/include/fastrtps/utils/TimedMutex.hpp" "${_contents}") - endif() -endif() -############################################################################### # Warning level ############################################################################### if(MSVC OR MSVC_IDE) diff --git a/include/fastrtps/utils/TimedMutex.hpp b/include/fastrtps/utils/TimedMutex.hpp index e4ff36a7368..66041b32900 100644 --- a/include/fastrtps/utils/TimedMutex.hpp +++ b/include/fastrtps/utils/TimedMutex.hpp @@ -45,8 +45,14 @@ class TimedMutex // See https://github.com/microsoft/STL/pull/3594 #if defined(_MSC_FULL_VER) && _MSC_FULL_VER >= 193632528 using xtime = _timespec64; -#endif // _MSC_FULL_VER check - +#endif + // _MSC_FULL_VER check + // On MSVC 19.38.32926.95 `_Thrd_success` was changed into `_Thrd_result::_Success`. + // See https://github.com/eProsima/Fast-DDS/issues/3783 + // See https://github.com/microsoft/STL/pull/3897 +#if defined(_MSC_FULL_VER) && _MSC_FULL_VER >= 193832926 +using _Thrd_success = _Thrd_result::_Success; +#endif public: TimedMutex() From 14967a925d55c08f8ed8b485d5a6196e673420be Mon Sep 17 00:00:00 2001 From: jimwang118 Date: Fri, 4 Aug 2023 14:52:32 +0800 Subject: [PATCH 06/23] fix macro on thread --- CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index e26012e5cb4..2d627832c54 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -41,6 +41,7 @@ message(STATUS "Version: ${PROJECT_VERSION}") # eProsima build options ############################################################################### option(EPROSIMA_BUILD "Activate internal building" OFF) + ############################################################################### # Warning level ############################################################################### From 7e8087d62c4a33704cbcf3a2309b90c647478803 Mon Sep 17 00:00:00 2001 From: jimwang118 Date: Fri, 4 Aug 2023 14:53:08 +0800 Subject: [PATCH 07/23] fix macro on thread --- include/fastrtps/utils/TimedMutex.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/fastrtps/utils/TimedMutex.hpp b/include/fastrtps/utils/TimedMutex.hpp index 66041b32900..76d40f1c6b5 100644 --- a/include/fastrtps/utils/TimedMutex.hpp +++ b/include/fastrtps/utils/TimedMutex.hpp @@ -51,7 +51,7 @@ class TimedMutex // See https://github.com/eProsima/Fast-DDS/issues/3783 // See https://github.com/microsoft/STL/pull/3897 #if defined(_MSC_FULL_VER) && _MSC_FULL_VER >= 193832926 -using _Thrd_success = _Thrd_result::_Success; + using _Thrd_success = _Thrd_result::_Success; #endif public: From 2d4d3c1904de71548cf7f875edc6610bbb4a5620 Mon Sep 17 00:00:00 2001 From: jimwang118 Date: Tue, 22 Aug 2023 06:46:29 +0000 Subject: [PATCH 08/23] Modify return value comparison --- include/fastrtps/utils/TimedMutex.hpp | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/include/fastrtps/utils/TimedMutex.hpp b/include/fastrtps/utils/TimedMutex.hpp index 76d40f1c6b5..ea361a16ff5 100644 --- a/include/fastrtps/utils/TimedMutex.hpp +++ b/include/fastrtps/utils/TimedMutex.hpp @@ -46,13 +46,6 @@ class TimedMutex #if defined(_MSC_FULL_VER) && _MSC_FULL_VER >= 193632528 using xtime = _timespec64; #endif - // _MSC_FULL_VER check - // On MSVC 19.38.32926.95 `_Thrd_success` was changed into `_Thrd_result::_Success`. - // See https://github.com/eProsima/Fast-DDS/issues/3783 - // See https://github.com/microsoft/STL/pull/3897 -#if defined(_MSC_FULL_VER) && _MSC_FULL_VER >= 193832926 - using _Thrd_success = _Thrd_result::_Success; -#endif public: TimedMutex() @@ -104,11 +97,11 @@ class TimedMutex nsecs -= secs; max_wait.tv_sec += secs.count(); max_wait.tv_nsec = (long)nsecs.count(); - return (_Thrd_success == _Mtx_timedlock(mutex_, (xtime*)&max_wait)); + return (0 == static_cast(_Mtx_timedlock(mutex_, (xtime*)&max_wait))); } else { - return (_Thrd_success == _Mtx_trylock(mutex_)); + return (0 == static_cast(_Mtx_trylock(mutex_))); } } @@ -153,7 +146,7 @@ class RecursiveTimedMutex bool try_lock() { - return (_Thrd_success == _Mtx_trylock(mutex_)); + return (0 == static_cast(_Mtx_trylock(mutex_))); } template @@ -179,11 +172,11 @@ class RecursiveTimedMutex nsecs -= secs; max_wait.tv_sec += secs.count(); max_wait.tv_nsec = (long)nsecs.count(); - return (_Thrd_success == _Mtx_timedlock(mutex_, (xtime*)&max_wait)); + return (0 == static_cast(_Mtx_timedlock(mutex_, (xtime*)&max_wait))); } else { - return (_Thrd_success == _Mtx_trylock(mutex_)); + return (0 == static_cast(_Mtx_trylock(mutex_))); } } From 4e6328ee26ad18e082b8170074c0a45a75ecd121 Mon Sep 17 00:00:00 2001 From: Eduardo Ponz Segrelles Date: Wed, 2 Aug 2023 07:08:26 +0200 Subject: [PATCH 09/23] Fix XMLParser null-derefence in parseLogConfig (#3769) * Refs #18395: Add regression test Signed-off-by: Eduardo Ponz * Refs #18395: Refactor XMLParser::parseLogConfig Signed-off-by: Eduardo Ponz --------- Signed-off-by: Eduardo Ponz Signed-off-by: jimwang118 --- src/cpp/rtps/xmlparser/XMLParser.cpp | 72 +++++++++++-------- test/unittest/xmlparser/XMLParserTests.cpp | 1 + .../regressions/18395_profile_bin.xml | 2 + 3 files changed, 47 insertions(+), 28 deletions(-) create mode 100644 test/unittest/xmlparser/regressions/18395_profile_bin.xml diff --git a/src/cpp/rtps/xmlparser/XMLParser.cpp b/src/cpp/rtps/xmlparser/XMLParser.cpp index 0826569fad6..9eba6315e91 100644 --- a/src/cpp/rtps/xmlparser/XMLParser.cpp +++ b/src/cpp/rtps/xmlparser/XMLParser.cpp @@ -1390,23 +1390,28 @@ XMLP_ret XMLParser::parseLogConfig( tinyxml2::XMLElement* p_root) { /* - - - - - - - - - - - - - - + + + + + + + + + + */ + + /* + * TODO(eduponz): Uphold XSD validation in parsing + * Even though the XSD above enforces the log tag to have at least one consumer, + * the parsing allows for an empty log tag (e.g. ``). + * This inconsistency is kept to keep a backwards compatible behaviour. + * In fact, test XMLParserTests.parseXMLNoRoot even checks that an empty log tag + * is valid. */ XMLP_ret ret = XMLP_ret::XML_OK; + tinyxml2::XMLElement* p_aux0 = p_root->FirstChildElement(LOG); if (p_aux0 == nullptr) { @@ -1414,31 +1419,37 @@ XMLP_ret XMLParser::parseLogConfig( } tinyxml2::XMLElement* p_element = p_aux0->FirstChildElement(); - const char* tag = nullptr; - while (nullptr != p_element) + + while (ret == XMLP_ret::XML_OK && nullptr != p_element) { - if (nullptr != (tag = p_element->Value())) + const char* tag = p_element->Value(); + if (nullptr != tag) { if (strcmp(tag, USE_DEFAULT) == 0) { - bool use_default = true; - std::string auxBool = p_element->GetText(); - if (std::strcmp(auxBool.c_str(), "FALSE") == 0) + if (nullptr == p_element->GetText()) { - use_default = false; + EPROSIMA_LOG_ERROR(XMLPARSER, "Cannot get text from tag: '" << tag << "'"); + ret = XMLP_ret::XML_ERROR; } - if (!use_default) + + if (ret == XMLP_ret::XML_OK) { - eprosima::fastdds::dds::Log::ClearConsumers(); + bool use_default = true; + std::string auxBool = p_element->GetText(); + if (std::strcmp(auxBool.c_str(), "FALSE") == 0) + { + use_default = false; + } + if (!use_default) + { + eprosima::fastdds::dds::Log::ClearConsumers(); + } } } else if (strcmp(tag, CONSUMER) == 0) { ret = parseXMLConsumer(*p_element); - if (ret == XMLP_ret::XML_ERROR) - { - return ret; - } } else { @@ -1446,8 +1457,13 @@ XMLP_ret XMLParser::parseLogConfig( ret = XMLP_ret::XML_ERROR; } } - p_element = p_element->NextSiblingElement(CONSUMER); + + if (ret == XMLP_ret::XML_OK) + { + p_element = p_element->NextSiblingElement(CONSUMER); + } } + return ret; } diff --git a/test/unittest/xmlparser/XMLParserTests.cpp b/test/unittest/xmlparser/XMLParserTests.cpp index a8c3a1d11e0..2c58d8fb315 100644 --- a/test/unittest/xmlparser/XMLParserTests.cpp +++ b/test/unittest/xmlparser/XMLParserTests.cpp @@ -60,6 +60,7 @@ TEST_F(XMLParserTests, regressions) EXPECT_EQ(XMLP_ret::XML_ERROR, XMLParser::loadXML("regressions/13513_profile_bin.xml", root)); EXPECT_EQ(XMLP_ret::XML_ERROR, XMLParser::loadXML("regressions/14456_profile_bin.xml", root)); EXPECT_EQ(XMLP_ret::XML_ERROR, XMLParser::loadXML("regressions/15344_profile_bin.xml", root)); + EXPECT_EQ(XMLP_ret::XML_ERROR, XMLParser::loadXML("regressions/18395_profile_bin.xml", root)); } TEST_F(XMLParserTests, NoFile) diff --git a/test/unittest/xmlparser/regressions/18395_profile_bin.xml b/test/unittest/xmlparser/regressions/18395_profile_bin.xml new file mode 100644 index 00000000000..9ff3a6a8fda --- /dev/null +++ b/test/unittest/xmlparser/regressions/18395_profile_bin.xml @@ -0,0 +1,2 @@ +v�< log>suu/use_de�ta-g/>��< /log>��ypes/������ + From 4b9f3c316534b29b5f4236b5caef6bd04858d95e Mon Sep 17 00:00:00 2001 From: jimwang118 Date: Wed, 2 Aug 2023 14:01:14 +0800 Subject: [PATCH 10/23] Fix macro definition in thread stl Signed-off-by: jimwang118 --- include/fastrtps/utils/TimedMutex.hpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/include/fastrtps/utils/TimedMutex.hpp b/include/fastrtps/utils/TimedMutex.hpp index e4ff36a7368..094156653b1 100644 --- a/include/fastrtps/utils/TimedMutex.hpp +++ b/include/fastrtps/utils/TimedMutex.hpp @@ -98,11 +98,11 @@ class TimedMutex nsecs -= secs; max_wait.tv_sec += secs.count(); max_wait.tv_nsec = (long)nsecs.count(); - return (_Thrd_success == _Mtx_timedlock(mutex_, (xtime*)&max_wait)); + return (_Thrd_result::_Success == _Mtx_timedlock(mutex_, (xtime*)&max_wait)); } else { - return (_Thrd_success == _Mtx_trylock(mutex_)); + return (_Thrd_result::_Success == _Mtx_trylock(mutex_)); } } @@ -147,7 +147,7 @@ class RecursiveTimedMutex bool try_lock() { - return (_Thrd_success == _Mtx_trylock(mutex_)); + return (_Thrd_result::_Success == _Mtx_trylock(mutex_)); } template @@ -173,11 +173,11 @@ class RecursiveTimedMutex nsecs -= secs; max_wait.tv_sec += secs.count(); max_wait.tv_nsec = (long)nsecs.count(); - return (_Thrd_success == _Mtx_timedlock(mutex_, (xtime*)&max_wait)); + return (_Thrd_result::_Success == _Mtx_timedlock(mutex_, (xtime*)&max_wait)); } else { - return (_Thrd_success == _Mtx_trylock(mutex_)); + return (_Thrd_result::_Success == _Mtx_trylock(mutex_)); } } From 6a89cf84bf015ff5d0d0338a56a0b9c95f33e95e Mon Sep 17 00:00:00 2001 From: jimwang118 Date: Thu, 3 Aug 2023 16:45:13 +0800 Subject: [PATCH 11/23] fix macro in thread Signed-off-by: jimwang118 --- CMakeLists.txt | 12 +++++++++++- include/fastrtps/utils/TimedMutex.hpp | 10 +++++----- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 2d627832c54..1d2b13df262 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -41,7 +41,17 @@ message(STATUS "Version: ${PROJECT_VERSION}") # eProsima build options ############################################################################### option(EPROSIMA_BUILD "Activate internal building" OFF) - +############################################################################### +# Replace _Thrd_success with _Thrd_result::_Success. As a workround of the +# unreleased version of MSVC, it will be deleted after release. +############################################################################### +if(CMAKE_CXX_COMPILER_ID MATCHES "MSVC") + if(CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL "19.38.32926.95") + file(READ "${PROJECT_SOURCE_DIR}/include/fastrtps/utils/TimedMutex.hpp" _contents) + string(REPLACE "_Thrd_success" "_Thrd_result::_Success" _contents "${_contents}") + file(WRITE "${PROJECT_SOURCE_DIR}/include/fastrtps/utils/TimedMutex.hpp" "${_contents}") + endif() +endif() ############################################################################### # Warning level ############################################################################### diff --git a/include/fastrtps/utils/TimedMutex.hpp b/include/fastrtps/utils/TimedMutex.hpp index 094156653b1..e4ff36a7368 100644 --- a/include/fastrtps/utils/TimedMutex.hpp +++ b/include/fastrtps/utils/TimedMutex.hpp @@ -98,11 +98,11 @@ class TimedMutex nsecs -= secs; max_wait.tv_sec += secs.count(); max_wait.tv_nsec = (long)nsecs.count(); - return (_Thrd_result::_Success == _Mtx_timedlock(mutex_, (xtime*)&max_wait)); + return (_Thrd_success == _Mtx_timedlock(mutex_, (xtime*)&max_wait)); } else { - return (_Thrd_result::_Success == _Mtx_trylock(mutex_)); + return (_Thrd_success == _Mtx_trylock(mutex_)); } } @@ -147,7 +147,7 @@ class RecursiveTimedMutex bool try_lock() { - return (_Thrd_result::_Success == _Mtx_trylock(mutex_)); + return (_Thrd_success == _Mtx_trylock(mutex_)); } template @@ -173,11 +173,11 @@ class RecursiveTimedMutex nsecs -= secs; max_wait.tv_sec += secs.count(); max_wait.tv_nsec = (long)nsecs.count(); - return (_Thrd_result::_Success == _Mtx_timedlock(mutex_, (xtime*)&max_wait)); + return (_Thrd_success == _Mtx_timedlock(mutex_, (xtime*)&max_wait)); } else { - return (_Thrd_result::_Success == _Mtx_trylock(mutex_)); + return (_Thrd_success == _Mtx_trylock(mutex_)); } } From 2d55134f794b20394f07afa1ca82b90ceb53a3f2 Mon Sep 17 00:00:00 2001 From: jimwang118 Date: Wed, 2 Aug 2023 14:01:14 +0800 Subject: [PATCH 12/23] Fix macro definition in thread stl Signed-off-by: jimwang118 --- include/fastrtps/utils/TimedMutex.hpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/include/fastrtps/utils/TimedMutex.hpp b/include/fastrtps/utils/TimedMutex.hpp index e4ff36a7368..094156653b1 100644 --- a/include/fastrtps/utils/TimedMutex.hpp +++ b/include/fastrtps/utils/TimedMutex.hpp @@ -98,11 +98,11 @@ class TimedMutex nsecs -= secs; max_wait.tv_sec += secs.count(); max_wait.tv_nsec = (long)nsecs.count(); - return (_Thrd_success == _Mtx_timedlock(mutex_, (xtime*)&max_wait)); + return (_Thrd_result::_Success == _Mtx_timedlock(mutex_, (xtime*)&max_wait)); } else { - return (_Thrd_success == _Mtx_trylock(mutex_)); + return (_Thrd_result::_Success == _Mtx_trylock(mutex_)); } } @@ -147,7 +147,7 @@ class RecursiveTimedMutex bool try_lock() { - return (_Thrd_success == _Mtx_trylock(mutex_)); + return (_Thrd_result::_Success == _Mtx_trylock(mutex_)); } template @@ -173,11 +173,11 @@ class RecursiveTimedMutex nsecs -= secs; max_wait.tv_sec += secs.count(); max_wait.tv_nsec = (long)nsecs.count(); - return (_Thrd_success == _Mtx_timedlock(mutex_, (xtime*)&max_wait)); + return (_Thrd_result::_Success == _Mtx_timedlock(mutex_, (xtime*)&max_wait)); } else { - return (_Thrd_success == _Mtx_trylock(mutex_)); + return (_Thrd_result::_Success == _Mtx_trylock(mutex_)); } } From b620a2a7caefcf00fd4f531cbe417fd1adacf037 Mon Sep 17 00:00:00 2001 From: jimwang118 Date: Thu, 3 Aug 2023 17:02:42 +0800 Subject: [PATCH 13/23] fix macro in thread Signed-off-by: jimwang118 --- include/fastrtps/utils/TimedMutex.hpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/include/fastrtps/utils/TimedMutex.hpp b/include/fastrtps/utils/TimedMutex.hpp index 094156653b1..e4ff36a7368 100644 --- a/include/fastrtps/utils/TimedMutex.hpp +++ b/include/fastrtps/utils/TimedMutex.hpp @@ -98,11 +98,11 @@ class TimedMutex nsecs -= secs; max_wait.tv_sec += secs.count(); max_wait.tv_nsec = (long)nsecs.count(); - return (_Thrd_result::_Success == _Mtx_timedlock(mutex_, (xtime*)&max_wait)); + return (_Thrd_success == _Mtx_timedlock(mutex_, (xtime*)&max_wait)); } else { - return (_Thrd_result::_Success == _Mtx_trylock(mutex_)); + return (_Thrd_success == _Mtx_trylock(mutex_)); } } @@ -147,7 +147,7 @@ class RecursiveTimedMutex bool try_lock() { - return (_Thrd_result::_Success == _Mtx_trylock(mutex_)); + return (_Thrd_success == _Mtx_trylock(mutex_)); } template @@ -173,11 +173,11 @@ class RecursiveTimedMutex nsecs -= secs; max_wait.tv_sec += secs.count(); max_wait.tv_nsec = (long)nsecs.count(); - return (_Thrd_result::_Success == _Mtx_timedlock(mutex_, (xtime*)&max_wait)); + return (_Thrd_success == _Mtx_timedlock(mutex_, (xtime*)&max_wait)); } else { - return (_Thrd_result::_Success == _Mtx_trylock(mutex_)); + return (_Thrd_success == _Mtx_trylock(mutex_)); } } From b48b645c3fd6d38c33ca6012149ada1b924fe8a1 Mon Sep 17 00:00:00 2001 From: jimwang118 Date: Fri, 4 Aug 2023 14:51:30 +0800 Subject: [PATCH 14/23] fix macro on thread Signed-off-by: jimwang118 --- CMakeLists.txt | 11 ----------- include/fastrtps/utils/TimedMutex.hpp | 10 ++++++++-- 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 1d2b13df262..e26012e5cb4 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -42,17 +42,6 @@ message(STATUS "Version: ${PROJECT_VERSION}") ############################################################################### option(EPROSIMA_BUILD "Activate internal building" OFF) ############################################################################### -# Replace _Thrd_success with _Thrd_result::_Success. As a workround of the -# unreleased version of MSVC, it will be deleted after release. -############################################################################### -if(CMAKE_CXX_COMPILER_ID MATCHES "MSVC") - if(CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL "19.38.32926.95") - file(READ "${PROJECT_SOURCE_DIR}/include/fastrtps/utils/TimedMutex.hpp" _contents) - string(REPLACE "_Thrd_success" "_Thrd_result::_Success" _contents "${_contents}") - file(WRITE "${PROJECT_SOURCE_DIR}/include/fastrtps/utils/TimedMutex.hpp" "${_contents}") - endif() -endif() -############################################################################### # Warning level ############################################################################### if(MSVC OR MSVC_IDE) diff --git a/include/fastrtps/utils/TimedMutex.hpp b/include/fastrtps/utils/TimedMutex.hpp index e4ff36a7368..66041b32900 100644 --- a/include/fastrtps/utils/TimedMutex.hpp +++ b/include/fastrtps/utils/TimedMutex.hpp @@ -45,8 +45,14 @@ class TimedMutex // See https://github.com/microsoft/STL/pull/3594 #if defined(_MSC_FULL_VER) && _MSC_FULL_VER >= 193632528 using xtime = _timespec64; -#endif // _MSC_FULL_VER check - +#endif + // _MSC_FULL_VER check + // On MSVC 19.38.32926.95 `_Thrd_success` was changed into `_Thrd_result::_Success`. + // See https://github.com/eProsima/Fast-DDS/issues/3783 + // See https://github.com/microsoft/STL/pull/3897 +#if defined(_MSC_FULL_VER) && _MSC_FULL_VER >= 193832926 +using _Thrd_success = _Thrd_result::_Success; +#endif public: TimedMutex() From 04c31116634985b96813f3e59276a3a2538b1f60 Mon Sep 17 00:00:00 2001 From: jimwang118 Date: Fri, 4 Aug 2023 14:52:32 +0800 Subject: [PATCH 15/23] fix macro on thread Signed-off-by: jimwang118 --- CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index e26012e5cb4..2d627832c54 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -41,6 +41,7 @@ message(STATUS "Version: ${PROJECT_VERSION}") # eProsima build options ############################################################################### option(EPROSIMA_BUILD "Activate internal building" OFF) + ############################################################################### # Warning level ############################################################################### From a4548f5a835f59add358358376380044fee1e2c5 Mon Sep 17 00:00:00 2001 From: jimwang118 Date: Fri, 4 Aug 2023 14:53:08 +0800 Subject: [PATCH 16/23] fix macro on thread Signed-off-by: jimwang118 --- include/fastrtps/utils/TimedMutex.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/fastrtps/utils/TimedMutex.hpp b/include/fastrtps/utils/TimedMutex.hpp index 66041b32900..76d40f1c6b5 100644 --- a/include/fastrtps/utils/TimedMutex.hpp +++ b/include/fastrtps/utils/TimedMutex.hpp @@ -51,7 +51,7 @@ class TimedMutex // See https://github.com/eProsima/Fast-DDS/issues/3783 // See https://github.com/microsoft/STL/pull/3897 #if defined(_MSC_FULL_VER) && _MSC_FULL_VER >= 193832926 -using _Thrd_success = _Thrd_result::_Success; + using _Thrd_success = _Thrd_result::_Success; #endif public: From 8e7e5f77471e46b222958e86f45fe6feb0b73e9b Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Wed, 2 Aug 2023 13:13:23 +0200 Subject: [PATCH 17/23] User configuration for SHM metatraffic (#3753) * Refs #18966. Forcing UDP for metatraffic. Signed-off-by: Miguel Company * Refs #19255. Allowing metatraffic depends on flags. Signed-off-by: Miguel Company * Refs #19255. NetworkFactory constructor receives RTPSParticipantAttributes. Signed-off-by: Miguel Company * Refs #19255. Parsing property from participant attributes. Signed-off-by: Miguel Company * Refs #19255. Remove unused mp_ResourceSemaphore. Signed-off-by: Miguel Company * Refs #19263. Fixed NetworkFactoryTests. Signed-off-by: Miguel Company * Refs #19263. Fixed TCPv6Tests. Signed-off-by: Miguel Company * Refs #19263. Fixed link errors on unit tests. Signed-off-by: Miguel Company * Refs #19263. Apply suggestions from code review. Signed-off-by: Miguel Company * Refs #19263. Additional suggestions from code review. Signed-off-by: Miguel Company * Refs #19263. Added possitive test. Signed-off-by: Miguel Company * Refs #19263. Added negative test. Signed-off-by: Miguel Company * Refs #19263. Configuration for avoid_builtin_multicast on PubSubWriter. Signed-off-by: Miguel Company * Refs #19263. Configuration for avoid_builtin_multicast on PubSubReader. Signed-off-by: Miguel Company * Refs #19263. Configuration for max_multicast_locators_number on PubSubReader/PubSubWriter. Signed-off-by: Miguel Company * Refs #19263. Enable multicast discovery on new test. Signed-off-by: Miguel Company * Refs #19263. Apply suggestions. Signed-off-by: Miguel Company * Refs #19263. Add feature to versions.md. Signed-off-by: Miguel Company * Refs #19263. Apply suggestion. Signed-off-by: Miguel Company --------- Signed-off-by: Miguel Company Signed-off-by: jimwang118 --- src/cpp/rtps/network/NetworkFactory.cpp | 52 +++++++++++- src/cpp/rtps/network/NetworkFactory.h | 9 +- .../rtps/participant/RTPSParticipantImpl.cpp | 19 +---- .../rtps/participant/RTPSParticipantImpl.h | 8 -- test/blackbox/api/dds-pim/PubSubReader.hpp | 14 ++++ test/blackbox/api/dds-pim/PubSubWriter.hpp | 14 ++++ .../api/fastrtps_deprecated/PubSubReader.hpp | 7 ++ .../api/fastrtps_deprecated/PubSubWriter.hpp | 7 ++ .../DDSBlackboxTestsTransportSHMUDP.cpp | 84 +++++++++++++++++++ test/unittest/rtps/network/CMakeLists.txt | 4 + .../rtps/network/NetworkFactoryTests.cpp | 6 +- test/unittest/transport/CMakeLists.txt | 12 +++ test/unittest/transport/TCPv6Tests.cpp | 4 +- versions.md | 3 + 14 files changed, 209 insertions(+), 34 deletions(-) diff --git a/src/cpp/rtps/network/NetworkFactory.cpp b/src/cpp/rtps/network/NetworkFactory.cpp index a641bc3c060..6a46faa8283 100644 --- a/src/cpp/rtps/network/NetworkFactory.cpp +++ b/src/cpp/rtps/network/NetworkFactory.cpp @@ -34,10 +34,36 @@ namespace rtps { using SendResourceList = fastdds::rtps::SendResourceList; -NetworkFactory::NetworkFactory() +NetworkFactory::NetworkFactory( + const RTPSParticipantAttributes& PParam) : maxMessageSizeBetweenTransports_(std::numeric_limits::max()) , minSendBufferSize_(std::numeric_limits::max()) { + const std::string* enforce_metatraffic = nullptr; + enforce_metatraffic = PropertyPolicyHelper::find_property(PParam.properties, "fastdds.shm.enforce_metatraffic"); + if (enforce_metatraffic) + { + if (*enforce_metatraffic == "unicast") + { + enforce_shm_unicast_metatraffic_ = true; + enforce_shm_multicast_metatraffic_ = false; + } + else if (*enforce_metatraffic == "all") + { + enforce_shm_unicast_metatraffic_ = true; + enforce_shm_multicast_metatraffic_ = true; + } + else if (*enforce_metatraffic == "none") + { + enforce_shm_unicast_metatraffic_ = false; + enforce_shm_multicast_metatraffic_ = false; + } + else + { + EPROSIMA_LOG_WARNING(RTPS_NETWORK, "Unrecognized value '" << *enforce_metatraffic << "'" << + " for 'fastdds.shm.enforce_metatraffic'. Using default value: 'none'"); + } + } } bool NetworkFactory::build_send_resources( @@ -246,9 +272,9 @@ bool NetworkFactory::getDefaultMetatrafficMulticastLocators( for (auto& transport : mRegisteredTransports) { - // For better fault-tolerance reasons, SHM multicast metatraffic is avoided if it is already provided + // For better fault-tolerance reasons, SHM metatraffic is avoided if it is already provided // by another transport - if (transport->kind() != LOCATOR_KIND_SHM) + if (enforce_shm_multicast_metatraffic_ || transport->kind() != LOCATOR_KIND_SHM) { result |= transport->getDefaultMetatrafficMulticastLocators(locators, metatraffic_multicast_port); } @@ -286,10 +312,28 @@ bool NetworkFactory::getDefaultMetatrafficUnicastLocators( uint32_t metatraffic_unicast_port) const { bool result = false; + + TransportInterface* shm_transport = nullptr; + for (auto& transport : mRegisteredTransports) { - result |= transport->getDefaultMetatrafficUnicastLocators(locators, metatraffic_unicast_port); + // For better fault-tolerance reasons, SHM metatraffic is avoided if it is already provided + // by another transport + if (enforce_shm_unicast_metatraffic_ || transport->kind() != LOCATOR_KIND_SHM) + { + result |= transport->getDefaultMetatrafficUnicastLocators(locators, metatraffic_unicast_port); + } + else + { + shm_transport = transport.get(); + } + } + + if (locators.size() == 0 && shm_transport) + { + result |= shm_transport->getDefaultMetatrafficUnicastLocators(locators, metatraffic_unicast_port); } + return result; } diff --git a/src/cpp/rtps/network/NetworkFactory.h b/src/cpp/rtps/network/NetworkFactory.h index e49d70a5bff..ade996764b7 100644 --- a/src/cpp/rtps/network/NetworkFactory.h +++ b/src/cpp/rtps/network/NetworkFactory.h @@ -43,7 +43,8 @@ class NetworkFactory { public: - NetworkFactory(); + NetworkFactory( + const RTPSParticipantAttributes& PParam); /** * Allow registration of a transport statically, by specifying the transport type and @@ -224,6 +225,12 @@ class NetworkFactory uint32_t minSendBufferSize_; + // Whether unicast metatraffic on SHM transport should always be used + bool enforce_shm_unicast_metatraffic_ = false; + + // Whether multicast metatraffic on SHM transport should always be used + bool enforce_shm_multicast_metatraffic_ = false; + /** * Calculate well-known ports. */ diff --git a/src/cpp/rtps/participant/RTPSParticipantImpl.cpp b/src/cpp/rtps/participant/RTPSParticipantImpl.cpp index 9634443b027..48f00d6a449 100644 --- a/src/cpp/rtps/participant/RTPSParticipantImpl.cpp +++ b/src/cpp/rtps/participant/RTPSParticipantImpl.cpp @@ -130,8 +130,8 @@ RTPSParticipantImpl::RTPSParticipantImpl( , m_att(PParam) , m_guid(guidP, c_EntityId_RTPSParticipant) , mp_builtinProtocols(nullptr) - , mp_ResourceSemaphore(new Semaphore(0)) , IdCounter(0) + , m_network_Factory(PParam) , type_check_fn_(nullptr) , client_override_(false) , internal_metatraffic_locators_(false) @@ -512,7 +512,6 @@ RTPSParticipantImpl::~RTPSParticipantImpl() } m_receiverResourcelist.clear(); - delete mp_ResourceSemaphore; delete mp_userParticipant; mp_userParticipant = nullptr; send_resource_list_.clear(); @@ -2034,22 +2033,6 @@ bool RTPSParticipantImpl::newRemoteEndpointDiscovered( return false; } -void RTPSParticipantImpl::ResourceSemaphorePost() -{ - if (mp_ResourceSemaphore != nullptr) - { - mp_ResourceSemaphore->post(); - } -} - -void RTPSParticipantImpl::ResourceSemaphoreWait() -{ - if (mp_ResourceSemaphore != nullptr) - { - mp_ResourceSemaphore->wait(); - } -} - void RTPSParticipantImpl::assert_remote_participant_liveliness( const GuidPrefix_t& remote_guid) { diff --git a/src/cpp/rtps/participant/RTPSParticipantImpl.h b/src/cpp/rtps/participant/RTPSParticipantImpl.h index 4674aa58ee8..03b68851747 100644 --- a/src/cpp/rtps/participant/RTPSParticipantImpl.h +++ b/src/cpp/rtps/participant/RTPSParticipantImpl.h @@ -240,12 +240,6 @@ class RTPSParticipantImpl return (uint32_t)m_att.participantID; } - //!Post to the resource semaphore - void ResourceSemaphorePost(); - - //!Wait for the resource semaphore - void ResourceSemaphoreWait(); - //!Get Pointer to the Event Resource. ResourceEvent& getEventResource() { @@ -529,8 +523,6 @@ class RTPSParticipantImpl ResourceEvent mp_event_thr; //! BuiltinProtocols of this RTPSParticipant BuiltinProtocols* mp_builtinProtocols; - //!Semaphore to wait for the listen thread creation. - Semaphore* mp_ResourceSemaphore; //!Id counter to correctly assign the ids to writers and readers. std::atomic IdCounter; //! Mutex to safely access endpoints collections diff --git a/test/blackbox/api/dds-pim/PubSubReader.hpp b/test/blackbox/api/dds-pim/PubSubReader.hpp index 3a71df84114..29b8257bb36 100644 --- a/test/blackbox/api/dds-pim/PubSubReader.hpp +++ b/test/blackbox/api/dds-pim/PubSubReader.hpp @@ -1270,6 +1270,13 @@ class PubSubReader return *this; } + PubSubReader& avoid_builtin_multicast( + bool value) + { + participant_qos_.wire_protocol().builtin.avoid_builtin_multicast = value; + return *this; + } + PubSubReader& property_policy( const eprosima::fastrtps::rtps::PropertyPolicy& property_policy) { @@ -1319,6 +1326,13 @@ class PubSubReader return *this; } + PubSubReader& max_multicast_locators_number( + size_t max_multicast_locators) + { + participant_qos_.allocation().locators.max_multicast_locators = max_multicast_locators; + return *this; + } + PubSubReader& lease_duration( eprosima::fastrtps::Duration_t lease_duration, eprosima::fastrtps::Duration_t announce_period) diff --git a/test/blackbox/api/dds-pim/PubSubWriter.hpp b/test/blackbox/api/dds-pim/PubSubWriter.hpp index 5148b147603..65f72a52d8b 100644 --- a/test/blackbox/api/dds-pim/PubSubWriter.hpp +++ b/test/blackbox/api/dds-pim/PubSubWriter.hpp @@ -1215,6 +1215,13 @@ class PubSubWriter return *this; } + PubSubWriter& avoid_builtin_multicast( + bool value) + { + participant_qos_.wire_protocol().builtin.avoid_builtin_multicast = value; + return *this; + } + PubSubWriter& property_policy( const eprosima::fastrtps::rtps::PropertyPolicy& property_policy) { @@ -1312,6 +1319,13 @@ class PubSubWriter return *this; } + PubSubWriter& max_multicast_locators_number( + size_t max_multicast_locators) + { + participant_qos_.allocation().locators.max_multicast_locators = max_multicast_locators; + return *this; + } + PubSubWriter& lease_duration( eprosima::fastrtps::Duration_t lease_duration, eprosima::fastrtps::Duration_t announce_period) diff --git a/test/blackbox/api/fastrtps_deprecated/PubSubReader.hpp b/test/blackbox/api/fastrtps_deprecated/PubSubReader.hpp index 3b5b506c18c..9541c2b1417 100644 --- a/test/blackbox/api/fastrtps_deprecated/PubSubReader.hpp +++ b/test/blackbox/api/fastrtps_deprecated/PubSubReader.hpp @@ -1026,6 +1026,13 @@ class PubSubReader return *this; } + PubSubReader& avoid_builtin_multicast( + bool value) + { + participant_attr_.rtps.builtin.avoid_builtin_multicast = value; + return *this; + } + PubSubReader& property_policy( const eprosima::fastrtps::rtps::PropertyPolicy property_policy) { diff --git a/test/blackbox/api/fastrtps_deprecated/PubSubWriter.hpp b/test/blackbox/api/fastrtps_deprecated/PubSubWriter.hpp index 5f777a20dfd..696c336e4dc 100644 --- a/test/blackbox/api/fastrtps_deprecated/PubSubWriter.hpp +++ b/test/blackbox/api/fastrtps_deprecated/PubSubWriter.hpp @@ -995,6 +995,13 @@ class PubSubWriter return *this; } + PubSubWriter& avoid_builtin_multicast( + bool value) + { + participant_attr_.rtps.builtin.avoid_builtin_multicast = value; + return *this; + } + PubSubWriter& property_policy( const eprosima::fastrtps::rtps::PropertyPolicy& property_policy) { diff --git a/test/blackbox/common/DDSBlackboxTestsTransportSHMUDP.cpp b/test/blackbox/common/DDSBlackboxTestsTransportSHMUDP.cpp index cbe06dba4b2..20cc3129778 100644 --- a/test/blackbox/common/DDSBlackboxTestsTransportSHMUDP.cpp +++ b/test/blackbox/common/DDSBlackboxTestsTransportSHMUDP.cpp @@ -13,7 +13,9 @@ // limitations under the License. #include "BlackboxTests.hpp" +#include "mock/BlackboxMockConsumer.h" +#include #include #include #include @@ -178,6 +180,88 @@ TEST_P(SHMUDP, Transport_Reliable_Reliable_test) run_parametrized_test(true, true); } +static bool has_shm_locators( + const ResourceLimitedVector& locators) +{ + auto loc_is_shm = [](const Locator_t& loc) + { + return LOCATOR_KIND_SHM == loc.kind; + }; + return std::any_of(locators.cbegin(), locators.cend(), loc_is_shm); +} + +static void check_shm_locators( + const eprosima::fastrtps::rtps::ParticipantDiscoveryInfo& info, + bool unicast, + bool multicast) +{ + EXPECT_EQ(multicast, has_shm_locators(info.info.metatraffic_locators.multicast)); + EXPECT_EQ(unicast, has_shm_locators(info.info.metatraffic_locators.unicast)); +} + +static void shm_metatraffic_test( + const std::string& topic_name, + const char* const value, + bool unicast, + bool multicast) +{ + PubSubWriter writer(topic_name + "/" + value); + PubSubReader reader(topic_name + "/" + value); + + auto discovery_checker = [unicast, multicast](const eprosima::fastrtps::rtps::ParticipantDiscoveryInfo& info) + { + check_shm_locators(info, unicast, multicast); + return true; + }; + reader.setOnDiscoveryFunction(discovery_checker); + reader.max_multicast_locators_number(2); + reader.init(); + ASSERT_TRUE(reader.isInitialized()); + + PropertyPolicy properties; + Property p; + p.name("fastdds.shm.enforce_metatraffic"); + p.value(value); + properties.properties().push_back(p); + writer.property_policy(properties).avoid_builtin_multicast(false).max_multicast_locators_number(2); + writer.init(); + ASSERT_TRUE(writer.isInitialized()); + + reader.wait_discovery(); + writer.wait_discovery(); +} + +TEST(SHMUDP, SHM_metatraffic_config) +{ + shm_metatraffic_test(TEST_TOPIC_NAME, "none", false, false); + shm_metatraffic_test(TEST_TOPIC_NAME, "unicast", true, false); + shm_metatraffic_test(TEST_TOPIC_NAME, "all", true, true); +} + +TEST(SHMUDP, SHM_metatraffic_wrong_config) +{ + using eprosima::fastdds::dds::BlackboxMockConsumer; + + /* Set up log */ + BlackboxMockConsumer* helper_consumer = new BlackboxMockConsumer(); + Log::ClearConsumers(); // Remove default consumers + Log::RegisterConsumer(std::unique_ptr(helper_consumer)); // Registering a consumer transfer ownership + // Filter specific message + Log::SetVerbosity(Log::Kind::Warning); + Log::SetCategoryFilter(std::regex("RTPS_NETWORK")); + Log::SetErrorStringFilter(std::regex(".*__WRONG_VALUE__.*")); + + // Perform test + shm_metatraffic_test(TEST_TOPIC_NAME, "__WRONG_VALUE__", false, false); + + /* Check logs */ + Log::Flush(); + EXPECT_EQ(helper_consumer->ConsumedEntries().size(), 1u); + + /* Clean-up */ + Log::Reset(); // This calls to ClearConsumers, which deletes the registered consumer +} + #ifdef INSTANTIATE_TEST_SUITE_P #define GTEST_INSTANTIATE_TEST_MACRO(x, y, z, w) INSTANTIATE_TEST_SUITE_P(x, y, z, w) #else diff --git a/test/unittest/rtps/network/CMakeLists.txt b/test/unittest/rtps/network/CMakeLists.txt index 7a2f36d7c86..9434682004f 100644 --- a/test/unittest/rtps/network/CMakeLists.txt +++ b/test/unittest/rtps/network/CMakeLists.txt @@ -22,6 +22,10 @@ set(NETWORKFACTORYTESTS_SOURCE ${PROJECT_SOURCE_DIR}/src/cpp/fastdds/log/StdoutErrConsumer.cpp ${PROJECT_SOURCE_DIR}/src/cpp/rtps/common/Time_t.cpp + ${PROJECT_SOURCE_DIR}/src/cpp/rtps/attributes/PropertyPolicy.cpp + + ${PROJECT_SOURCE_DIR}/src/cpp/rtps/flowcontrol/ThroughputControllerDescriptor.cpp + ${PROJECT_SOURCE_DIR}/src/cpp/rtps/transport/ChannelResource.cpp ${PROJECT_SOURCE_DIR}/src/cpp/rtps/transport/UDPChannelResource.cpp ${PROJECT_SOURCE_DIR}/src/cpp/rtps/transport/UDPTransportInterface.cpp diff --git a/test/unittest/rtps/network/NetworkFactoryTests.cpp b/test/unittest/rtps/network/NetworkFactoryTests.cpp index 72ddae4949e..536547d872e 100644 --- a/test/unittest/rtps/network/NetworkFactoryTests.cpp +++ b/test/unittest/rtps/network/NetworkFactoryTests.cpp @@ -16,6 +16,7 @@ #include +#include #include #include #include @@ -34,7 +35,8 @@ class NetworkTests : public ::testing::Test { public: - NetworkFactory networkFactoryUnderTest; + RTPSParticipantAttributes pattr{}; + NetworkFactory networkFactoryUnderTest{pattr}; void HELPER_RegisterTransportWithKindAndChannels( int kind, unsigned int channels); @@ -648,7 +650,7 @@ TEST_F(NetworkTests, LocatorShrink) std::vector test_cases; fill_blackbox_locators_test_cases(test_cases); - NetworkFactory f; + NetworkFactory f{pattr}; UDPv4TransportDescriptor udpv4; f.RegisterTransport(&udpv4); // TODO: Register more transports diff --git a/test/unittest/transport/CMakeLists.txt b/test/unittest/transport/CMakeLists.txt index 1ef6441f393..a54270c19d1 100644 --- a/test/unittest/transport/CMakeLists.txt +++ b/test/unittest/transport/CMakeLists.txt @@ -53,6 +53,8 @@ set(UDPV4TESTS_SOURCE ${PROJECT_SOURCE_DIR}/src/cpp/fastdds/log/OStreamConsumer.cpp ${PROJECT_SOURCE_DIR}/src/cpp/fastdds/log/StdoutConsumer.cpp ${PROJECT_SOURCE_DIR}/src/cpp/fastdds/log/StdoutErrConsumer.cpp + ${PROJECT_SOURCE_DIR}/src/cpp/rtps/attributes/PropertyPolicy.cpp + ${PROJECT_SOURCE_DIR}/src/cpp/rtps/flowcontrol/ThroughputControllerDescriptor.cpp ${PROJECT_SOURCE_DIR}/src/cpp/rtps/transport/UDPv4Transport.cpp ${PROJECT_SOURCE_DIR}/src/cpp/rtps/transport/UDPTransportInterface.cpp ${PROJECT_SOURCE_DIR}/src/cpp/rtps/transport/ChannelResource.cpp @@ -70,7 +72,9 @@ set(UDPV6TESTS_SOURCE ${PROJECT_SOURCE_DIR}/src/cpp/fastdds/log/OStreamConsumer.cpp ${PROJECT_SOURCE_DIR}/src/cpp/fastdds/log/StdoutConsumer.cpp ${PROJECT_SOURCE_DIR}/src/cpp/fastdds/log/StdoutErrConsumer.cpp + ${PROJECT_SOURCE_DIR}/src/cpp/rtps/attributes/PropertyPolicy.cpp ${PROJECT_SOURCE_DIR}/src/cpp/rtps/common/Time_t.cpp + ${PROJECT_SOURCE_DIR}/src/cpp/rtps/flowcontrol/ThroughputControllerDescriptor.cpp ${PROJECT_SOURCE_DIR}/src/cpp/rtps/network/NetworkFactory.cpp ${PROJECT_SOURCE_DIR}/src/cpp/rtps/transport/ChannelResource.cpp ${PROJECT_SOURCE_DIR}/src/cpp/rtps/transport/UDPChannelResource.cpp @@ -91,7 +95,9 @@ set(TCPV4TESTS_SOURCE ${PROJECT_SOURCE_DIR}/src/cpp/fastdds/log/OStreamConsumer.cpp ${PROJECT_SOURCE_DIR}/src/cpp/fastdds/log/StdoutConsumer.cpp ${PROJECT_SOURCE_DIR}/src/cpp/fastdds/log/StdoutErrConsumer.cpp + ${PROJECT_SOURCE_DIR}/src/cpp/rtps/attributes/PropertyPolicy.cpp ${PROJECT_SOURCE_DIR}/src/cpp/rtps/common/Time_t.cpp + ${PROJECT_SOURCE_DIR}/src/cpp/rtps/flowcontrol/ThroughputControllerDescriptor.cpp ${PROJECT_SOURCE_DIR}/src/cpp/rtps/messages/RTPSMessageCreator.cpp ${PROJECT_SOURCE_DIR}/src/cpp/rtps/network/NetworkFactory.cpp ${PROJECT_SOURCE_DIR}/src/cpp/rtps/resources/ResourceEvent.cpp @@ -133,7 +139,9 @@ set(TCPV6TESTS_SOURCE ${PROJECT_SOURCE_DIR}/src/cpp/fastdds/log/OStreamConsumer.cpp ${PROJECT_SOURCE_DIR}/src/cpp/fastdds/log/StdoutConsumer.cpp ${PROJECT_SOURCE_DIR}/src/cpp/fastdds/log/StdoutErrConsumer.cpp + ${PROJECT_SOURCE_DIR}/src/cpp/rtps/attributes/PropertyPolicy.cpp ${PROJECT_SOURCE_DIR}/src/cpp/rtps/common/Time_t.cpp + ${PROJECT_SOURCE_DIR}/src/cpp/rtps/flowcontrol/ThroughputControllerDescriptor.cpp ${PROJECT_SOURCE_DIR}/src/cpp/rtps/messages/RTPSMessageCreator.cpp ${PROJECT_SOURCE_DIR}/src/cpp/rtps/network/NetworkFactory.cpp ${PROJECT_SOURCE_DIR}/src/cpp/rtps/resources/ResourceEvent.cpp @@ -177,6 +185,8 @@ set(TEST_UDPV4TESTS_SOURCE ${PROJECT_SOURCE_DIR}/src/cpp/fastdds/core/policy/ParameterList.cpp ${PROJECT_SOURCE_DIR}/src/cpp/utils/IPFinder.cpp ${PROJECT_SOURCE_DIR}/src/cpp/utils/IPLocator.cpp + ${PROJECT_SOURCE_DIR}/src/cpp/rtps/attributes/PropertyPolicy.cpp + ${PROJECT_SOURCE_DIR}/src/cpp/rtps/flowcontrol/ThroughputControllerDescriptor.cpp ${PROJECT_SOURCE_DIR}/src/cpp/rtps/transport/test_UDPv4Transport.cpp ${PROJECT_SOURCE_DIR}/src/cpp/rtps/transport/UDPChannelResource.cpp ${PROJECT_SOURCE_DIR}/src/cpp/rtps/transport/UDPTransportInterface.cpp @@ -196,7 +206,9 @@ set(SHAREDMEMTESTS_SOURCE ${PROJECT_SOURCE_DIR}/src/cpp/fastdds/log/OStreamConsumer.cpp ${PROJECT_SOURCE_DIR}/src/cpp/fastdds/log/StdoutConsumer.cpp ${PROJECT_SOURCE_DIR}/src/cpp/fastdds/log/StdoutErrConsumer.cpp + ${PROJECT_SOURCE_DIR}/src/cpp/rtps/attributes/PropertyPolicy.cpp ${PROJECT_SOURCE_DIR}/src/cpp/rtps/common/Time_t.cpp + ${PROJECT_SOURCE_DIR}/src/cpp/rtps/flowcontrol/ThroughputControllerDescriptor.cpp ${PROJECT_SOURCE_DIR}/src/cpp/rtps/network/NetworkFactory.cpp ${PROJECT_SOURCE_DIR}/src/cpp/rtps/transport/shared_mem/SharedMemTransport.cpp ${PROJECT_SOURCE_DIR}/src/cpp/rtps/transport/shared_mem/SharedMemTransportDescriptor.cpp diff --git a/test/unittest/transport/TCPv6Tests.cpp b/test/unittest/transport/TCPv6Tests.cpp index decf4409227..57fb33ae97f 100644 --- a/test/unittest/transport/TCPv6Tests.cpp +++ b/test/unittest/transport/TCPv6Tests.cpp @@ -19,6 +19,7 @@ #include #include +#include #include #include #include @@ -151,7 +152,8 @@ TEST_F(TCPv6Tests, opening_and_closing_input_channel) multicastFilterLocator.port = g_default_port; // arbitrary IPLocator::setIPv6(multicastFilterLocator, 0xff31, 0, 0, 0, 0, 0, 0x8000, 0x1234); - NetworkFactory factory; + RTPSParticipantAttributes p_attr{}; + NetworkFactory factory{p_attr}; factory.RegisterTransport(descriptor); std::vector> receivers; factory.BuildReceiverResources(multicastFilterLocator, receivers, 0x8FFF); diff --git a/versions.md b/versions.md index 7bd19f2948b..1b38209c725 100644 --- a/versions.md +++ b/versions.md @@ -1,6 +1,9 @@ Forthcoming ----------- +* Added participant property to configure SHM transport metatraffic behavior. + No metatraffic over SHM transport by default. + Version 2.11.0 -------------- From 99a52fb65057788909066f4e6aabd1ffe141d453 Mon Sep 17 00:00:00 2001 From: Eduardo Ponz Segrelles Date: Thu, 3 Aug 2023 13:40:51 +0200 Subject: [PATCH 18/23] Allow participant profiles with no rtps tag (#3771) * Refs #19315: Allow participant profiles with no rtps tag Signed-off-by: Eduardo Ponz * Refs #19315: Apply suggestions Signed-off-by: Eduardo Ponz * Refs #19315: Apply suggestions 2 Signed-off-by: Eduardo Ponz --------- Signed-off-by: Eduardo Ponz Signed-off-by: jimwang118 --- src/cpp/rtps/xmlparser/XMLParser.cpp | 59 +++++++++++++------ test/unittest/xmlparser/XMLParserTests.cpp | 17 ++---- .../simple_participant_profiles_nok.xml | 14 +++++ .../simple_participant_profiles_ok.xml | 24 ++++++++ 4 files changed, 86 insertions(+), 28 deletions(-) create mode 100644 test/unittest/xmlparser/regressions/simple_participant_profiles_nok.xml create mode 100644 test/unittest/xmlparser/regressions/simple_participant_profiles_ok.xml diff --git a/src/cpp/rtps/xmlparser/XMLParser.cpp b/src/cpp/rtps/xmlparser/XMLParser.cpp index 9eba6315e91..f82b731d891 100644 --- a/src/cpp/rtps/xmlparser/XMLParser.cpp +++ b/src/cpp/rtps/xmlparser/XMLParser.cpp @@ -1771,39 +1771,64 @@ XMLP_ret XMLParser::fillDataNode( addAllAttributes(p_profile, participant_node); uint8_t ident = 1; - tinyxml2::XMLElement* p_element = p_profile->FirstChildElement(DOMAIN_ID); - if (nullptr != p_element) + tinyxml2::XMLElement* p_element; + tinyxml2::XMLElement* p_aux0 = nullptr; + const char* name = nullptr; + std::set tags_present; + + /* + * The only allowed elements are and + * - The min occurrences of are 0, and its max is 1; look for it. + * - The min occurrences of are 0, and its max is 1; look for it. + */ + for (p_element = p_profile->FirstChildElement(); p_element != nullptr; p_element = p_element->NextSiblingElement()) { - // domainId - uint32Type - if (XMLP_ret::XML_OK != getXMLUint(p_element, &participant_node.get()->domainId, ident)) + name = p_element->Name(); + if (tags_present.count(name) != 0) { + EPROSIMA_LOG_ERROR(XMLPARSER, "Duplicated element found in 'participant'. Tag: " << name); + return XMLP_ret::XML_ERROR; + } + tags_present.emplace(name); + + if (strcmp(p_element->Name(), DOMAIN_ID) == 0) + { + // domainId - uint32Type + if (XMLP_ret::XML_OK != getXMLUint(p_element, &participant_node.get()->domainId, ident)) + { + return XMLP_ret::XML_ERROR; + } + } + else if (strcmp(p_element->Name(), RTPS) == 0) + { + p_aux0 = p_element; + } + else + { + EPROSIMA_LOG_ERROR(XMLPARSER, "Found incorrect tag '" << p_element->Name() << "'"); return XMLP_ret::XML_ERROR; } } + tags_present.clear(); - p_element = p_profile->FirstChildElement(RTPS); - if (nullptr == p_element) + // is not present, but that's OK + if (nullptr == p_aux0) { - EPROSIMA_LOG_ERROR(XMLPARSER, "Not found '" << RTPS << "' tag"); - return XMLP_ret::XML_ERROR; + return XMLP_ret::XML_OK; } - tinyxml2::XMLElement* p_aux0 = nullptr; - const char* name = nullptr; - - std::unordered_map tags_present; - - for (p_aux0 = p_element->FirstChildElement(); p_aux0 != nullptr; p_aux0 = p_aux0->NextSiblingElement()) + // Check contents of + for (p_aux0 = p_aux0->FirstChildElement(); p_aux0 != nullptr; p_aux0 = p_aux0->NextSiblingElement()) { name = p_aux0->Name(); - if (tags_present[name]) + if (tags_present.count(name) != 0) { EPROSIMA_LOG_ERROR(XMLPARSER, - "Duplicated element found in 'rtpsParticipantAttributesType'. Name: " << name); + "Duplicated element found in 'rtpsParticipantAttributesType'. Tag: " << name); return XMLP_ret::XML_ERROR; } - tags_present[name] = true; + tags_present.emplace(name); if (strcmp(name, ALLOCATION) == 0) { diff --git a/test/unittest/xmlparser/XMLParserTests.cpp b/test/unittest/xmlparser/XMLParserTests.cpp index 2c58d8fb315..d210e092042 100644 --- a/test/unittest/xmlparser/XMLParserTests.cpp +++ b/test/unittest/xmlparser/XMLParserTests.cpp @@ -61,6 +61,8 @@ TEST_F(XMLParserTests, regressions) EXPECT_EQ(XMLP_ret::XML_ERROR, XMLParser::loadXML("regressions/14456_profile_bin.xml", root)); EXPECT_EQ(XMLP_ret::XML_ERROR, XMLParser::loadXML("regressions/15344_profile_bin.xml", root)); EXPECT_EQ(XMLP_ret::XML_ERROR, XMLParser::loadXML("regressions/18395_profile_bin.xml", root)); + EXPECT_EQ(XMLP_ret::XML_ERROR, XMLParser::loadXML("regressions/simple_participant_profiles_nok.xml", root)); + EXPECT_EQ(XMLP_ret::XML_OK, XMLParser::loadXML("regressions/simple_participant_profiles_ok.xml", root)); } TEST_F(XMLParserTests, NoFile) @@ -1611,10 +1613,9 @@ TEST_F(XMLParserTests, parseLogConfig) /* * This test checks the return of the negative cases of the fillDataNode given a ParticipantAttributes DataNode * 1. Check passing a nullptr as if the XMLElement was wrongly parsed above - * 2. Check missing required rtps tag - * 3. Check missing DomainId value in tag - * 4. Check bad values for all attributes - * 5. Check a non existant attribute tag + * 2. Check missing DomainId value in tag + * 3. Check bad values for all attributes + * 4. Check a non existant attribute tag */ TEST_F(XMLParserTests, fillDataNodeParticipantNegativeClauses) { @@ -1636,12 +1637,6 @@ TEST_F(XMLParserTests, fillDataNodeParticipantNegativeClauses) "; char xml[500]; - // Misssing rtps tag - sprintf(xml, xml_p, "0"); - ASSERT_EQ(tinyxml2::XMLError::XML_SUCCESS, xml_doc.Parse(xml)); - titleElement = xml_doc.RootElement(); - EXPECT_EQ(XMLP_ret::XML_ERROR, XMLParserTest::fillDataNode_wrapper(titleElement, *participant_node)); - // Misssing DomainId Value sprintf(xml, xml_p, ""); ASSERT_EQ(tinyxml2::XMLError::XML_SUCCESS, xml_doc.Parse(xml)); @@ -1759,6 +1754,7 @@ TEST_F(XMLParserTests, parseXMLProfilesRoot) // , , , , and are read as xml child elements of the // root element. std::vector elements_ok { + "participant", "publisher", "data_writer", "subscriber", @@ -1776,7 +1772,6 @@ TEST_F(XMLParserTests, parseXMLProfilesRoot) std::vector elements_error { "library_settings", - "participant", "requester", "replier" }; diff --git a/test/unittest/xmlparser/regressions/simple_participant_profiles_nok.xml b/test/unittest/xmlparser/regressions/simple_participant_profiles_nok.xml new file mode 100644 index 00000000000..6c8f945c202 --- /dev/null +++ b/test/unittest/xmlparser/regressions/simple_participant_profiles_nok.xml @@ -0,0 +1,14 @@ + + + + + 1 + 2 + + + + + + + + diff --git a/test/unittest/xmlparser/regressions/simple_participant_profiles_ok.xml b/test/unittest/xmlparser/regressions/simple_participant_profiles_ok.xml new file mode 100644 index 00000000000..ef1729dde01 --- /dev/null +++ b/test/unittest/xmlparser/regressions/simple_participant_profiles_ok.xml @@ -0,0 +1,24 @@ + + + + + + + + + + + + + + + + 1 + + + + 1 + + + + From bc7c1a7974d74cbed8810a4ff12802c5c7ccfb3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20Poderoso?= <120394830+JesusPoderoso@users.noreply.github.com> Date: Thu, 3 Aug 2023 14:00:08 +0200 Subject: [PATCH 19/23] Custom pools on DDS layer feature (#3755) * Custom Payload pools test implementation (#3719) * Refs #19024: Public API implementation Signed-off-by: JesusPoderoso * Refs #19024: Update versions.md Signed-off-by: JesusPoderoso * Refs #19023: Fix build issues Signed-off-by: JesusPoderoso * Refs #19023: Custom Payload pools test implementation Signed-off-by: Javier Santiago * Refs #19023: Update test to use public API Signed-off-by: JesusPoderoso * Refs #19023: Please linters Signed-off-by: JesusPoderoso * Refs #19023: Added delay between writing and checking payload request Signed-off-by: Javier Santiago --------- Signed-off-by: JesusPoderoso Signed-off-by: Javier Santiago Co-authored-by: JesusPoderoso Signed-off-by: Eduardo Ponz * Include custom pools impl (#3740) Signed-off-by: JesusPoderoso Signed-off-by: Eduardo Ponz * Refs #19024: Modified custom payload pool and datasharing interaction Signed-off-by: Javier Santiago Signed-off-by: Eduardo Ponz * Refs #19024. Correctly set payload owner on test pools. Signed-off-by: Miguel Company Signed-off-by: Eduardo Ponz --------- Signed-off-by: JesusPoderoso Signed-off-by: Javier Santiago Signed-off-by: Eduardo Ponz Signed-off-by: Miguel Company Co-authored-by: jsantiago-eProsima <90755661+jsantiago-eProsima@users.noreply.github.com> Co-authored-by: Javier Santiago Co-authored-by: Miguel Company Signed-off-by: jimwang118 --- include/fastdds/dds/publisher/Publisher.hpp | 8 +- include/fastdds/dds/subscriber/Subscriber.hpp | 8 +- src/cpp/fastdds/publisher/DataWriterImpl.cpp | 21 +++- src/cpp/fastdds/publisher/DataWriterImpl.hpp | 5 +- src/cpp/fastdds/publisher/Publisher.cpp | 10 +- src/cpp/fastdds/publisher/PublisherImpl.cpp | 15 ++- src/cpp/fastdds/publisher/PublisherImpl.hpp | 9 +- src/cpp/fastdds/subscriber/DataReaderImpl.cpp | 31 ++++-- src/cpp/fastdds/subscriber/DataReaderImpl.hpp | 7 +- src/cpp/fastdds/subscriber/Subscriber.cpp | 10 +- src/cpp/fastdds/subscriber/SubscriberImpl.cpp | 15 ++- src/cpp/fastdds/subscriber/SubscriberImpl.hpp | 9 +- .../fastdds/publisher/DataWriterImpl.hpp | 3 +- .../fastdds/publisher/PublisherImpl.hpp | 5 +- .../fastdds/subscriber/DataReaderImpl.hpp | 3 +- .../fastdds/subscriber/SubscriberImpl.hpp | 5 +- .../api/dds-pim/CustomPayloadPool.hpp | 104 ++++++++++++++++++ .../blackbox/common/DDSBlackboxTestsBasic.cpp | 62 +++++++++++ test/unittest/common/CustomPayloadPool.hpp | 104 ++++++++++++++++++ .../dds/publisher/DataWriterTests.cpp | 54 +++++++++ .../dds/subscriber/DataReaderTests.cpp | 56 ++++++++++ .../fastdds/publisher/PublisherImpl.hpp | 5 +- versions.md | 1 + 23 files changed, 500 insertions(+), 50 deletions(-) create mode 100644 test/blackbox/api/dds-pim/CustomPayloadPool.hpp create mode 100644 test/unittest/common/CustomPayloadPool.hpp diff --git a/include/fastdds/dds/publisher/Publisher.hpp b/include/fastdds/dds/publisher/Publisher.hpp index 8f6e1a40838..5006e144d9b 100644 --- a/include/fastdds/dds/publisher/Publisher.hpp +++ b/include/fastdds/dds/publisher/Publisher.hpp @@ -155,13 +155,15 @@ class Publisher : public DomainEntity * @param qos QoS of the DataWriter. * @param listener Pointer to the listener (default: nullptr). * @param mask StatusMask that holds statuses the listener responds to (default: all). + * @param payload_pool IPayloadPool shared pointer that defines writer payload (default: nullptr). * @return Pointer to the created DataWriter. nullptr if failed. */ RTPS_DllAPI DataWriter* create_datawriter( Topic* topic, const DataWriterQos& qos, DataWriterListener* listener = nullptr, - const StatusMask& mask = StatusMask::all()); + const StatusMask& mask = StatusMask::all(), + std::shared_ptr payload_pool = nullptr); /** * This operation creates a DataWriter. The returned DataWriter will be attached and belongs to the Publisher. @@ -170,13 +172,15 @@ class Publisher : public DomainEntity * @param profile_name DataWriter profile name. * @param listener Pointer to the listener (default: nullptr). * @param mask StatusMask that holds statuses the listener responds to (default: all). + * @param payload_pool IPayloadPool shared pointer that defines writer payload (default: nullptr). * @return Pointer to the created DataWriter. nullptr if failed. */ RTPS_DllAPI DataWriter* create_datawriter_with_profile( Topic* topic, const std::string& profile_name, DataWriterListener* listener = nullptr, - const StatusMask& mask = StatusMask::all()); + const StatusMask& mask = StatusMask::all(), + std::shared_ptr payload_pool = nullptr); /** * This operation deletes a DataWriter that belongs to the Publisher. diff --git a/include/fastdds/dds/subscriber/Subscriber.hpp b/include/fastdds/dds/subscriber/Subscriber.hpp index 337ace92387..5cb5e21e55e 100644 --- a/include/fastdds/dds/subscriber/Subscriber.hpp +++ b/include/fastdds/dds/subscriber/Subscriber.hpp @@ -162,13 +162,15 @@ class Subscriber : public DomainEntity * @param reader_qos QoS of the DataReader. * @param listener Pointer to the listener (default: nullptr) * @param mask StatusMask that holds statuses the listener responds to (default: all). + * @param payload_pool IPayloadPool shared pointer that defines reader payload (default: nullptr). * @return Pointer to the created DataReader. nullptr if failed. */ RTPS_DllAPI DataReader* create_datareader( TopicDescription* topic, const DataReaderQos& reader_qos, DataReaderListener* listener = nullptr, - const StatusMask& mask = StatusMask::all()); + const StatusMask& mask = StatusMask::all(), + std::shared_ptr payload_pool = nullptr); /** * This operation creates a DataReader. The returned DataReader will be attached and belongs to the Subscriber. @@ -177,13 +179,15 @@ class Subscriber : public DomainEntity * @param profile_name DataReader profile name. * @param listener Pointer to the listener (default: nullptr) * @param mask StatusMask that holds statuses the listener responds to (default: all). + * @param payload_pool IPayloadPool shared pointer that defines reader payload (default: nullptr). * @return Pointer to the created DataReader. nullptr if failed. */ RTPS_DllAPI DataReader* create_datareader_with_profile( TopicDescription* topic, const std::string& profile_name, DataReaderListener* listener = nullptr, - const StatusMask& mask = StatusMask::all()); + const StatusMask& mask = StatusMask::all(), + std::shared_ptr payload_pool = nullptr); /** * This operation deletes a DataReader that belongs to the Subscriber. diff --git a/src/cpp/fastdds/publisher/DataWriterImpl.cpp b/src/cpp/fastdds/publisher/DataWriterImpl.cpp index a1d6d607c71..42bed2a9f51 100644 --- a/src/cpp/fastdds/publisher/DataWriterImpl.cpp +++ b/src/cpp/fastdds/publisher/DataWriterImpl.cpp @@ -146,7 +146,8 @@ DataWriterImpl::DataWriterImpl( TypeSupport type, Topic* topic, const DataWriterQos& qos, - DataWriterListener* listen) + DataWriterListener* listen, + std::shared_ptr payload_pool) : publisher_(p) , type_(type) , topic_(topic) @@ -174,6 +175,12 @@ DataWriterImpl::DataWriterImpl( fastrtps::rtps::RTPSParticipantImpl::preprocess_endpoint_attributes( EntityId_t::unknown(), publisher_->get_participant_impl()->id_counter(), endpoint_attributes, guid_.entityId); guid_.guidPrefix = publisher_->get_participant_impl()->guid().guidPrefix; + + if (payload_pool != nullptr) + { + is_custom_payload_pool_ = true; + payload_pool_ = payload_pool; + } } DataWriterImpl::DataWriterImpl( @@ -1939,7 +1946,7 @@ bool DataWriterImpl::release_payload_pool() bool result = true; - if (is_data_sharing_compatible_) + if (is_data_sharing_compatible_ || is_custom_payload_pool_) { // No-op } @@ -1994,6 +2001,11 @@ ReturnCode_t DataWriterImpl::check_datasharing_compatible( return ReturnCode_t::RETCODE_OK; break; case DataSharingKind::ON: + if (is_custom_payload_pool_) + { + EPROSIMA_LOG_ERROR(DATA_WRITER, "Custom payload pool detected. Cannot force Data sharing usage."); + return ReturnCode_t::RETCODE_INCONSISTENT_POLICY; + } #if HAVE_SECURITY if (has_security_enabled) { @@ -2019,6 +2031,11 @@ ReturnCode_t DataWriterImpl::check_datasharing_compatible( return ReturnCode_t::RETCODE_OK; break; case DataSharingKind::AUTO: + if (is_custom_payload_pool_) + { + EPROSIMA_LOG_INFO(DATA_WRITER, "Custom payload pool detected. Data Sharing disabled."); + return ReturnCode_t::RETCODE_OK; + } #if HAVE_SECURITY if (has_security_enabled) { diff --git a/src/cpp/fastdds/publisher/DataWriterImpl.hpp b/src/cpp/fastdds/publisher/DataWriterImpl.hpp index c9ef7699876..7c5dbcf9ded 100644 --- a/src/cpp/fastdds/publisher/DataWriterImpl.hpp +++ b/src/cpp/fastdds/publisher/DataWriterImpl.hpp @@ -108,7 +108,8 @@ class DataWriterImpl : protected rtps::IReaderDataFilter TypeSupport type, Topic* topic, const DataWriterQos& qos, - DataWriterListener* listener = nullptr); + DataWriterListener* listener = nullptr, + std::shared_ptr payload_pool = nullptr); DataWriterImpl( PublisherImpl* p, @@ -488,6 +489,8 @@ class DataWriterImpl : protected rtps::IReaderDataFilter std::shared_ptr payload_pool_; + bool is_custom_payload_pool_ = false; + std::unique_ptr loans_; fastrtps::rtps::GUID_t guid_; diff --git a/src/cpp/fastdds/publisher/Publisher.cpp b/src/cpp/fastdds/publisher/Publisher.cpp index 3aded76fc39..0cc285cd0ff 100644 --- a/src/cpp/fastdds/publisher/Publisher.cpp +++ b/src/cpp/fastdds/publisher/Publisher.cpp @@ -112,18 +112,20 @@ DataWriter* Publisher::create_datawriter( Topic* topic, const DataWriterQos& qos, DataWriterListener* listener, - const StatusMask& mask) + const StatusMask& mask, + std::shared_ptr payload_pool) { - return impl_->create_datawriter(topic, qos, listener, mask); + return impl_->create_datawriter(topic, qos, listener, mask, payload_pool); } DataWriter* Publisher::create_datawriter_with_profile( Topic* topic, const std::string& profile_name, DataWriterListener* listener, - const StatusMask& mask) + const StatusMask& mask, + std::shared_ptr payload_pool) { - return impl_->create_datawriter_with_profile(topic, profile_name, listener, mask); + return impl_->create_datawriter_with_profile(topic, profile_name, listener, mask, payload_pool); } ReturnCode_t Publisher::delete_datawriter( diff --git a/src/cpp/fastdds/publisher/PublisherImpl.cpp b/src/cpp/fastdds/publisher/PublisherImpl.cpp index d99335a204a..bcdd2b11993 100644 --- a/src/cpp/fastdds/publisher/PublisherImpl.cpp +++ b/src/cpp/fastdds/publisher/PublisherImpl.cpp @@ -206,16 +206,18 @@ DataWriterImpl* PublisherImpl::create_datawriter_impl( const TypeSupport& type, Topic* topic, const DataWriterQos& qos, - DataWriterListener* listener) + DataWriterListener* listener, + std::shared_ptr payload_pool) { - return new DataWriterImpl(this, type, topic, qos, listener); + return new DataWriterImpl(this, type, topic, qos, listener, payload_pool); } DataWriter* PublisherImpl::create_datawriter( Topic* topic, const DataWriterQos& qos, DataWriterListener* listener, - const StatusMask& mask) + const StatusMask& mask, + std::shared_ptr payload_pool) { EPROSIMA_LOG_INFO(PUBLISHER, "CREATING WRITER IN TOPIC: " << topic->get_name()); //Look for the correct type registration @@ -234,7 +236,7 @@ DataWriter* PublisherImpl::create_datawriter( return nullptr; } - DataWriterImpl* impl = create_datawriter_impl(type_support, topic, qos, listener); + DataWriterImpl* impl = create_datawriter_impl(type_support, topic, qos, listener, payload_pool); return create_datawriter(topic, impl, mask); } @@ -269,7 +271,8 @@ DataWriter* PublisherImpl::create_datawriter_with_profile( Topic* topic, const std::string& profile_name, DataWriterListener* listener, - const StatusMask& mask) + const StatusMask& mask, + std::shared_ptr payload_pool) { // TODO (ILG): Change when we have full XML support for DDS QoS profiles PublisherAttributes attr; @@ -277,7 +280,7 @@ DataWriter* PublisherImpl::create_datawriter_with_profile( { DataWriterQos qos = default_datawriter_qos_; utils::set_qos_from_attributes(qos, attr); - return create_datawriter(topic, qos, listener, mask); + return create_datawriter(topic, qos, listener, mask, payload_pool); } return nullptr; diff --git a/src/cpp/fastdds/publisher/PublisherImpl.hpp b/src/cpp/fastdds/publisher/PublisherImpl.hpp index 7561023e059..183413cff49 100644 --- a/src/cpp/fastdds/publisher/PublisherImpl.hpp +++ b/src/cpp/fastdds/publisher/PublisherImpl.hpp @@ -105,13 +105,15 @@ class PublisherImpl Topic* topic, const DataWriterQos& qos, DataWriterListener* listener, - const StatusMask& mask = StatusMask::all()); + const StatusMask& mask = StatusMask::all(), + std::shared_ptr payload_pool = nullptr); DataWriter* create_datawriter_with_profile( Topic* topic, const std::string& profile_name, DataWriterListener* listener, - const StatusMask& mask = StatusMask::all()); + const StatusMask& mask = StatusMask::all(), + std::shared_ptr payload_pool = nullptr); ReturnCode_t delete_datawriter( const DataWriter* writer); @@ -255,7 +257,8 @@ class PublisherImpl const TypeSupport& type, Topic* topic, const DataWriterQos& qos, - DataWriterListener* listener); + DataWriterListener* listener, + std::shared_ptr payload_pool); static void set_qos( PublisherQos& to, diff --git a/src/cpp/fastdds/subscriber/DataReaderImpl.cpp b/src/cpp/fastdds/subscriber/DataReaderImpl.cpp index 11d36614c76..5346fcbd0cc 100644 --- a/src/cpp/fastdds/subscriber/DataReaderImpl.cpp +++ b/src/cpp/fastdds/subscriber/DataReaderImpl.cpp @@ -102,7 +102,8 @@ DataReaderImpl::DataReaderImpl( const TypeSupport& type, TopicDescription* topic, const DataReaderQos& qos, - DataReaderListener* listener) + DataReaderListener* listener, + std::shared_ptr payload_pool) : subscriber_(s) , type_(type) , topic_(topic) @@ -124,6 +125,12 @@ DataReaderImpl::DataReaderImpl( RTPSParticipantImpl::preprocess_endpoint_attributes( EntityId_t::unknown(), subscriber_->get_participant_impl()->id_counter(), endpoint_attributes, guid_.entityId); guid_.guidPrefix = subscriber_->get_participant_impl()->guid().guidPrefix; + + if (payload_pool != nullptr) + { + is_custom_payload_pool_ = true; + payload_pool_ = payload_pool; + } } ReturnCode_t DataReaderImpl::enable() @@ -1717,13 +1724,17 @@ std::shared_ptr DataReaderImpl::get_payload_pool() PoolConfig config = PoolConfig::from_history_attributes(history_.m_att); - if (!payload_pool_) + if (!sample_pool_) { - payload_pool_ = TopicPayloadPoolRegistry::get(topic_->get_impl()->get_rtps_topic_name(), config); sample_pool_ = std::make_shared(config, type_); } - - payload_pool_->reserve_history(config, true); + if (!is_custom_payload_pool_) + { + std::shared_ptr topic_payload_pool = TopicPayloadPoolRegistry::get( + topic_->get_impl()->get_rtps_topic_name(), config); + topic_payload_pool->reserve_history(config, true); + payload_pool_ = topic_payload_pool; + } return payload_pool_; } @@ -1731,8 +1742,14 @@ void DataReaderImpl::release_payload_pool() { assert(payload_pool_); - PoolConfig config = PoolConfig::from_history_attributes(history_.m_att); - payload_pool_->release_history(config, true); + if (!is_custom_payload_pool_) + { + PoolConfig config = PoolConfig::from_history_attributes(history_.m_att); + std::shared_ptr topic_payload_pool = + std::dynamic_pointer_cast(payload_pool_); + topic_payload_pool->release_history(config, true); + } + payload_pool_.reset(); } diff --git a/src/cpp/fastdds/subscriber/DataReaderImpl.hpp b/src/cpp/fastdds/subscriber/DataReaderImpl.hpp index 7b261a69bbb..faed35ec4e0 100644 --- a/src/cpp/fastdds/subscriber/DataReaderImpl.hpp +++ b/src/cpp/fastdds/subscriber/DataReaderImpl.hpp @@ -102,7 +102,8 @@ class DataReaderImpl const TypeSupport& type, TopicDescription* topic, const DataReaderQos& qos, - DataReaderListener* listener = nullptr); + DataReaderListener* listener = nullptr, + std::shared_ptr payload_pool = nullptr); public: @@ -482,8 +483,10 @@ class DataReaderImpl DataReader* user_datareader_ = nullptr; - std::shared_ptr payload_pool_; std::shared_ptr sample_pool_; + std::shared_ptr payload_pool_; + + bool is_custom_payload_pool_ = false; detail::SampleInfoPool sample_info_pool_; detail::DataReaderLoanManager loan_manager_; diff --git a/src/cpp/fastdds/subscriber/Subscriber.cpp b/src/cpp/fastdds/subscriber/Subscriber.cpp index 6df62f9d5a4..d62f4adab00 100644 --- a/src/cpp/fastdds/subscriber/Subscriber.cpp +++ b/src/cpp/fastdds/subscriber/Subscriber.cpp @@ -106,18 +106,20 @@ DataReader* Subscriber::create_datareader( TopicDescription* topic, const DataReaderQos& reader_qos, DataReaderListener* listener, - const StatusMask& mask) + const StatusMask& mask, + std::shared_ptr payload_pool) { - return impl_->create_datareader(topic, reader_qos, listener, mask); + return impl_->create_datareader(topic, reader_qos, listener, mask, payload_pool); } DataReader* Subscriber::create_datareader_with_profile( TopicDescription* topic, const std::string& profile_name, DataReaderListener* listener, - const StatusMask& mask) + const StatusMask& mask, + std::shared_ptr payload_pool) { - return impl_->create_datareader_with_profile(topic, profile_name, listener, mask); + return impl_->create_datareader_with_profile(topic, profile_name, listener, mask, payload_pool); } ReturnCode_t Subscriber::delete_datareader( diff --git a/src/cpp/fastdds/subscriber/SubscriberImpl.cpp b/src/cpp/fastdds/subscriber/SubscriberImpl.cpp index fa68b1f5b5c..999185c46f8 100644 --- a/src/cpp/fastdds/subscriber/SubscriberImpl.cpp +++ b/src/cpp/fastdds/subscriber/SubscriberImpl.cpp @@ -175,16 +175,18 @@ DataReaderImpl* SubscriberImpl::create_datareader_impl( const TypeSupport& type, TopicDescription* topic, const DataReaderQos& qos, - DataReaderListener* listener) + DataReaderListener* listener, + std::shared_ptr payload_pool) { - return new DataReaderImpl(this, type, topic, qos, listener); + return new DataReaderImpl(this, type, topic, qos, listener, payload_pool); } DataReader* SubscriberImpl::create_datareader( TopicDescription* topic, const DataReaderQos& qos, DataReaderListener* listener, - const StatusMask& mask) + const StatusMask& mask, + std::shared_ptr payload_pool) { EPROSIMA_LOG_INFO(SUBSCRIBER, "CREATING SUBSCRIBER IN TOPIC: " << topic->get_name()); //Look for the correct type registration @@ -205,7 +207,7 @@ DataReader* SubscriberImpl::create_datareader( topic->get_impl()->reference(); - DataReaderImpl* impl = create_datareader_impl(type_support, topic, qos, listener); + DataReaderImpl* impl = create_datareader_impl(type_support, topic, qos, listener, payload_pool); DataReader* reader = new DataReader(impl, mask); impl->user_datareader_ = reader; @@ -230,7 +232,8 @@ DataReader* SubscriberImpl::create_datareader_with_profile( TopicDescription* topic, const std::string& profile_name, DataReaderListener* listener, - const StatusMask& mask) + const StatusMask& mask, + std::shared_ptr payload_pool) { // TODO (ILG): Change when we have full XML support for DDS QoS profiles SubscriberAttributes attr; @@ -238,7 +241,7 @@ DataReader* SubscriberImpl::create_datareader_with_profile( { DataReaderQos qos = default_datareader_qos_; utils::set_qos_from_attributes(qos, attr); - return create_datareader(topic, qos, listener, mask); + return create_datareader(topic, qos, listener, mask, payload_pool); } return nullptr; diff --git a/src/cpp/fastdds/subscriber/SubscriberImpl.hpp b/src/cpp/fastdds/subscriber/SubscriberImpl.hpp index fb6ba2df590..7f8212fa811 100644 --- a/src/cpp/fastdds/subscriber/SubscriberImpl.hpp +++ b/src/cpp/fastdds/subscriber/SubscriberImpl.hpp @@ -97,13 +97,15 @@ class SubscriberImpl TopicDescription* topic, const DataReaderQos& reader_qos, DataReaderListener* listener = nullptr, - const StatusMask& mask = StatusMask::all()); + const StatusMask& mask = StatusMask::all(), + std::shared_ptr payload_pool = nullptr); DataReader* create_datareader_with_profile( TopicDescription* topic, const std::string& profile_name, DataReaderListener* listener, - const StatusMask& mask = StatusMask::all()); + const StatusMask& mask = StatusMask::all(), + std::shared_ptr payload_pool = nullptr); ReturnCode_t delete_datareader( const DataReader* reader); @@ -298,7 +300,8 @@ class SubscriberImpl const TypeSupport& type, TopicDescription* topic, const DataReaderQos& qos, - DataReaderListener* listener); + DataReaderListener* listener, + std::shared_ptr payload_pool); }; } /* namespace dds */ diff --git a/src/cpp/statistics/fastdds/publisher/DataWriterImpl.hpp b/src/cpp/statistics/fastdds/publisher/DataWriterImpl.hpp index 498c3554efe..b91f9855dcf 100644 --- a/src/cpp/statistics/fastdds/publisher/DataWriterImpl.hpp +++ b/src/cpp/statistics/fastdds/publisher/DataWriterImpl.hpp @@ -58,8 +58,9 @@ class DataWriterImpl : public efd::DataWriterImpl efd::Topic* topic, const efd::DataWriterQos& qos, efd::DataWriterListener* listener, + std::shared_ptr payload_pool, std::shared_ptr stat_listener) - : BaseType(p, type, topic, qos, listener) + : BaseType(p, type, topic, qos, listener, payload_pool) , statistics_listener_(stat_listener) { } diff --git a/src/cpp/statistics/fastdds/publisher/PublisherImpl.hpp b/src/cpp/statistics/fastdds/publisher/PublisherImpl.hpp index ca51992f742..72fca819ff2 100644 --- a/src/cpp/statistics/fastdds/publisher/PublisherImpl.hpp +++ b/src/cpp/statistics/fastdds/publisher/PublisherImpl.hpp @@ -62,9 +62,10 @@ class PublisherImpl : public efd::PublisherImpl const efd::TypeSupport& type, efd::Topic* topic, const efd::DataWriterQos& qos, - efd::DataWriterListener* listener) override + efd::DataWriterListener* listener, + std::shared_ptr payload_pool) override { - return new DataWriterImpl(this, type, topic, qos, listener, statistics_listener_); + return new DataWriterImpl(this, type, topic, qos, listener, payload_pool, statistics_listener_); } private: diff --git a/src/cpp/statistics/fastdds/subscriber/DataReaderImpl.hpp b/src/cpp/statistics/fastdds/subscriber/DataReaderImpl.hpp index 5a78523aa4a..0454c08bb50 100644 --- a/src/cpp/statistics/fastdds/subscriber/DataReaderImpl.hpp +++ b/src/cpp/statistics/fastdds/subscriber/DataReaderImpl.hpp @@ -48,8 +48,9 @@ class DataReaderImpl : public efd::DataReaderImpl efd::TopicDescription* topic, const efd::DataReaderQos& qos, efd::DataReaderListener* listener, + std::shared_ptr payload_pool, std::shared_ptr stat_listener) - : BaseType(s, type, topic, qos, listener) + : BaseType(s, type, topic, qos, listener, payload_pool) , statistics_listener_(stat_listener) { } diff --git a/src/cpp/statistics/fastdds/subscriber/SubscriberImpl.hpp b/src/cpp/statistics/fastdds/subscriber/SubscriberImpl.hpp index db68fb09b7b..3cdc5fd1ba1 100644 --- a/src/cpp/statistics/fastdds/subscriber/SubscriberImpl.hpp +++ b/src/cpp/statistics/fastdds/subscriber/SubscriberImpl.hpp @@ -53,9 +53,10 @@ class SubscriberImpl : public efd::SubscriberImpl const efd::TypeSupport& type, efd::TopicDescription* topic, const efd::DataReaderQos& qos, - efd::DataReaderListener* listener) override + efd::DataReaderListener* listener, + std::shared_ptr payload_pool) override { - return new DataReaderImpl(this, type, topic, qos, listener, statistics_listener_); + return new DataReaderImpl(this, type, topic, qos, listener, payload_pool, statistics_listener_); } private: diff --git a/test/blackbox/api/dds-pim/CustomPayloadPool.hpp b/test/blackbox/api/dds-pim/CustomPayloadPool.hpp new file mode 100644 index 00000000000..e4f00f2aa32 --- /dev/null +++ b/test/blackbox/api/dds-pim/CustomPayloadPool.hpp @@ -0,0 +1,104 @@ +// Copyright 2023 Proyectos y Sistemas de Mantenimiento SL (eProsima). +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +/** + * @file CustomPayloadPool.hpp + */ + +#ifndef DDS_CUSTOM_PAYLOAD_POOL_HPP +#define DDS_CUSTOM_PAYLOAD_POOL_HPP + +#include +#include +#include +#include + +class CustomPayloadPool : public eprosima::fastrtps::rtps::IPayloadPool +{ +public: + + ~CustomPayloadPool() = default; + + bool get_payload( + unsigned int size, + eprosima::fastrtps::rtps::CacheChange_t& cache_change) + { + // Reserve new memory for the payload buffer + unsigned char* payload = new unsigned char[size]; + + // Assign the payload buffer to the CacheChange and update sizes + cache_change.serializedPayload.data = payload; + cache_change.serializedPayload.length = size; + cache_change.serializedPayload.max_size = size; + + // Tell the CacheChange who needs to release its payload + cache_change.payload_owner(this); + + ++requested_payload_count; + + return true; + } + + bool get_payload( + eprosima::fastrtps::rtps::SerializedPayload_t& data, + eprosima::fastrtps::rtps::IPayloadPool*& /*data_owner*/, + eprosima::fastrtps::rtps::CacheChange_t& cache_change) + { + // Reserve new memory for the payload buffer + unsigned char* payload = new unsigned char[data.length]; + + // Copy the data + memcpy(payload, data.data, data.length); + + // Assign the payload buffer to the CacheChange and update sizes + cache_change.serializedPayload.data = payload; + cache_change.serializedPayload.length = data.length; + cache_change.serializedPayload.max_size = data.length; + + // Tell the CacheChange who needs to release its payload + cache_change.payload_owner(this); + + ++requested_payload_count; + + return true; + } + + bool release_payload( + eprosima::fastrtps::rtps::CacheChange_t& cache_change) + { + // Ensure precondition + assert(this == cache_change.payload_owner()); + + // Dealloc the buffer of the payload + delete[] cache_change.serializedPayload.data; + + // Reset sizes and pointers + cache_change.serializedPayload.data = nullptr; + cache_change.serializedPayload.length = 0; + cache_change.serializedPayload.max_size = 0; + + // Reset the owner of the payload + cache_change.payload_owner(nullptr); + + ++returned_payload_count; + + return true; + } + + uint32_t requested_payload_count = 0; + uint32_t returned_payload_count = 0; + +}; + +#endif // DDS_CUSTOM_PAYLOAD_POOL_HPP diff --git a/test/blackbox/common/DDSBlackboxTestsBasic.cpp b/test/blackbox/common/DDSBlackboxTestsBasic.cpp index c4cd711ecd1..bf626cfdb0f 100644 --- a/test/blackbox/common/DDSBlackboxTestsBasic.cpp +++ b/test/blackbox/common/DDSBlackboxTestsBasic.cpp @@ -42,6 +42,7 @@ #include #include "BlackboxTests.hpp" +#include "../api/dds-pim/CustomPayloadPool.hpp" #include "../api/dds-pim/PubSubReader.hpp" #include "../api/dds-pim/PubSubWriter.hpp" #include "../api/dds-pim/PubSubWriterReader.hpp" @@ -696,6 +697,67 @@ TEST(DDSBasic, participant_ignore_local_endpoints_two_participants) EXPECT_EQ(reader.block_for_all(std::chrono::seconds(1)), 5); } +/** + * @test This test checks both the visibility of custom pool functions + * for DataReader and DataWriters while also testing their correct + * behavior + */ +TEST(DDSBasic, endpoint_custom_payload_pools) +{ + DomainParticipant* participant = + DomainParticipantFactory::get_instance()->create_participant(0, PARTICIPANT_QOS_DEFAULT); + ASSERT_NE(participant, nullptr); + + Subscriber* subscriber = participant->create_subscriber(SUBSCRIBER_QOS_DEFAULT); + ASSERT_NE(subscriber, nullptr); + + Publisher* publisher = participant->create_publisher(PUBLISHER_QOS_DEFAULT); + ASSERT_NE(publisher, nullptr); + + // Register type + TypeSupport type; + + type.reset(new StringTestPubSubType()); + type.register_type(participant); + ASSERT_NE(nullptr, type); + + type.register_type(participant); + + Topic* topic = participant->create_topic("footopic", type.get_type_name(), TOPIC_QOS_DEFAULT); + ASSERT_NE(topic, nullptr); + + // Next QoS config checks the default qos configuration, + // create_datareader() should not return nullptr. + DataReaderQos reader_qos = DATAREADER_QOS_DEFAULT; + + std::shared_ptr reader_payload_pool = std::make_shared(); + + std::shared_ptr writer_payload_pool = std::make_shared(); + + DataReader* data_reader = subscriber->create_datareader( + topic, reader_qos, nullptr, StatusMask::all(), reader_payload_pool); + + DataWriterQos writer_qos = DATAWRITER_QOS_DEFAULT; + + DataWriter* data_writer = publisher->create_datawriter( + topic, writer_qos, nullptr, StatusMask::all(), writer_payload_pool); + + ASSERT_NE(data_reader, nullptr); + ASSERT_NE(data_writer, nullptr); + + StringTest data; + data.message("Lorem Ipsum"); + + data_writer->write(&data, HANDLE_NIL); + + std::this_thread::sleep_for(std::chrono::seconds(2)); + + ASSERT_EQ(reader_payload_pool->requested_payload_count, 1u); + ASSERT_EQ(writer_payload_pool->requested_payload_count, 1u); + + participant->delete_contained_entities(); +} + } // namespace dds } // namespace fastdds } // namespace eprosima diff --git a/test/unittest/common/CustomPayloadPool.hpp b/test/unittest/common/CustomPayloadPool.hpp new file mode 100644 index 00000000000..e4f00f2aa32 --- /dev/null +++ b/test/unittest/common/CustomPayloadPool.hpp @@ -0,0 +1,104 @@ +// Copyright 2023 Proyectos y Sistemas de Mantenimiento SL (eProsima). +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +/** + * @file CustomPayloadPool.hpp + */ + +#ifndef DDS_CUSTOM_PAYLOAD_POOL_HPP +#define DDS_CUSTOM_PAYLOAD_POOL_HPP + +#include +#include +#include +#include + +class CustomPayloadPool : public eprosima::fastrtps::rtps::IPayloadPool +{ +public: + + ~CustomPayloadPool() = default; + + bool get_payload( + unsigned int size, + eprosima::fastrtps::rtps::CacheChange_t& cache_change) + { + // Reserve new memory for the payload buffer + unsigned char* payload = new unsigned char[size]; + + // Assign the payload buffer to the CacheChange and update sizes + cache_change.serializedPayload.data = payload; + cache_change.serializedPayload.length = size; + cache_change.serializedPayload.max_size = size; + + // Tell the CacheChange who needs to release its payload + cache_change.payload_owner(this); + + ++requested_payload_count; + + return true; + } + + bool get_payload( + eprosima::fastrtps::rtps::SerializedPayload_t& data, + eprosima::fastrtps::rtps::IPayloadPool*& /*data_owner*/, + eprosima::fastrtps::rtps::CacheChange_t& cache_change) + { + // Reserve new memory for the payload buffer + unsigned char* payload = new unsigned char[data.length]; + + // Copy the data + memcpy(payload, data.data, data.length); + + // Assign the payload buffer to the CacheChange and update sizes + cache_change.serializedPayload.data = payload; + cache_change.serializedPayload.length = data.length; + cache_change.serializedPayload.max_size = data.length; + + // Tell the CacheChange who needs to release its payload + cache_change.payload_owner(this); + + ++requested_payload_count; + + return true; + } + + bool release_payload( + eprosima::fastrtps::rtps::CacheChange_t& cache_change) + { + // Ensure precondition + assert(this == cache_change.payload_owner()); + + // Dealloc the buffer of the payload + delete[] cache_change.serializedPayload.data; + + // Reset sizes and pointers + cache_change.serializedPayload.data = nullptr; + cache_change.serializedPayload.length = 0; + cache_change.serializedPayload.max_size = 0; + + // Reset the owner of the payload + cache_change.payload_owner(nullptr); + + ++returned_payload_count; + + return true; + } + + uint32_t requested_payload_count = 0; + uint32_t returned_payload_count = 0; + +}; + +#endif // DDS_CUSTOM_PAYLOAD_POOL_HPP diff --git a/test/unittest/dds/publisher/DataWriterTests.cpp b/test/unittest/dds/publisher/DataWriterTests.cpp index 5b57dd1305d..017e77537e2 100644 --- a/test/unittest/dds/publisher/DataWriterTests.cpp +++ b/test/unittest/dds/publisher/DataWriterTests.cpp @@ -24,6 +24,10 @@ #include #include #include +#include +#include +#include +#include #include #include @@ -36,6 +40,8 @@ #include +#include "../../common/CustomPayloadPool.hpp" + #include #include @@ -1885,6 +1891,54 @@ TEST(DataWriterTests, InstancePolicyAllocationConsistencyKeyed) ASSERT_EQ(ReturnCode_t::RETCODE_OK, default_data_writer1->set_qos(qos2)); } + +/* + * This test checks the proper behavior of the custom payload pool DataReader overload. + */ +TEST(DataWriterTests, CustomPoolCreation) +{ + DomainParticipant* participant = + DomainParticipantFactory::get_instance()->create_participant(0, PARTICIPANT_QOS_DEFAULT); + ASSERT_NE(participant, nullptr); + + Subscriber* subscriber = participant->create_subscriber(SUBSCRIBER_QOS_DEFAULT); + ASSERT_NE(subscriber, nullptr); + + Publisher* publisher = participant->create_publisher(PUBLISHER_QOS_DEFAULT); + ASSERT_NE(publisher, nullptr); + + TypeSupport type(new TopicDataTypeMock()); + type.register_type(participant); + + Topic* topic = participant->create_topic("footopic", type.get_type_name(), TOPIC_QOS_DEFAULT); + ASSERT_NE(topic, nullptr); + + // Next QoS config checks the default qos configuration, + // create_datareader() should not return nullptr. + DataReaderQos reader_qos = DATAREADER_QOS_DEFAULT; + + std::shared_ptr payload_pool = std::make_shared(); + + DataReader* data_reader = subscriber->create_datareader(topic, reader_qos); + + DataWriterQos writer_qos = DATAWRITER_QOS_DEFAULT; + + DataWriter* data_writer = publisher->create_datawriter(topic, writer_qos, nullptr, StatusMask::all(), payload_pool); + + ASSERT_NE(data_writer, nullptr); + ASSERT_NE(data_reader, nullptr); + + FooType data; + + data_writer->write(&data, HANDLE_NIL); + + ASSERT_EQ(payload_pool->requested_payload_count, 1u); + + participant->delete_contained_entities(); + + DomainParticipantFactory::get_instance()->delete_participant(participant); +} + } // namespace dds } // namespace fastdds } // namespace eprosima diff --git a/test/unittest/dds/subscriber/DataReaderTests.cpp b/test/unittest/dds/subscriber/DataReaderTests.cpp index 048bfd98a50..4ab05b7c493 100644 --- a/test/unittest/dds/subscriber/DataReaderTests.cpp +++ b/test/unittest/dds/subscriber/DataReaderTests.cpp @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -68,6 +69,10 @@ #include #include +#include "../../common/CustomPayloadPool.hpp" +#include "fastdds/dds/common/InstanceHandle.hpp" +#include "fastdds/dds/core/policy/QosPolicies.hpp" + #include #if defined(__cplusplus_winrt) @@ -3489,6 +3494,57 @@ TEST_F(DataReaderTests, InstancePolicyAllocationConsistencyKeyed) ASSERT_EQ(ReturnCode_t::RETCODE_OK, default_data_reader2->set_qos(qos2)); } +/* + * This test checks the proper behavior of the custom payload pool DataReader overload. + */ +TEST_F(DataReaderTests, CustomPoolCreation) +{ + DomainParticipant* participant = + DomainParticipantFactory::get_instance()->create_participant(0, PARTICIPANT_QOS_DEFAULT); + ASSERT_NE(participant, nullptr); + + Subscriber* subscriber = participant->create_subscriber(SUBSCRIBER_QOS_DEFAULT); + ASSERT_NE(subscriber, nullptr); + + Publisher* publisher = participant->create_publisher(PUBLISHER_QOS_DEFAULT); + ASSERT_NE(publisher, nullptr); + + TypeSupport type(new FooTypeSupport()); + type.register_type(participant); + + Topic* topic = participant->create_topic("footopic", type.get_type_name(), TOPIC_QOS_DEFAULT); + ASSERT_NE(topic, nullptr); + + // Next QoS config checks the default qos configuration, + // create_datareader() should not return nullptr. + DataReaderQos reader_qos = DATAREADER_QOS_DEFAULT; + + std::shared_ptr payload_pool = std::make_shared(); + + DataReader* data_reader = + subscriber->create_datareader(topic, reader_qos, nullptr, StatusMask::all(), payload_pool); + + DataWriterQos writer_qos = DATAWRITER_QOS_DEFAULT; + writer_qos.reliability().kind = eprosima::fastdds::dds::BEST_EFFORT_RELIABILITY_QOS; + + DataWriter* data_writer = publisher->create_datawriter(topic, writer_qos); + + FooType data; + data.index(0); + data.message()[0] = '\0'; + data.message()[1] = '\0'; + + data_writer->write(&data, HANDLE_NIL); + + ASSERT_EQ(payload_pool->requested_payload_count, 1u); + + ASSERT_NE(data_reader, nullptr); + + participant->delete_contained_entities(); + + DomainParticipantFactory::get_instance()->delete_participant(participant); +} + int main( int argc, char** argv) diff --git a/test/unittest/statistics/dds/StatisticsDomainParticipantMockTests/mock/statistics/fastdds/publisher/PublisherImpl.hpp b/test/unittest/statistics/dds/StatisticsDomainParticipantMockTests/mock/statistics/fastdds/publisher/PublisherImpl.hpp index 69479797f44..90cfd731f66 100644 --- a/test/unittest/statistics/dds/StatisticsDomainParticipantMockTests/mock/statistics/fastdds/publisher/PublisherImpl.hpp +++ b/test/unittest/statistics/dds/StatisticsDomainParticipantMockTests/mock/statistics/fastdds/publisher/PublisherImpl.hpp @@ -75,9 +75,10 @@ class PublisherImpl : public efd::PublisherImpl const efd::TypeSupport& type, efd::Topic* topic, const efd::DataWriterQos& qos, - efd::DataWriterListener* listener) override + efd::DataWriterListener* listener, + std::shared_ptr payload_pool) override { - return new DataWriterImpl(this, type, topic, qos, listener, statistics_listener_); + return new DataWriterImpl(this, type, topic, qos, listener, payload_pool, statistics_listener_); } efd::DataWriter* create_datawriter( diff --git a/versions.md b/versions.md index 1b38209c725..9d9af773259 100644 --- a/versions.md +++ b/versions.md @@ -3,6 +3,7 @@ Forthcoming * Added participant property to configure SHM transport metatraffic behavior. No metatraffic over SHM transport by default. +* Exposed custom payload pool on DDS endpoints declaration Version 2.11.0 -------------- From aa15d042920554e567c901de1c4da208dc74c78f Mon Sep 17 00:00:00 2001 From: Eduardo Ponz Segrelles Date: Mon, 7 Aug 2023 21:58:57 +0200 Subject: [PATCH 20/23] Fix encapsulation format in WLP (#3784) * Refs #19347: Fix encapsulation format in WLP. Improve WLP checks Signed-off-by: Eduardo Ponz * Refs #19347: Correctly set CDR endianess for BE Signed-off-by: Eduardo Ponz * Refs #19347: Uncrustify Signed-off-by: Eduardo Ponz * Refs #19347: Fix doxygen Signed-off-by: Eduardo Ponz * Refs #19347: Apply suggestions Signed-off-by: Eduardo Ponz * Refs #19347: Fix Windows warning Signed-off-by: Eduardo Ponz * Refs #19347: Apply suggestions Signed-off-by: Eduardo Ponz * Refs #19347: Correct condition when setting the payload encapsulation Signed-off-by: Eduardo Ponz * Refs #19347: Remove legacy typedef Signed-off-by: Eduardo Ponz * Refs #19347: Initialize uint32_t variable Signed-off-by: Eduardo Ponz --------- Signed-off-by: Eduardo Ponz Signed-off-by: jimwang118 --- .../rtps/builtin/liveliness/WLPListener.h | 54 +++++--- include/fastdds/rtps/common/CDRMessage_t.h | 6 +- .../fastdds/rtps/common/SerializedPayload.h | 2 +- include/fastdds/rtps/messages/CDRMessage.h | 11 ++ include/fastdds/rtps/messages/CDRMessage.hpp | 14 ++ src/cpp/rtps/builtin/liveliness/WLP.cpp | 4 +- .../rtps/builtin/liveliness/WLPListener.cpp | 123 +++++++++++++----- 7 files changed, 158 insertions(+), 56 deletions(-) diff --git a/include/fastdds/rtps/builtin/liveliness/WLPListener.h b/include/fastdds/rtps/builtin/liveliness/WLPListener.h index a1859a202b7..825fe431933 100644 --- a/include/fastdds/rtps/builtin/liveliness/WLPListener.h +++ b/include/fastdds/rtps/builtin/liveliness/WLPListener.h @@ -21,14 +21,14 @@ #define _FASTDDS_RTPS_WLPLISTENER_H_ #ifndef DOXYGEN_SHOULD_SKIP_THIS_PUBLIC -#include -#include +#include #include - +#include +#include #include namespace eprosima { -namespace fastrtps{ +namespace fastrtps { namespace rtps { class WLP; @@ -39,14 +39,16 @@ struct CacheChange_t; * Class WLPListener that receives the liveliness messages asserting the liveliness of remote endpoints. * @ingroup LIVELINESS_MODULE */ -class WLPListener: public ReaderListener { +class WLPListener : public ReaderListener +{ public: /** * @brief Constructor * @param pwlp Pointer to the writer liveliness protocol */ - WLPListener(WLP* pwlp); + WLPListener( + WLP* pwlp); /** * @brief Destructor @@ -60,27 +62,41 @@ class WLPListener: public ReaderListener { */ void onNewCacheChangeAdded( RTPSReader* reader, - const CacheChange_t* const change) override; + const CacheChange_t* const change) override; private: /** - * Separate the Key between the GuidPrefix_t and the liveliness Kind - * @param key InstanceHandle_t to separate. - * @param guidP GuidPrefix_t pointer to store the info. - * @param liveliness Liveliness Kind Pointer. - * @return True if correctly separated. - */ + * Separate the Key between the GuidPrefix_t and the liveliness Kind + * @param key InstanceHandle_t to separate. + * @param guidP GuidPrefix_t pointer to store the info. + * @param liveliness Liveliness Kind Pointer. + * @return True if correctly separated. + */ bool separateKey( InstanceHandle_t& key, GuidPrefix_t* guidP, LivelinessQosPolicyKind* liveliness); /** - * Compute the key from a CacheChange_t - * @param change - */ - bool computeKey(CacheChange_t* change); + * Compute the key from a CacheChange_t + * @param change + */ + bool computeKey( + CacheChange_t* change); + + /** + * @brief Check that the ParticipantMessageData kind is a valid one for WLP and extract the liveliness kind. + * + * @param[in] serialized_kind A pointer to the first octet of the kind array. The function assumes 4 elements + * in the array. + * @param[out] liveliness_kind A reference to the LivelinessQosPolicyKind. + * + * @return True if the kind corresponds with one for WLP, false otherwise. + */ + bool get_wlp_kind( + const octet* serialized_kind, + LivelinessQosPolicyKind& liveliness_kind); //! A pointer to the writer liveliness protocol WLP* mp_WLP; @@ -89,6 +105,6 @@ class WLPListener: public ReaderListener { } /* namespace rtps */ } /* namespace eprosima */ -} -#endif +} // namespace eprosima +#endif // ifndef DOXYGEN_SHOULD_SKIP_THIS_PUBLIC #endif /* _FASTDDS_RTPS_WLPLISTENER_H_ */ diff --git a/include/fastdds/rtps/common/CDRMessage_t.h b/include/fastdds/rtps/common/CDRMessage_t.h index 3c93a564274..567f4e314bd 100644 --- a/include/fastdds/rtps/common/CDRMessage_t.h +++ b/include/fastdds/rtps/common/CDRMessage_t.h @@ -96,7 +96,11 @@ struct RTPS_DllAPI CDRMessage_t final const SerializedPayload_t& payload) : wraps(true) { - msg_endian = payload.encapsulation == PL_CDR_BE ? BIGEND : LITTLEEND; + msg_endian = LITTLEEND; + if (payload.encapsulation == PL_CDR_BE || payload.encapsulation == CDR_BE) + { + msg_endian = BIGEND; + } pos = payload.pos; length = payload.length; buffer = payload.data; diff --git a/include/fastdds/rtps/common/SerializedPayload.h b/include/fastdds/rtps/common/SerializedPayload.h index 17e2aeea6c2..4422c43efe5 100644 --- a/include/fastdds/rtps/common/SerializedPayload.h +++ b/include/fastdds/rtps/common/SerializedPayload.h @@ -42,7 +42,7 @@ namespace rtps { #define PL_CDR_LE 0x0003 #if FASTDDS_IS_BIG_ENDIAN_TARGET -#define DEFAULT_ENCAPSULATION CDR_LE +#define DEFAULT_ENCAPSULATION CDR_BE #define PL_DEFAULT_ENCAPSULATION PL_CDR_BE #else #define DEFAULT_ENCAPSULATION CDR_LE diff --git a/include/fastdds/rtps/messages/CDRMessage.h b/include/fastdds/rtps/messages/CDRMessage.h index 7856906f361..20e55b9550f 100644 --- a/include/fastdds/rtps/messages/CDRMessage.h +++ b/include/fastdds/rtps/messages/CDRMessage.h @@ -312,6 +312,17 @@ inline bool addParticipantGenericMessage( ///@} +/** + * @brief Skip bytes in serialized buffer + * + * @param msg The CDR message + * @param length The number of bytes to skip + * @return true if skipped, false otherwise + */ +inline bool skip( + CDRMessage_t* msg, + uint32_t length); + } /* namespace CDRMessage */ } /* namespace rtps */ diff --git a/include/fastdds/rtps/messages/CDRMessage.hpp b/include/fastdds/rtps/messages/CDRMessage.hpp index a39156855d4..d3161570767 100644 --- a/include/fastdds/rtps/messages/CDRMessage.hpp +++ b/include/fastdds/rtps/messages/CDRMessage.hpp @@ -1301,6 +1301,20 @@ inline bool CDRMessage::readParticipantGenericMessage( return true; } +inline bool CDRMessage::skip( + CDRMessage_t* msg, + uint32_t length) +{ + // Validate input + bool ret = (msg != nullptr) && (msg->pos + length <= msg->length); + if (ret) + { + // Advance index the number of specified bytes + msg->pos += length; + } + return ret; +} + } // namespace rtps } // namespace fastrtps } // namespace eprosima diff --git a/src/cpp/rtps/builtin/liveliness/WLP.cpp b/src/cpp/rtps/builtin/liveliness/WLP.cpp index 6c1b49f6979..775c5ebd41e 100644 --- a/src/cpp/rtps/builtin/liveliness/WLP.cpp +++ b/src/cpp/rtps/builtin/liveliness/WLP.cpp @@ -900,9 +900,9 @@ bool WLP::send_liveliness_message( if (change != nullptr) { - change->serializedPayload.encapsulation = (uint16_t)PL_DEFAULT_ENCAPSULATION; + change->serializedPayload.encapsulation = (uint16_t)DEFAULT_ENCAPSULATION; change->serializedPayload.data[0] = 0; - change->serializedPayload.data[1] = PL_DEFAULT_ENCAPSULATION; + change->serializedPayload.data[1] = DEFAULT_ENCAPSULATION; change->serializedPayload.data[2] = 0; change->serializedPayload.data[3] = 0; diff --git a/src/cpp/rtps/builtin/liveliness/WLPListener.cpp b/src/cpp/rtps/builtin/liveliness/WLPListener.cpp index fc13dcabca1..ff5683fe3c4 100644 --- a/src/cpp/rtps/builtin/liveliness/WLPListener.cpp +++ b/src/cpp/rtps/builtin/liveliness/WLPListener.cpp @@ -16,28 +16,33 @@ * @file WLPListener.cpp * */ - #include -#include -#include +#include +#include +#include +#include -#include +#include #include - -#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include #include -#include - -#include - - +#include namespace eprosima { namespace fastrtps { namespace rtps { - WLPListener::WLPListener( WLP* plwp) : mp_WLP(plwp) @@ -48,8 +53,6 @@ WLPListener::~WLPListener() { } -typedef std::vector::iterator WPIT; - void WLPListener::onNewCacheChangeAdded( RTPSReader* reader, const CacheChange_t* const changeIN) @@ -57,7 +60,7 @@ void WLPListener::onNewCacheChangeAdded( std::lock_guard guard2(*mp_WLP->mp_builtinProtocols->mp_PDP->getMutex()); GuidPrefix_t guidP; - LivelinessQosPolicyKind livelinessKind; + LivelinessQosPolicyKind livelinessKind = AUTOMATIC_LIVELINESS_QOS; CacheChange_t* change = (CacheChange_t*)changeIN; if (!computeKey(change)) { @@ -74,23 +77,50 @@ void WLPListener::onNewCacheChangeAdded( break; } } - if (change->serializedPayload.length > 0) - { - if (PL_CDR_BE == change->serializedPayload.data[1]) - { - change->serializedPayload.encapsulation = (uint16_t)PL_CDR_BE; - } - else - { - change->serializedPayload.encapsulation = (uint16_t)PL_CDR_LE; - } - for (size_t i = 0; i < 12; ++i) + // Serialized payload should have at least 4 bytes of representation header, 12 of GuidPrefix, + // 4 of kind, and 4 of length. + constexpr uint32_t participant_msg_data_kind_size = 4; + constexpr uint32_t participant_msg_data_length_size = 4; + constexpr uint32_t min_serialized_length = SerializedPayload_t::representation_header_size + + GuidPrefix_t::size + + participant_msg_data_kind_size + + participant_msg_data_length_size; + + if (change->serializedPayload.length >= min_serialized_length) + { + constexpr uint32_t participant_msg_data_kind_pos = 16; + constexpr uint32_t encapsulation_pos = 1; + uint32_t data_length = 0; + + // Extract encapsulation from the second byte of the representation header. Done prior to + // creating the CDRMessage_t, as the CDRMessage_t ctor uses it for its own state. + change->serializedPayload.encapsulation = + static_cast(change->serializedPayload.data[encapsulation_pos]); + + // Create CDR message from buffer to deserialize contents for further validation + CDRMessage_t cdr_message(change->serializedPayload); + + bool message_ok = ( + // Skip representation header + CDRMessage::skip(&cdr_message, SerializedPayload_t::representation_header_size) + // Extract GuidPrefix + && CDRMessage::readData(&cdr_message, guidP.value, GuidPrefix_t::size) + // Skip kind, it will be validated later + && CDRMessage::skip(&cdr_message, participant_msg_data_kind_size) + // Extract and validate liveliness kind + && get_wlp_kind(&change->serializedPayload.data[participant_msg_data_kind_pos], livelinessKind) + // Extract data length + && CDRMessage::readUInt32(&cdr_message, &data_length) + // Check that serialized length is correctly set + && (change->serializedPayload.length >= min_serialized_length + data_length)); + + if (!message_ok) { - guidP.value[i] = change->serializedPayload.data[i + 4]; + EPROSIMA_LOG_INFO(RTPS_LIVELINESS, "Ignoring incorrect WLP ParticipantDataMessage"); + history->remove_change(change); + return; } - livelinessKind = (LivelinessQosPolicyKind)(change->serializedPayload.data[19] - 0x01); - } else { @@ -99,6 +129,8 @@ void WLPListener::onNewCacheChangeAdded( &guidP, &livelinessKind)) { + EPROSIMA_LOG_INFO(RTPS_LIVELINESS, "Ignoring not WLP ParticipantDataMessage"); + history->remove_change(change); return; } } @@ -130,12 +162,13 @@ bool WLPListener::separateKey( GuidPrefix_t* guidP, LivelinessQosPolicyKind* liveliness) { - for (uint8_t i = 0; i < 12; ++i) + bool ret = get_wlp_kind(&key.value[12], *liveliness); + if (ret) { - guidP->value[i] = key.value[i]; + // Extract GuidPrefix + memcpy(guidP->value, key.value, 12); } - *liveliness = (LivelinessQosPolicyKind)key.value[15]; - return true; + return ret; } bool WLPListener::computeKey( @@ -154,6 +187,30 @@ bool WLPListener::computeKey( return true; } +bool WLPListener::get_wlp_kind( + const octet* serialized_kind, + LivelinessQosPolicyKind& liveliness_kind) +{ + /* + * From RTPS 2.5 9.6.3.1, the ParticipantMessageData kinds for WLP are: + * - PARTICIPANT_MESSAGE_DATA_KIND_AUTOMATIC_LIVELINESS_UPDATE {0x00, 0x00, 0x00, 0x01} + * - PARTICIPANT_MESSAGE_DATA_KIND_MANUAL_LIVELINESS_UPDATE {0x00, 0x00, 0x00, 0x02} + */ + bool is_wlp = ( + serialized_kind[0] == 0 + && serialized_kind[1] == 0 + && serialized_kind[2] == 0 + && (serialized_kind[3] == 0x01 || serialized_kind[3] == 0x02)); + + if (is_wlp) + { + // Adjust and cast to LivelinessQosPolicyKind enum, where AUTOMATIC_LIVELINESS_QOS == 0 + liveliness_kind = static_cast(serialized_kind[3] - 0x01); + } + + return is_wlp; +} + } /* namespace rtps */ } /* namespace eprosima */ } // namespace eprosima From 1aac62dd3dfd0042884fc30008b7344111d4fef2 Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Fri, 11 Aug 2023 07:05:19 +0200 Subject: [PATCH 21/23] Fix RemoteBuiltinEndpointHonoring blackbox test (#3787) * Refs #19358. Only account for WLP heartbeats and acknacks. Signed-off-by: Miguel Company * Refs #19358. Change test expectations. Signed-off-by: Miguel Company * Refs #19358. Rename counters. Signed-off-by: Miguel Company * Refs #19358. Whitelist localhost to ensure only the two test participants communicate. Signed-off-by: Miguel Company --------- Signed-off-by: Miguel Company Signed-off-by: jimwang118 --- .../common/BlackboxTestsDiscovery.cpp | 72 ++++++++++++++----- 1 file changed, 56 insertions(+), 16 deletions(-) diff --git a/test/blackbox/common/BlackboxTestsDiscovery.cpp b/test/blackbox/common/BlackboxTestsDiscovery.cpp index 0bec190d67f..69f3f0b14d0 100644 --- a/test/blackbox/common/BlackboxTestsDiscovery.cpp +++ b/test/blackbox/common/BlackboxTestsDiscovery.cpp @@ -1810,36 +1810,76 @@ TEST(Discovery, RemoteBuiltinEndpointHonoring) auto reader_test_transport = std::make_shared(); auto writer_test_transport = std::make_shared(); - uint32_t num_reader_heartbeat = 0; - uint32_t num_reader_acknack = 0; + uint32_t num_wlp_reader_heartbeat = 0; + uint32_t num_wlp_reader_acknack = 0; - reader_test_transport->drop_heartbeat_messages_filter_ = [&num_reader_heartbeat](CDRMessage_t&) + reader_test_transport->drop_heartbeat_messages_filter_ = [&num_wlp_reader_heartbeat](CDRMessage_t& msg) { - num_reader_heartbeat++; + auto old_pos = msg.pos; + msg.pos += 4; + eprosima::fastrtps::rtps::EntityId_t writer_entity_id; + eprosima::fastrtps::rtps::CDRMessage::readEntityId(&msg, &writer_entity_id); + msg.pos = old_pos; + + if (eprosima::fastrtps::rtps::c_EntityId_WriterLiveliness == writer_entity_id) + { + num_wlp_reader_heartbeat++; + } return false; }; - reader_test_transport->drop_ack_nack_messages_filter_ = [&num_reader_acknack](CDRMessage_t&) + reader_test_transport->drop_ack_nack_messages_filter_ = [&num_wlp_reader_acknack](CDRMessage_t& msg) { - num_reader_acknack++; + auto old_pos = msg.pos; + msg.pos += 4; + eprosima::fastrtps::rtps::EntityId_t writer_entity_id; + eprosima::fastrtps::rtps::CDRMessage::readEntityId(&msg, &writer_entity_id); + msg.pos = old_pos; + + if (eprosima::fastrtps::rtps::c_EntityId_WriterLiveliness == writer_entity_id) + { + num_wlp_reader_acknack++; + } return false; }; - uint32_t num_writer_heartbeat = 0; - uint32_t num_writer_acknack = 0; + reader_test_transport->interfaceWhiteList.push_back("127.0.0.1"); + + uint32_t num_wlp_writer_heartbeat = 0; + uint32_t num_wlp_writer_acknack = 0; - writer_test_transport->drop_heartbeat_messages_filter_ = [&num_writer_heartbeat](CDRMessage_t&) + writer_test_transport->drop_heartbeat_messages_filter_ = [&num_wlp_writer_heartbeat](CDRMessage_t& msg) { - num_writer_heartbeat++; + auto old_pos = msg.pos; + msg.pos += 4; + eprosima::fastrtps::rtps::EntityId_t writer_entity_id; + eprosima::fastrtps::rtps::CDRMessage::readEntityId(&msg, &writer_entity_id); + msg.pos = old_pos; + + if (eprosima::fastrtps::rtps::c_EntityId_WriterLiveliness == writer_entity_id) + { + num_wlp_writer_heartbeat++; + } return false; }; - writer_test_transport->drop_ack_nack_messages_filter_ = [&num_writer_acknack](CDRMessage_t&) + writer_test_transport->drop_ack_nack_messages_filter_ = [&num_wlp_writer_acknack](CDRMessage_t& msg) { - num_writer_acknack++; + auto old_pos = msg.pos; + msg.pos += 4; + eprosima::fastrtps::rtps::EntityId_t writer_entity_id; + eprosima::fastrtps::rtps::CDRMessage::readEntityId(&msg, &writer_entity_id); + msg.pos = old_pos; + + if (eprosima::fastrtps::rtps::c_EntityId_WriterLiveliness == writer_entity_id) + { + num_wlp_writer_acknack++; + } return false; }; + writer_test_transport->interfaceWhiteList.push_back("127.0.0.1"); + reader.disable_builtin_transport().add_user_transport_to_pparams(reader_test_transport). use_writer_liveliness_protocol(false); writer.disable_builtin_transport().add_user_transport_to_pparams(writer_test_transport); @@ -1856,10 +1896,10 @@ TEST(Discovery, RemoteBuiltinEndpointHonoring) std::this_thread::sleep_for(std::chrono::seconds(5)); - ASSERT_EQ(num_reader_heartbeat, 3u); - ASSERT_EQ(num_reader_acknack, 3u); - ASSERT_EQ(num_writer_heartbeat, 3u); - ASSERT_EQ(num_writer_acknack, 3u); + ASSERT_EQ(num_wlp_reader_heartbeat, 0u); + ASSERT_EQ(num_wlp_reader_acknack, 0u); + ASSERT_EQ(num_wlp_writer_heartbeat, 0u); + ASSERT_EQ(num_wlp_writer_acknack, 0u); } //! Regression test for redmine issue 10674 From f10790bf096a3557444a6529f93d117b6f6cc6ba Mon Sep 17 00:00:00 2001 From: Eduardo Ponz Segrelles Date: Fri, 11 Aug 2023 13:40:25 +0200 Subject: [PATCH 22/23] Fix DomainParticipant::register_remote_type return when negotiating type (#3786) * Refs #19359: Change ReturnCode when negotiating through type lookup service Signed-off-by: Eduardo Ponz * Refs #19359: Improve API reference Signed-off-by: Eduardo Ponz * Refs #19359: Apply suggestions Signed-off-by: Eduardo Ponz --------- Signed-off-by: Eduardo Ponz Signed-off-by: jimwang118 --- .../fastdds/dds/domain/DomainParticipant.hpp | 20 ++++++++++--------- .../fastdds/domain/DomainParticipantImpl.cpp | 2 +- .../fastdds/domain/DomainParticipantImpl.hpp | 17 ++++++++++++++++ 3 files changed, 29 insertions(+), 10 deletions(-) diff --git a/include/fastdds/dds/domain/DomainParticipant.hpp b/include/fastdds/dds/domain/DomainParticipant.hpp index f140fc65b81..7ac5f4b8998 100644 --- a/include/fastdds/dds/domain/DomainParticipant.hpp +++ b/include/fastdds/dds/domain/DomainParticipant.hpp @@ -850,19 +850,21 @@ class DomainParticipant : public Entity const fastrtps::types::TypeIdentifierSeq& in) const; /** - * Helps the user to solve all dependencies calling internally to the typelookup service - * and registers the resulting dynamic type. - * The registration will be perform asynchronously and the user will be notified through the - * given callback, which receives the type_name as unique argument. - * If the type is already registered, the function will return true, but the callback will not be called. - * If the given type_information is enough to build the type without using the typelookup service, - * it will return true and the callback will be never called. + * Helps the user to solve all dependencies calling internally to the type lookup service and + * registers the resulting dynamic type. + * The registration may be perform asynchronously, case in which the user will be notified + * through the given callback, which receives the type_name as unique argument. * * @param type_information * @param type_name * @param callback - * @return true if type is already available (callback will not be called). false if type isn't available yet - * (the callback will be called if negotiation is success, and ignored in other case). + * @return RETCODE_OK If the given type_information is enough to build the type without using + * the typelookup service (callback will not be called). + * @return RETCODE_OK if the given type is already available (callback will not be called). + * @return RETCODE_NO_DATA if type is not available yet (the callback will be called if + * negotiation is success, and ignored in other case). + * @return RETCODE_NOT_ENABLED if the DomainParticipant is not enabled. + * @return RETCODE_PRECONDITION_NOT_MET if the DomainParticipant type lookup service is disabled. */ RTPS_DllAPI ReturnCode_t register_remote_type( const fastrtps::types::TypeInformation& type_information, diff --git a/src/cpp/fastdds/domain/DomainParticipantImpl.cpp b/src/cpp/fastdds/domain/DomainParticipantImpl.cpp index 496b53969c6..7be131a0222 100644 --- a/src/cpp/fastdds/domain/DomainParticipantImpl.cpp +++ b/src/cpp/fastdds/domain/DomainParticipantImpl.cpp @@ -1801,7 +1801,7 @@ ReturnCode_t DomainParticipantImpl::register_remote_type( // Move the filled vector to the map parent_requests_.emplace(std::make_pair(requestId, std::move(vector))); - return ReturnCode_t::RETCODE_OK; + return ReturnCode_t::RETCODE_NO_DATA; } return ReturnCode_t::RETCODE_PRECONDITION_NOT_MET; } diff --git a/src/cpp/fastdds/domain/DomainParticipantImpl.hpp b/src/cpp/fastdds/domain/DomainParticipantImpl.hpp index d93792c241d..ae11876d443 100644 --- a/src/cpp/fastdds/domain/DomainParticipantImpl.hpp +++ b/src/cpp/fastdds/domain/DomainParticipantImpl.hpp @@ -489,6 +489,23 @@ class DomainParticipantImpl fastrtps::rtps::SampleIdentity get_types( const fastrtps::types::TypeIdentifierSeq& in) const; + /** + * Helps the user to solve all dependencies calling internally to the typelookup service and + * registers the resulting dynamic type. + * The registration may be perform asynchronously, case in which the user will be notified + * through the given callback, which receives the type_name as unique argument. + * + * @param type_information + * @param type_name + * @param callback + * @return RETCODE_OK If the given type_information is enough to build the type without using + * the typelookup service (callback will not be called). + * @return RETCODE_OK if the given type is already available (callback will not be called). + * @return RETCODE_NO_DATA if type is not available yet (the callback will be called if + * negotiation is success, and ignored in other case). + * @return RETCODE_NOT_ENABLED if the DomainParticipant is not enabled. + * @return RETCODE_PRECONDITION_NOT_MET if the DomainParticipant type lookup service is disabled. + */ ReturnCode_t register_remote_type( const fastrtps::types::TypeInformation& type_information, const std::string& type_name, From 64737e69f0e78af1c61c12e43f95bcb5c3354928 Mon Sep 17 00:00:00 2001 From: jimwang118 Date: Tue, 22 Aug 2023 06:46:29 +0000 Subject: [PATCH 23/23] Modify return value comparison Signed-off-by: jimwang118 --- include/fastrtps/utils/TimedMutex.hpp | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/include/fastrtps/utils/TimedMutex.hpp b/include/fastrtps/utils/TimedMutex.hpp index 76d40f1c6b5..ea361a16ff5 100644 --- a/include/fastrtps/utils/TimedMutex.hpp +++ b/include/fastrtps/utils/TimedMutex.hpp @@ -46,13 +46,6 @@ class TimedMutex #if defined(_MSC_FULL_VER) && _MSC_FULL_VER >= 193632528 using xtime = _timespec64; #endif - // _MSC_FULL_VER check - // On MSVC 19.38.32926.95 `_Thrd_success` was changed into `_Thrd_result::_Success`. - // See https://github.com/eProsima/Fast-DDS/issues/3783 - // See https://github.com/microsoft/STL/pull/3897 -#if defined(_MSC_FULL_VER) && _MSC_FULL_VER >= 193832926 - using _Thrd_success = _Thrd_result::_Success; -#endif public: TimedMutex() @@ -104,11 +97,11 @@ class TimedMutex nsecs -= secs; max_wait.tv_sec += secs.count(); max_wait.tv_nsec = (long)nsecs.count(); - return (_Thrd_success == _Mtx_timedlock(mutex_, (xtime*)&max_wait)); + return (0 == static_cast(_Mtx_timedlock(mutex_, (xtime*)&max_wait))); } else { - return (_Thrd_success == _Mtx_trylock(mutex_)); + return (0 == static_cast(_Mtx_trylock(mutex_))); } } @@ -153,7 +146,7 @@ class RecursiveTimedMutex bool try_lock() { - return (_Thrd_success == _Mtx_trylock(mutex_)); + return (0 == static_cast(_Mtx_trylock(mutex_))); } template @@ -179,11 +172,11 @@ class RecursiveTimedMutex nsecs -= secs; max_wait.tv_sec += secs.count(); max_wait.tv_nsec = (long)nsecs.count(); - return (_Thrd_success == _Mtx_timedlock(mutex_, (xtime*)&max_wait)); + return (0 == static_cast(_Mtx_timedlock(mutex_, (xtime*)&max_wait))); } else { - return (_Thrd_success == _Mtx_trylock(mutex_)); + return (0 == static_cast(_Mtx_trylock(mutex_))); } }