Enforce SHM ports open mode exclusions (#4635)

* Refs #20701. Added unit regression tests. Signed-off-by: Miguel Company <[email protected]> * Refs #20701. Added blackbox regression test. Signed-off-by: Miguel Company <[email protected]> * Refs #20701. Fix issue. Signed-off-by: Miguel Company <[email protected]> --------- Signed-off-by: Miguel Company <[email protected]>
eProsima · Apr 3, 2024 · 3d159dc · 3d159dc
1 parent decf689
commit 3d159dc
Show file tree

Hide file tree

Showing 3 changed files with 113 additions and 1 deletion.
diff --git 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<RobustExclusiveLock>(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<RobustSharedLock>(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
@@ -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<HelloWorldPubSubType> 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<SharedMemTransportDescriptor>();
+    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
@@ -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";