diff --git a/src/cpp/fastdds/domain/DomainParticipantFactory.cpp b/src/cpp/fastdds/domain/DomainParticipantFactory.cpp index e7aef7ed1e9..3003200b798 100644 --- a/src/cpp/fastdds/domain/DomainParticipantFactory.cpp +++ b/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 guard(mtx_participants_); - using VectorIt = std::map>::iterator; - VectorIt vector_it = participants_.find(did); - - if (vector_it == participants_.end()) { - // Insert the vector - std::vector new_vector; - auto pair_it = participants_.insert(std::make_pair(did, std::move(new_vector))); - vector_it = pair_it.first; - } + std::lock_guard guard(mtx_participants_); + using VectorIt = std::map>::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 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; } diff --git a/src/cpp/fastdds/domain/DomainParticipantImpl.cpp b/src/cpp/fastdds/domain/DomainParticipantImpl.cpp index a457b20e595..1e37ec63823 100644 --- a/src/cpp/fastdds/domain/DomainParticipantImpl.cpp +++ b/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_); diff --git a/src/cpp/rtps/RTPSDomain.cpp b/src/cpp/rtps/RTPSDomain.cpp index 17e80faa76e..d89595b498f 100644 --- a/src/cpp/rtps/RTPSDomain.cpp +++ b/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 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 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( diff --git a/src/cpp/rtps/RTPSDomainImpl.hpp b/src/cpp/rtps/RTPSDomainImpl.hpp index fff743ec2e3..fefa90b6001 100644 --- a/src/cpp/rtps/RTPSDomainImpl.hpp +++ b/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); diff --git a/test/mock/dds/DomainParticipantImpl/fastdds/domain/DomainParticipantImpl.hpp b/test/mock/dds/DomainParticipantImpl/fastdds/domain/DomainParticipantImpl.hpp index bb7fc5dd580..4251d19da5f 100644 --- a/test/mock/dds/DomainParticipantImpl/fastdds/domain/DomainParticipantImpl.hpp +++ b/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; diff --git a/test/mock/rtps/RTPSDomainImpl/rtps/RTPSDomainImpl.hpp b/test/mock/rtps/RTPSDomainImpl/rtps/RTPSDomainImpl.hpp index 55ee66304a8..3569092bd8b 100644 --- a/test/mock/rtps/RTPSDomainImpl/rtps/RTPSDomainImpl.hpp +++ b/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; } /** diff --git a/test/unittest/dds/participant/ParticipantTests.cpp b/test/unittest/dds/participant/ParticipantTests.cpp index 3b05ba58160..13f2abd2581 100644 --- a/test/unittest/dds/participant/ParticipantTests.cpp +++ b/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