Fix segfault when creating two participant with same fixed id [18051]

src/cpp/fastdds/domain/DomainParticipantFactory.cpp

@@ -166,30 +166,38 @@ DomainParticipant* DomainParticipantFactory::create_participant(
             new eprosima::fastdds::statistics::dds::DomainParticipantImpl(dom_part, did, pqos, listen);
 #endif // FASTDDS_STATISTICS
 
+    if (fastrtps::rtps::GUID_t::unknown() != dom_part_impl->guid())
     {
-        std::lock_guard<std::mutex> guard(mtx_participants_);
-        using VectorIt = std::map<DomainId_t, std::vector<DomainParticipantImpl*>>::iterator;
-        VectorIt vector_it = participants_.find(did);
-
-        if (vector_it == participants_.end())
         {
-            // Insert the vector
-            std::vector<DomainParticipantImpl*> new_vector;
-            auto pair_it = participants_.insert(std::make_pair(did, std::move(new_vector)));
-            vector_it = pair_it.first;
-        }
+            std::lock_guard<std::mutex> guard(mtx_participants_);
+            using VectorIt = std::map<DomainId_t, std::vector<DomainParticipantImpl*>>::iterator;
+            VectorIt vector_it = participants_.find(did);
 
-        vector_it->second.push_back(dom_part_impl);
-    }
+            if (vector_it == participants_.end())
+            {
+                // Insert the vector
+                std::vector<DomainParticipantImpl*> new_vector;
+                auto pair_it = participants_.insert(std::make_pair(did, std::move(new_vector)));
+                vector_it = pair_it.first;
+            }
 
-    if (factory_qos_.entity_factory().autoenable_created_entities)
-    {
-        if (ReturnCode_t::RETCODE_OK != dom_part->enable())
+            vector_it->second.push_back(dom_part_impl);
+        }
+
+        if (factory_qos_.entity_factory().autoenable_created_entities)
         {
-            delete_participant(dom_part);
-            return nullptr;
+            if (ReturnCode_t::RETCODE_OK != dom_part->enable())
+            {
+                delete_participant(dom_part);
+                return nullptr;
+            }
         }
     }
+    else
+    {
+        delete dom_part_impl;
+        return nullptr;
+    }
 
     return dom_part;
 }

src/cpp/fastdds/domain/DomainParticipantImpl.cpp

@@ -127,7 +127,10 @@ DomainParticipantImpl::DomainParticipantImpl(
 
     // Pre calculate participant id and generated guid
     participant_id_ = qos_.wire_protocol().participant_id;
-    eprosima::fastrtps::rtps::RTPSDomainImpl::create_participant_guid(participant_id_, guid_);
+    if (!eprosima::fastrtps::rtps::RTPSDomainImpl::create_participant_guid(participant_id_, guid_))
+    {
+        EPROSIMA_LOG_ERROR(DOMAIN_PARTICIPANT, "Error generating GUID for participant");
+    }
 
     /* Fill physical data properties if they are found and empty */
     std::string* property_value = fastrtps::rtps::PropertyPolicyHelper::find_property(
@@ -254,6 +257,8 @@ ReturnCode_t DomainParticipantImpl::enable()
 {
     // Should not have been previously enabled
     assert(get_rtps_participant() == nullptr);
+    // Should not have failed assigning the GUID
+    assert (guid_ != GUID_t::unknown());
 
     fastrtps::rtps::RTPSParticipantAttributes rtps_attr;
     utils::set_attributes_from_qos(rtps_attr, qos_);

src/cpp/rtps/RTPSDomain.cpp

@@ -636,19 +636,39 @@ uint32_t RTPSDomainImpl::get_id_for_prefix(
     return ret;
 }
 
-void RTPSDomainImpl::create_participant_guid(
+bool RTPSDomainImpl::reserve_participant_id(
+        int32_t& participant_id)
+{
+    std::lock_guard<std::mutex> guard(m_mutex);
+    if (participant_id < 0)
+    {
+        participant_id = getNewId();
+    }
+    else
+    {
+        if (m_RTPSParticipantIDs[participant_id].reserved == true)
+        {
+            return false;
+        }
+        m_RTPSParticipantIDs[participant_id].reserved = true;
+    }
+
+    return true;
+}
+
+bool RTPSDomainImpl::create_participant_guid(
         int32_t& participant_id,
         GUID_t& guid)
 {
-    if (participant_id < 0)
+    bool ret_value = get_instance()->reserve_participant_id(participant_id);
+
+    if (ret_value)
     {
-        auto instance = get_instance();
-        std::lock_guard<std::mutex> guard(instance->m_mutex);
-        participant_id = instance->getNewId();
+        guid_prefix_create(participant_id, guid.guidPrefix);
+        guid.entityId = c_EntityId_RTPSParticipant;
     }
 
-    guid_prefix_create(participant_id, guid.guidPrefix);
-    guid.entityId = c_EntityId_RTPSParticipant;
+    return ret_value;
 }
 
 RTPSParticipantImpl* RTPSDomainImpl::find_local_participant(

src/cpp/rtps/RTPSDomainImpl.hpp

@@ -149,8 +149,10 @@ class RTPSDomainImpl
      * @param [in, out] participant_id   Participant identifier for which to generate the GUID.
      *                                   When negative, it will be modified to the first non-existent participant id.
      * @param [out]     guid             GUID corresponding to participant_id
+     *
+     * @return True value if guid was created. False in other case.
      */
-    static void create_participant_guid(
+    static bool create_participant_guid(
             int32_t& participant_id,
             GUID_t& guid);
 
@@ -216,6 +218,16 @@ class RTPSDomainImpl
             int32_t input_id,
             uint32_t& participant_id);
 
+    /**
+     * Reserves a participant id.
+     * @param [in, out] participant_id   Participant identifier for reservation.
+     *                                   When negative, it will be modified to the first non-reserved participant id.
+     *
+     * @return True value if reservation was possible. False in other case.
+     */
+    bool reserve_participant_id(
+            int32_t& participant_id);
+
     uint32_t get_id_for_prefix(
             uint32_t participant_id);
 

test/mock/dds/DomainParticipantImpl/fastdds/domain/DomainParticipantImpl.hpp

@@ -85,6 +85,7 @@ class DomainParticipantImpl
     {
         participant_->impl_ = this;
 
+        guid_.guidPrefix.value[11] = 1;
         eprosima::fastrtps::TopicAttributes top_attr;
         eprosima::fastrtps::xmlparser::XMLProfileManager::getDefaultTopicAttributes(top_attr);
         default_topic_qos_.history() = top_attr.historyQos;

test/mock/rtps/RTPSDomainImpl/rtps/RTPSDomainImpl.hpp

@@ -61,10 +61,12 @@ class RTPSDomainImpl
         return nullptr;
     }
 
-    static void create_participant_guid(
+    static bool create_participant_guid(
             int32_t& /*participant_id*/,
-            GUID_t& /*guid*/)
+            GUID_t& guid)
     {
+        guid.guidPrefix.value[11] = 1;
+        return true;
     }
 
     /**

test/unittest/dds/participant/ParticipantTests.cpp

@@ -3725,6 +3725,62 @@ TEST(ParticipantTests, UnsupportedMethods)
     ASSERT_EQ(DomainParticipantFactory::get_instance()->delete_participant(participant), ReturnCode_t::RETCODE_OK);
 }
 
+/*
+ * Regression test for redmine issue #18050.
+ *
+ * This test tries to create two participants with the same fixed id.
+ */
+TEST(ParticipantTests, TwoParticipantWithSameFixedId)
+{
+    // Test participants enabled from beginning
+    {
+        DomainParticipantQos participant_qos;
+        participant_qos.wire_protocol().participant_id = 1;
+
+        // Create the first participant
+        DomainParticipant* participant1 =
+                DomainParticipantFactory::get_instance()->create_participant(0, participant_qos);
+        ASSERT_NE(participant1, nullptr);
+
+        // Creating a second participant with the same fixed id should fail
+        DomainParticipant* participant2 =
+                DomainParticipantFactory::get_instance()->create_participant(0, participant_qos);
+        ASSERT_EQ(participant2, nullptr);
+
+        // Destroy the first participant
+        ASSERT_EQ(DomainParticipantFactory::get_instance()->delete_participant(participant1), ReturnCode_t::RETCODE_OK);
+    }
+
+    // Test participants disabled from beginning
+    {
+        DomainParticipantFactoryQos factory_qos;
+        ASSERT_EQ(ReturnCode_t::RETCODE_OK, DomainParticipantFactory::get_instance()->get_qos(factory_qos));
+        factory_qos.entity_factory().autoenable_created_entities = false;
+        ASSERT_EQ(ReturnCode_t::RETCODE_OK, DomainParticipantFactory::get_instance()->set_qos(factory_qos));
+
+        DomainParticipantQos participant_qos;
+        participant_qos.wire_protocol().participant_id = 1;
+
+        // Create the first participant
+        DomainParticipant* participant1 =
+                DomainParticipantFactory::get_instance()->create_participant(0, participant_qos);
+        ASSERT_NE(participant1, nullptr);
+
+        // Creating a second participant with the same fixed id should fail
+        DomainParticipant* participant2 =
+                DomainParticipantFactory::get_instance()->create_participant(0, participant_qos);
+        ASSERT_EQ(participant2, nullptr);
+
+        ASSERT_EQ(ReturnCode_t::RETCODE_OK, participant1->enable());
+
+        // Destroy the first participant
+        ASSERT_EQ(DomainParticipantFactory::get_instance()->delete_participant(participant1), ReturnCode_t::RETCODE_OK);
+
+        factory_qos.entity_factory().autoenable_created_entities = true;
+        ASSERT_EQ(ReturnCode_t::RETCODE_OK, DomainParticipantFactory::get_instance()->set_qos(factory_qos));
+    }
+}
+
 } // namespace dds
 } // namespace fastdds
 } // namespace eprosima