Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 11 commits into from
Apr 13, 2023
11 changes: 10 additions & 1 deletion src/cpp/fastdds/domain/DomainParticipantImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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");
MiguelCompany marked this conversation as resolved.
Show resolved Hide resolved
}

/* Fill physical data properties if they are found and empty */
std::string* property_value = fastrtps::rtps::PropertyPolicyHelper::find_property(
Expand Down Expand Up @@ -255,6 +258,12 @@ ReturnCode_t DomainParticipantImpl::enable()
// Should not have been previously enabled
assert(get_rtps_participant() == nullptr);

// Preconditions
if (guid_ == GUID_t::unknown())
{
return ReturnCode_t::RETCODE_PRECONDITION_NOT_MET;
}

richiware marked this conversation as resolved.
Show resolved Hide resolved
fastrtps::rtps::RTPSParticipantAttributes rtps_attr;
utils::set_attributes_from_qos(rtps_attr, qos_);
rtps_attr.participantID = participant_id_;
Expand Down
34 changes: 27 additions & 7 deletions src/cpp/rtps/RTPSDomain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
14 changes: 13 additions & 1 deletion src/cpp/rtps/RTPSDomainImpl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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-existent participant id.
richiware marked this conversation as resolved.
Show resolved Hide resolved
*
* @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);

Expand Down
3 changes: 2 additions & 1 deletion test/mock/rtps/RTPSDomainImpl/rtps/RTPSDomainImpl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,11 @@ class RTPSDomainImpl
return nullptr;
}

static void create_participant_guid(
static bool create_participant_guid(
int32_t& /*participant_id*/,
GUID_t& /*guid*/)
{
return true;
}

/**
Expand Down
61 changes: 61 additions & 0 deletions test/unittest/dds/participant/ParticipantTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3725,6 +3725,67 @@ 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_NE(participant2, nullptr);

ASSERT_EQ(ReturnCode_t::RETCODE_OK, participant1->enable());

ASSERT_EQ(ReturnCode_t::RETCODE_PRECONDITION_NOT_MET, participant2->enable());

// Destroy the first participant
ASSERT_EQ(DomainParticipantFactory::get_instance()->delete_participant(participant1), ReturnCode_t::RETCODE_OK);

// Destroy the second participant
ASSERT_EQ(DomainParticipantFactory::get_instance()->delete_participant(participant2), ReturnCode_t::RETCODE_OK);
MiguelCompany marked this conversation as resolved.
Show resolved Hide resolved

factory_qos.entity_factory().autoenable_created_entities = true;
ASSERT_EQ(ReturnCode_t::RETCODE_OK, DomainParticipantFactory::get_instance()->set_qos(factory_qos));
}
}

MiguelCompany marked this conversation as resolved.
Show resolved Hide resolved
} // namespace dds
} // namespace fastdds
} // namespace eprosima
Expand Down