From 729149c149fe1e2d70fcff440e55c6b558ea2e44 Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Wed, 3 Apr 2024 07:16:06 +0200 Subject: [PATCH] Enforce SHM ports open mode exclusions (#4635) * Refs #20701. Added unit regression tests. Signed-off-by: Miguel Company * Refs #20701. Added blackbox regression test. Signed-off-by: Miguel Company * Refs #20701. Fix issue. Signed-off-by: Miguel Company --------- Signed-off-by: Miguel Company (cherry picked from commit 3d159dc8c0e50902c24cbdc51a649d1f4de58e6a) --- .../transport/shared_mem/SharedMemGlobal.hpp | 29 +++++++++- .../common/BlackboxTestsTransportSHM.cpp | 55 +++++++++++++++++++ test/unittest/transport/SharedMemTests.cpp | 30 ++++++++++ 3 files changed, 113 insertions(+), 1 deletion(-) diff --git a/src/cpp/rtps/transport/shared_mem/SharedMemGlobal.hpp b/src/cpp/rtps/transport/shared_mem/SharedMemGlobal.hpp index c3e3a9a35a4..df602f5314b 100644 --- a/src/cpp/rtps/transport/shared_mem/SharedMemGlobal.hpp +++ b/src/cpp/rtps/transport/shared_mem/SharedMemGlobal.hpp @@ -899,12 +899,22 @@ class SharedMemGlobal void lock_read_exclusive() { + if (OpenMode::ReadShared == open_mode()) + { + throw std::runtime_error("port is opened ReadShared"); + } + std::string lock_name = std::string(node_->domain_name) + "_port" + std::to_string(node_->port_id) + "_el"; read_exclusive_lock_ = std::unique_ptr(new RobustExclusiveLock(lock_name)); } void lock_read_shared() { + if (OpenMode::ReadExclusive == open_mode()) + { + throw std::runtime_error("port is opened ReadExclusive"); + } + std::string lock_name = std::string(node_->domain_name) + "_port" + std::to_string(node_->port_id) + "_sl"; read_shared_lock_ = std::unique_ptr(new RobustSharedLock(lock_name)); } @@ -1125,7 +1135,24 @@ class SharedMemGlobal std::stringstream ss; ss << port_node->port_id << " (" << port_node->uuid.to_string() << - ") because is ReadExclusive locked"; + ") because it was already locked"; + + err_reason = ss.str(); + port.reset(); + } + } + else if (open_mode == Port::OpenMode::ReadShared) + { + try + { + port->lock_read_shared(); + } + catch (const std::exception&) + { + std::stringstream ss; + + ss << port_node->port_id << " (" << port_node->uuid.to_string() << + ") because it had a ReadExclusive lock"; err_reason = ss.str(); port.reset(); diff --git a/test/blackbox/common/BlackboxTestsTransportSHM.cpp b/test/blackbox/common/BlackboxTestsTransportSHM.cpp index 6b53a6598dd..448429735dd 100644 --- a/test/blackbox/common/BlackboxTestsTransportSHM.cpp +++ b/test/blackbox/common/BlackboxTestsTransportSHM.cpp @@ -16,6 +16,7 @@ #include "BlackboxTests.hpp" +#include "PubSubParticipant.hpp" #include "PubSubReader.hpp" #include "PubSubWriter.hpp" @@ -32,6 +33,8 @@ using namespace eprosima::fastrtps; using SharedMemTransportDescriptor = eprosima::fastdds::rtps::SharedMemTransportDescriptor; using test_SharedMemTransportDescriptor = eprosima::fastdds::rtps::test_SharedMemTransportDescriptor; +using Locator = eprosima::fastdds::rtps::Locator; +using LocatorList = eprosima::fastdds::rtps::LocatorList; TEST(SHM, TransportPubSub) { @@ -74,6 +77,58 @@ TEST(SHM, TransportPubSub) reader.wait_participant_undiscovery(); } +/* Regression test for redmine issue #20701 + * + * This test checks that the SHM transport will not listen on the same port + * in unicast and multicast at the same time. + * It does so by specifying custom default locators on a DataReader and then + * checking that the port mutation took place, thus producing a different port. + */ +TEST(SHM, SamePortUnicastMulticast) +{ + PubSubReader participant(TEST_TOPIC_NAME); + + Locator locator; + locator.kind = LOCATOR_KIND_SHM; + locator.port = global_port; + + LocatorList unicast_list; + LocatorList multicast_list; + + // Note: this is using knowledge of the SHM locator address format since + // SHMLocator is not exposed to the user. + locator.address[0] = 'U'; + unicast_list.push_back(locator); + + // Note: this is using knowledge of the SHM locator address format since + // SHMLocator is not exposed to the user. + locator.address[0] = 'M'; + multicast_list.push_back(locator); + + // Create the reader with the custom transport and locators + auto testTransport = std::make_shared(); + participant + .disable_builtin_transport() + .add_user_transport_to_pparams(testTransport) + .set_default_unicast_locators(unicast_list) + .set_default_multicast_locators(multicast_list) + .init(); + + ASSERT_TRUE(participant.isInitialized()); + + // Retrieve the listening locators and check that one port is different + LocatorList reader_locators; + participant.get_native_reader().get_listening_locators(reader_locators); + + ASSERT_EQ(reader_locators.size(), 2u); + auto it = reader_locators.begin(); + auto first_port = it->port; + ++it; + auto second_port = it->port; + EXPECT_NE(first_port, second_port); + EXPECT_TRUE(first_port == global_port || second_port == global_port); +} + // Regression test for redmine #19500 TEST(SHM, IgnoreNonExistentSegment) { diff --git a/test/unittest/transport/SharedMemTests.cpp b/test/unittest/transport/SharedMemTests.cpp index 0456160a9e1..35470bf9489 100644 --- a/test/unittest/transport/SharedMemTests.cpp +++ b/test/unittest/transport/SharedMemTests.cpp @@ -906,6 +906,36 @@ TEST_F(SHMTransportTests, port_lock_read_exclusive) port = shared_mem_manager->open_port(0, 1, 1000, SharedMemGlobal::Port::OpenMode::ReadExclusive); } +// Regression test for redmine issue #20701 +TEST_F(SHMTransportTests, port_lock_read_shared_then_exclusive) +{ + auto shared_mem_manager = SharedMemManager::create(domain_name); + + shared_mem_manager->remove_port(0); + + auto port = shared_mem_manager->open_port(0, 1, 1000, SharedMemGlobal::Port::OpenMode::ReadShared); + ASSERT_THROW(shared_mem_manager->open_port(0, 1, 1000, SharedMemGlobal::Port::OpenMode::ReadExclusive), + std::exception); + + port.reset(); + port = shared_mem_manager->open_port(0, 1, 1000, SharedMemGlobal::Port::OpenMode::ReadExclusive); +} + +// Regression test for redmine issue #20701 +TEST_F(SHMTransportTests, port_lock_read_exclusive_then_shared) +{ + auto shared_mem_manager = SharedMemManager::create(domain_name); + + shared_mem_manager->remove_port(0); + + auto port = shared_mem_manager->open_port(0, 1, 1000, SharedMemGlobal::Port::OpenMode::ReadExclusive); + ASSERT_THROW(shared_mem_manager->open_port(0, 1, 1000, SharedMemGlobal::Port::OpenMode::ReadShared), + std::exception); + + port.reset(); + port = shared_mem_manager->open_port(0, 1, 1000, SharedMemGlobal::Port::OpenMode::ReadShared); +} + TEST_F(SHMTransportTests, robust_exclusive_lock) { const std::string lock_name = "robust_exclusive_lock_test1_el";