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

[21620] Fix load profiles TSAN issue (#3281) #5212

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions include/fastdds/dds/domain/DomainParticipantFactory.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,13 @@ class DomainParticipantFactory
std::shared_ptr<fastrtps::rtps::RTPSDomainImpl> rtps_domain_;

std::shared_ptr<detail::LogResources> log_resources_;

/**
* This mutex guards the access to load the profiles.
* Is used to lock every thread that is trying to load the profiles, so only the first one loads it and
* until it is not finished the rest of them does not leave function \c load_profiles .
*/
mutable std::mutex default_xml_profiles_loaded_mtx_;
};

} // namespace dds
Expand Down
10 changes: 8 additions & 2 deletions src/cpp/fastdds/domain/DomainParticipantFactory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -321,11 +321,17 @@ ReturnCode_t DomainParticipantFactory::get_participant_qos_from_profile(

ReturnCode_t DomainParticipantFactory::load_profiles()
{
if (false == default_xml_profiles_loaded)
// NOTE: This could be done with a bool atomic to avoid taking the mutex in most cases, however the use of
// atomic over mutex is not deterministically better, and this way is easier to read and understand.

// Only load profiles once, if not, wait for profiles to be loaded
std::lock_guard<std::mutex> _(default_xml_profiles_loaded_mtx_);
if (!default_xml_profiles_loaded)
{
SystemInfo::set_environment_file();
XMLProfileManager::loadDefaultXMLFile();
// Only load profile once

// Change as already loaded
default_xml_profiles_loaded = true;

// Only change default participant qos when not explicitly set by the user
Expand Down
63 changes: 63 additions & 0 deletions test/blackbox/common/BlackboxTestsDiscovery.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2068,3 +2068,66 @@ TEST(Discovery, MulticastInitialPeer)
writer.wait_discovery();
reader.wait_discovery();
}

//! Regression test for redmine issue 17162
TEST(Discovery, MultipleXMLProfileLoad)
{
// These test may fail because one of the participants disappear before the other has found it.
// Thus, use condition variable so threads only finish once the discovery has taken place.
std::condition_variable cv;
std::mutex cv_mtx;
std::atomic<int> n_discoveries(0);

auto participant_creation_reader = [&]()
{
PubSubReader<HelloWorldPubSubType> participant(TEST_TOPIC_NAME);
participant.init();
participant.wait_discovery();

// Notify discovery has happen
{
std::unique_lock<std::mutex> lock(cv_mtx);
n_discoveries++;
}
cv.notify_all();

std::unique_lock<std::mutex> lock(cv_mtx);
cv.wait(
lock,
[&]()
{
return n_discoveries >= 2;
}
);
};

auto participant_creation_writer = [&]()
{
PubSubWriter<HelloWorldPubSubType> participant(TEST_TOPIC_NAME);
participant.init();
participant.wait_discovery();

// Notify discovery has happen
{
std::unique_lock<std::mutex> lock(cv_mtx);
n_discoveries++;
}
cv.notify_all();

std::unique_lock<std::mutex> lock(cv_mtx);
cv.wait(
lock,
[&]()
{
return n_discoveries >= 2;
}
);
};

// Start thread creating second participant
std::thread thr_reader(participant_creation_reader);
std::thread thr_writer(participant_creation_writer);

thr_reader.join();
thr_writer.join();
}