From d863c2d723d7d8b6ae80ea5a317428fa972422bf Mon Sep 17 00:00:00 2001 From: Zakaria Talbi Lalmi Date: Mon, 8 Jun 2026 12:15:21 +0200 Subject: [PATCH 01/17] Protected ReaderLocator local_reader_ access with writer mutex. Signed-off-by: Zakaria Talbi Lalmi --- src/cpp/rtps/writer/StatefulWriter.cpp | 5 ++++- src/cpp/rtps/writer/StatelessWriter.cpp | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/cpp/rtps/writer/StatefulWriter.cpp b/src/cpp/rtps/writer/StatefulWriter.cpp index 84b292e2ffb..025c60899ec 100644 --- a/src/cpp/rtps/writer/StatefulWriter.cpp +++ b/src/cpp/rtps/writer/StatefulWriter.cpp @@ -445,11 +445,14 @@ bool StatefulWriter::intraprocess_heartbeat( bool liveliness) { bool returned_value = false; + std::unique_lock lockW(mp_mutex); LocalReaderPointer::Instance local_reader = reader_proxy->local_reader(); + lockW.unlock(); if (local_reader) { - std::unique_lock lockW(mp_mutex); + // std::unique_lock lockW(mp_mutex); + lockW.lock(); SequenceNumber_t first_seq = get_seq_num_min(); SequenceNumber_t last_seq = get_seq_num_max(); diff --git a/src/cpp/rtps/writer/StatelessWriter.cpp b/src/cpp/rtps/writer/StatelessWriter.cpp index c86014c09e6..9ab92e7d80a 100644 --- a/src/cpp/rtps/writer/StatelessWriter.cpp +++ b/src/cpp/rtps/writer/StatelessWriter.cpp @@ -317,6 +317,7 @@ bool StatelessWriter::intraprocess_delivery( CacheChange_t* change, ReaderLocator& reader_locator) { + std::lock_guard guard(mp_mutex); LocalReaderPointer::Instance local_reader = reader_locator.local_reader(); if (local_reader && From 1da68c1c563c3694a81a4b7046b456374d11b08a Mon Sep 17 00:00:00 2001 From: Zakaria Talbi Lalmi Date: Mon, 8 Jun 2026 14:39:07 +0200 Subject: [PATCH 02/17] Fix pthread_cond lifetime race in sample_lost_waitset tests. Signed-off-by: Zakaria Talbi Lalmi --- .../core/condition/StatusConditionImpl.cpp | 3 + test/blackbox/api/dds-pim/PubSubReader.hpp | 1 + .../common/DDSBlackboxTestsListeners.cpp | 442 +++++++++--------- 3 files changed, 231 insertions(+), 215 deletions(-) diff --git a/src/cpp/fastdds/core/condition/StatusConditionImpl.cpp b/src/cpp/fastdds/core/condition/StatusConditionImpl.cpp index 882e9a53210..53af5396402 100644 --- a/src/cpp/fastdds/core/condition/StatusConditionImpl.cpp +++ b/src/cpp/fastdds/core/condition/StatusConditionImpl.cpp @@ -67,12 +67,15 @@ ReturnCode_t StatusConditionImpl::set_enabled_statuses( const StatusMask& StatusConditionImpl::get_enabled_statuses() const { std::lock_guard guard(mutex_); + // TODO: same issue as get_raw_status, reference escapes the lock. return mask_; } const StatusMask& StatusConditionImpl::get_raw_status() const { std::lock_guard guard(mutex_); + // TODO: returning a reference to a mutex-protected member that escapes the lock is a data race. + // Fix requires changing the return type to StatusMask (by value), which is an API break. return status_; } diff --git a/test/blackbox/api/dds-pim/PubSubReader.hpp b/test/blackbox/api/dds-pim/PubSubReader.hpp index 177f873bf0f..2901970672c 100644 --- a/test/blackbox/api/dds-pim/PubSubReader.hpp +++ b/test/blackbox/api/dds-pim/PubSubReader.hpp @@ -2601,6 +2601,7 @@ class PubSubReaderWithWaitsets : public PubSubReader ~PubSubReaderWithWaitsets() override { + waitset_thread_.stop(); } void createSubscriber() override diff --git a/test/blackbox/common/DDSBlackboxTestsListeners.cpp b/test/blackbox/common/DDSBlackboxTestsListeners.cpp index 64dd9fcdb44..628f9e2751a 100644 --- a/test/blackbox/common/DDSBlackboxTestsListeners.cpp +++ b/test/blackbox/common/DDSBlackboxTestsListeners.cpp @@ -1306,57 +1306,59 @@ TEST(DDSStatus, sample_lost_re_dw_re_persistence_dr) */ TEST(DDSStatus, sample_lost_waitset_be_dw_be_dr) { - PubSubReaderWithWaitsets reader(TEST_TOPIC_NAME); - PubSubWriter writer(TEST_TOPIC_NAME); - std::mutex test_step_mtx; std::condition_variable test_step_cv; uint8_t test_step = 0; - writer.reliability(eprosima::fastdds::dds::BEST_EFFORT_RELIABILITY_QOS); - reader.reliability(eprosima::fastdds::dds::BEST_EFFORT_RELIABILITY_QOS); + { + PubSubReaderWithWaitsets reader(TEST_TOPIC_NAME); + PubSubWriter writer(TEST_TOPIC_NAME); - sample_lost_test_init(reader, writer, [&test_step_mtx, &test_step_cv, &test_step]( - const eprosima::fastdds::dds::SampleLostStatus& status) - { + writer.reliability(eprosima::fastdds::dds::BEST_EFFORT_RELIABILITY_QOS); + reader.reliability(eprosima::fastdds::dds::BEST_EFFORT_RELIABILITY_QOS); + + sample_lost_test_init(reader, writer, [&test_step_mtx, &test_step_cv, &test_step]( + const eprosima::fastdds::dds::SampleLostStatus& status) { - std::unique_lock lock(test_step_mtx); - if (0 == test_step && 3 == status.total_count && 3 == status.total_count_change) - { - ++test_step; - } - else if (1 == test_step && 4 == status.total_count && 1 == status.total_count_change) - { - ++test_step; - } - else if (2 == test_step && 5 == status.total_count && 1 == status.total_count_change) { - ++test_step; - } - else if (3 == test_step && 7 == status.total_count && 2 == status.total_count_change) - { - ++test_step; - } - else - { - test_step = 0; + std::unique_lock lock(test_step_mtx); + if (0 == test_step && 3 == status.total_count && 3 == status.total_count_change) + { + ++test_step; + } + else if (1 == test_step && 4 == status.total_count && 1 == status.total_count_change) + { + ++test_step; + } + else if (2 == test_step && 5 == status.total_count && 1 == status.total_count_change) + { + ++test_step; + } + else if (3 == test_step && 7 == status.total_count && 2 == status.total_count_change) + { + ++test_step; + } + else + { + test_step = 0; + } } - } - test_step_cv.notify_all(); - }); + test_step_cv.notify_all(); + }); - auto data = default_helloworld_data_generator(13); + auto data = default_helloworld_data_generator(13); - reader.startReception(data); - writer.send(data, 100); + reader.startReception(data); + writer.send(data, 100); - std::unique_lock lock(test_step_mtx); - test_step_cv.wait(lock, [&test_step]() - { - return 4 == test_step; - }); + std::unique_lock lock(test_step_mtx); + test_step_cv.wait(lock, [&test_step]() + { + return 4 == test_step; + }); + } } /*! @@ -1365,60 +1367,62 @@ TEST(DDSStatus, sample_lost_waitset_be_dw_be_dr) */ TEST(DDSStatus, sample_lost_waitset_be_dw_lj_be_dr) { - PubSubReaderWithWaitsets reader(TEST_TOPIC_NAME); - PubSubWriter writer(TEST_TOPIC_NAME); - - writer.reliability(eprosima::fastdds::dds::BEST_EFFORT_RELIABILITY_QOS); - sample_lost_test_dw_init(writer); - - auto data = default_helloworld_data_generator(4); - writer.send(data, 50); - std::mutex test_step_mtx; std::condition_variable test_step_cv; uint8_t test_step = 0; - reader.reliability(eprosima::fastdds::dds::BEST_EFFORT_RELIABILITY_QOS); - sample_lost_test_dr_init(reader, [&test_step_mtx, &test_step_cv, &test_step]( - const eprosima::fastdds::dds::SampleLostStatus& status) - { + { + PubSubReaderWithWaitsets reader(TEST_TOPIC_NAME); + PubSubWriter writer(TEST_TOPIC_NAME); + + writer.reliability(eprosima::fastdds::dds::BEST_EFFORT_RELIABILITY_QOS); + sample_lost_test_dw_init(writer); + + auto data = default_helloworld_data_generator(4); + writer.send(data, 50); + + reader.reliability(eprosima::fastdds::dds::BEST_EFFORT_RELIABILITY_QOS); + sample_lost_test_dr_init(reader, [&test_step_mtx, &test_step_cv, &test_step]( + const eprosima::fastdds::dds::SampleLostStatus& status) { - std::unique_lock lock(test_step_mtx); - if (0 == test_step && 1 == status.total_count && 1 == status.total_count_change) - { - ++test_step; - } - else if (1 == test_step && 2 == status.total_count && 1 == status.total_count_change) - { - ++test_step; - } - else if (2 == test_step && 4 == status.total_count && 2 == status.total_count_change) - { - ++test_step; - } - else { - test_step = 0; + std::unique_lock lock(test_step_mtx); + if (0 == test_step && 1 == status.total_count && 1 == status.total_count_change) + { + ++test_step; + } + else if (1 == test_step && 2 == status.total_count && 1 == status.total_count_change) + { + ++test_step; + } + else if (2 == test_step && 4 == status.total_count && 2 == status.total_count_change) + { + ++test_step; + } + else + { + test_step = 0; + } } - } - test_step_cv.notify_all(); - }); + test_step_cv.notify_all(); + }); - // Wait for discovery. - writer.wait_discovery(); - reader.wait_discovery(); + // Wait for discovery. + writer.wait_discovery(); + reader.wait_discovery(); - data = default_helloworld_data_generator(9); + data = default_helloworld_data_generator(9); - reader.startReception(data); - writer.send(data, 100); + reader.startReception(data); + writer.send(data, 100); - std::unique_lock lock(test_step_mtx); - test_step_cv.wait(lock, [&test_step]() - { - return 3 == test_step; - }); + std::unique_lock lock(test_step_mtx); + test_step_cv.wait(lock, [&test_step]() + { + return 3 == test_step; + }); + } } /*! @@ -1426,39 +1430,41 @@ TEST(DDSStatus, sample_lost_waitset_be_dw_lj_be_dr) */ TEST(DDSStatus, sample_lost_waitset_re_dw_re_dr) { - PubSubReaderWithWaitsets reader(TEST_TOPIC_NAME); - PubSubWriter writer(TEST_TOPIC_NAME); - - writer.reliability(eprosima::fastdds::dds::RELIABLE_RELIABILITY_QOS); - reader.reliability(eprosima::fastdds::dds::RELIABLE_RELIABILITY_QOS); - std::mutex test_step_mtx; std::condition_variable test_step_cv; int32_t test_count = 0; int32_t test_count_change_accum = 0; - sample_lost_test_init(reader, writer, [&test_step_mtx, &test_step_cv, &test_count, &test_count_change_accum]( - const eprosima::fastdds::dds::SampleLostStatus& status) - { + { + PubSubReaderWithWaitsets reader(TEST_TOPIC_NAME); + PubSubWriter writer(TEST_TOPIC_NAME); + + writer.reliability(eprosima::fastdds::dds::RELIABLE_RELIABILITY_QOS); + reader.reliability(eprosima::fastdds::dds::RELIABLE_RELIABILITY_QOS); + + sample_lost_test_init(reader, writer, [&test_step_mtx, &test_step_cv, &test_count, &test_count_change_accum]( + const eprosima::fastdds::dds::SampleLostStatus& status) { - std::unique_lock lock(test_step_mtx); - test_count = status.total_count; - test_count_change_accum += status.total_count_change; - } + { + std::unique_lock lock(test_step_mtx); + test_count = status.total_count; + test_count_change_accum += status.total_count_change; + } - test_step_cv.notify_all(); - }); + test_step_cv.notify_all(); + }); - auto data = default_helloworld_data_generator(13); + auto data = default_helloworld_data_generator(13); - reader.startReception(data); - writer.send(data, 100); + reader.startReception(data); + writer.send(data, 100); - std::unique_lock lock(test_step_mtx); - test_step_cv.wait(lock, [&test_count, &test_count_change_accum]() - { - return 7 == test_count && 7 == test_count_change_accum; - }); + std::unique_lock lock(test_step_mtx); + test_step_cv.wait(lock, [&test_count, &test_count_change_accum]() + { + return 7 == test_count && 7 == test_count_change_accum; + }); + } } /*! @@ -1467,48 +1473,50 @@ TEST(DDSStatus, sample_lost_waitset_re_dw_re_dr) */ TEST(DDSStatus, sample_lost_waitset_re_dw_lj_re_dr) { - PubSubReaderWithWaitsets reader(TEST_TOPIC_NAME); - PubSubWriter writer(TEST_TOPIC_NAME); - - writer.reliability(eprosima::fastdds::dds::RELIABLE_RELIABILITY_QOS); - sample_lost_test_dw_init(writer); - - auto data = default_helloworld_data_generator(4); - writer.send(data, 50); - std::mutex test_step_mtx; std::condition_variable test_step_cv; int32_t test_count = 0; int32_t test_count_change_accum = 0; - reader.reliability(eprosima::fastdds::dds::RELIABLE_RELIABILITY_QOS); - sample_lost_test_dr_init(reader, [&test_step_mtx, &test_step_cv, &test_count, &test_count_change_accum]( - const eprosima::fastdds::dds::SampleLostStatus& status) - { + { + PubSubReaderWithWaitsets reader(TEST_TOPIC_NAME); + PubSubWriter writer(TEST_TOPIC_NAME); + + writer.reliability(eprosima::fastdds::dds::RELIABLE_RELIABILITY_QOS); + sample_lost_test_dw_init(writer); + + auto data = default_helloworld_data_generator(4); + writer.send(data, 50); + + reader.reliability(eprosima::fastdds::dds::RELIABLE_RELIABILITY_QOS); + sample_lost_test_dr_init(reader, [&test_step_mtx, &test_step_cv, &test_count, &test_count_change_accum]( + const eprosima::fastdds::dds::SampleLostStatus& status) { - std::unique_lock lock(test_step_mtx); - test_count = status.total_count; - test_count_change_accum += status.total_count_change; - } + { + std::unique_lock lock(test_step_mtx); + test_count = status.total_count; + test_count_change_accum += status.total_count_change; + } - test_step_cv.notify_all(); - }); + test_step_cv.notify_all(); + }); - // Wait for discovery. - writer.wait_discovery(); - reader.wait_discovery(); - std::this_thread::sleep_for(std::chrono::seconds(1)); // Make sure the GAP message are received for the fourth sample. + // Wait for discovery. + writer.wait_discovery(); + reader.wait_discovery(); + std::this_thread::sleep_for(std::chrono::seconds(1)); // Make sure the GAP message are received for the fourth sample. - data = default_helloworld_data_generator(9); + data = default_helloworld_data_generator(9); - reader.startReception(data); - writer.send(data, 100); + reader.startReception(data); + writer.send(data, 100); - std::unique_lock lock(test_step_mtx); - test_step_cv.wait(lock, [&test_count, &test_count_change_accum]() - { - return 4 == test_count && 4 == test_count_change_accum; - }); + std::unique_lock lock(test_step_mtx); + test_step_cv.wait(lock, [&test_count, &test_count_change_accum]() + { + return 4 == test_count && 4 == test_count_change_accum; + }); + } } /*! @@ -1516,56 +1524,58 @@ TEST(DDSStatus, sample_lost_waitset_re_dw_lj_re_dr) */ TEST(DDSStatus, sample_lost_waitset_re_dw_be_dr) { - PubSubReaderWithWaitsets reader(TEST_TOPIC_NAME); - PubSubWriter writer(TEST_TOPIC_NAME); - - writer.reliability(eprosima::fastdds::dds::RELIABLE_RELIABILITY_QOS); - reader.reliability(eprosima::fastdds::dds::BEST_EFFORT_RELIABILITY_QOS); - std::mutex test_step_mtx; std::condition_variable test_step_cv; uint8_t test_step = 0; - sample_lost_test_init(reader, writer, [&test_step_mtx, &test_step_cv, &test_step]( - const eprosima::fastdds::dds::SampleLostStatus& status) - { + { + PubSubReaderWithWaitsets reader(TEST_TOPIC_NAME); + PubSubWriter writer(TEST_TOPIC_NAME); + + writer.reliability(eprosima::fastdds::dds::RELIABLE_RELIABILITY_QOS); + reader.reliability(eprosima::fastdds::dds::BEST_EFFORT_RELIABILITY_QOS); + + sample_lost_test_init(reader, writer, [&test_step_mtx, &test_step_cv, &test_step]( + const eprosima::fastdds::dds::SampleLostStatus& status) { - std::unique_lock lock(test_step_mtx); - if (0 == test_step && 3 == status.total_count && 3 == status.total_count_change) - { - ++test_step; - } - else if (1 == test_step && 4 == status.total_count && 1 == status.total_count_change) { - ++test_step; - } - else if (2 == test_step && 5 == status.total_count && 1 == status.total_count_change) - { - ++test_step; - } - else if (3 == test_step && 7 == status.total_count && 2 == status.total_count_change) - { - ++test_step; - } - else - { - test_step = 0; + std::unique_lock lock(test_step_mtx); + if (0 == test_step && 3 == status.total_count && 3 == status.total_count_change) + { + ++test_step; + } + else if (1 == test_step && 4 == status.total_count && 1 == status.total_count_change) + { + ++test_step; + } + else if (2 == test_step && 5 == status.total_count && 1 == status.total_count_change) + { + ++test_step; + } + else if (3 == test_step && 7 == status.total_count && 2 == status.total_count_change) + { + ++test_step; + } + else + { + test_step = 0; + } } - } - test_step_cv.notify_all(); - }); + test_step_cv.notify_all(); + }); - auto data = default_helloworld_data_generator(13); + auto data = default_helloworld_data_generator(13); - reader.startReception(data); - writer.send(data, 100); + reader.startReception(data); + writer.send(data, 100); - std::unique_lock lock(test_step_mtx); - test_step_cv.wait(lock, [&test_step]() - { - return 4 == test_step; - }); + std::unique_lock lock(test_step_mtx); + test_step_cv.wait(lock, [&test_step]() + { + return 4 == test_step; + }); + } } /*! @@ -1574,60 +1584,62 @@ TEST(DDSStatus, sample_lost_waitset_re_dw_be_dr) */ TEST(DDSStatus, sample_lost_waitset_re_dw_lj_be_dr) { - PubSubReaderWithWaitsets reader(TEST_TOPIC_NAME); - PubSubWriter writer(TEST_TOPIC_NAME); - - writer.reliability(eprosima::fastdds::dds::RELIABLE_RELIABILITY_QOS); - sample_lost_test_dw_init(writer); - - auto data = default_helloworld_data_generator(4); - writer.send(data, 50); - std::mutex test_step_mtx; std::condition_variable test_step_cv; uint8_t test_step = 0; - reader.reliability(eprosima::fastdds::dds::BEST_EFFORT_RELIABILITY_QOS); - sample_lost_test_dr_init(reader, [&test_step_mtx, &test_step_cv, &test_step]( - const eprosima::fastdds::dds::SampleLostStatus& status) - { + { + PubSubReaderWithWaitsets reader(TEST_TOPIC_NAME); + PubSubWriter writer(TEST_TOPIC_NAME); + + writer.reliability(eprosima::fastdds::dds::RELIABLE_RELIABILITY_QOS); + sample_lost_test_dw_init(writer); + + auto data = default_helloworld_data_generator(4); + writer.send(data, 50); + + reader.reliability(eprosima::fastdds::dds::BEST_EFFORT_RELIABILITY_QOS); + sample_lost_test_dr_init(reader, [&test_step_mtx, &test_step_cv, &test_step]( + const eprosima::fastdds::dds::SampleLostStatus& status) { - std::unique_lock lock(test_step_mtx); - if (0 == test_step && 1 == status.total_count && 1 == status.total_count_change) { - ++test_step; - } - else if (1 == test_step && 2 == status.total_count && 1 == status.total_count_change) - { - ++test_step; - } - else if (2 == test_step && 4 == status.total_count && 2 == status.total_count_change) - { - ++test_step; - } - else - { - test_step = 0; + std::unique_lock lock(test_step_mtx); + if (0 == test_step && 1 == status.total_count && 1 == status.total_count_change) + { + ++test_step; + } + else if (1 == test_step && 2 == status.total_count && 1 == status.total_count_change) + { + ++test_step; + } + else if (2 == test_step && 4 == status.total_count && 2 == status.total_count_change) + { + ++test_step; + } + else + { + test_step = 0; + } } - } - test_step_cv.notify_all(); - }); + test_step_cv.notify_all(); + }); - // Wait for discovery. - writer.wait_discovery(); - reader.wait_discovery(); + // Wait for discovery. + writer.wait_discovery(); + reader.wait_discovery(); - data = default_helloworld_data_generator(9); + data = default_helloworld_data_generator(9); - reader.startReception(data); - writer.send(data, 100); + reader.startReception(data); + writer.send(data, 100); - std::unique_lock lock(test_step_mtx); - test_step_cv.wait(lock, [&test_step]() - { - return 3 == test_step; - }); + std::unique_lock lock(test_step_mtx); + test_step_cv.wait(lock, [&test_step]() + { + return 3 == test_step; + }); + } } /* From c401a68b2ee6e4bcf76cae5bf899c3bf6f800ebd Mon Sep 17 00:00:00 2001 From: Zakaria Talbi Lalmi Date: Mon, 8 Jun 2026 15:20:05 +0200 Subject: [PATCH 03/17] Make CustomPayloadPool counters thread-safe. Signed-off-by: Zakaria Talbi Lalmi --- test/blackbox/api/dds-pim/CustomPayloadPool.hpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/blackbox/api/dds-pim/CustomPayloadPool.hpp b/test/blackbox/api/dds-pim/CustomPayloadPool.hpp index acd57808de8..267f6675a5d 100644 --- a/test/blackbox/api/dds-pim/CustomPayloadPool.hpp +++ b/test/blackbox/api/dds-pim/CustomPayloadPool.hpp @@ -20,6 +20,7 @@ #define DDS_CUSTOM_PAYLOAD_POOL_HPP #include +#include #include #include #include @@ -95,8 +96,8 @@ class CustomPayloadPool : public eprosima::fastdds::rtps::IPayloadPool return true; } - uint32_t requested_payload_count = 0; - uint32_t returned_payload_count = 0; + std::atomic requested_payload_count {0}; + std::atomic returned_payload_count {0}; }; From c1c11c2cb262725dd45aab12fcf2ca3236097592 Mon Sep 17 00:00:00 2001 From: Zakaria Talbi Lalmi Date: Mon, 8 Jun 2026 15:39:23 +0200 Subject: [PATCH 04/17] Fix participant_matched_ data race in PubSubWriter. Signed-off-by: Zakaria Talbi Lalmi --- test/blackbox/api/dds-pim/PubSubReader.hpp | 3 ++- test/blackbox/api/dds-pim/PubSubWriter.hpp | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/test/blackbox/api/dds-pim/PubSubReader.hpp b/test/blackbox/api/dds-pim/PubSubReader.hpp index 2901970672c..c73ff33a82e 100644 --- a/test/blackbox/api/dds-pim/PubSubReader.hpp +++ b/test/blackbox/api/dds-pim/PubSubReader.hpp @@ -1973,6 +1973,7 @@ class PubSubReader unsigned int get_participants_matched() const { + std::unique_lock lock(mutexDiscovery_); return participant_matched_; } @@ -2272,7 +2273,7 @@ class PubSubReader std::list total_msgs_; std::mutex mutex_; std::condition_variable cv_; - std::mutex mutexDiscovery_; + mutable std::mutex mutexDiscovery_; std::condition_variable cvDiscovery_; std::atomic matched_; unsigned int participant_matched_; diff --git a/test/blackbox/api/dds-pim/PubSubWriter.hpp b/test/blackbox/api/dds-pim/PubSubWriter.hpp index b758518de3f..72e755fd8d9 100644 --- a/test/blackbox/api/dds-pim/PubSubWriter.hpp +++ b/test/blackbox/api/dds-pim/PubSubWriter.hpp @@ -1794,6 +1794,7 @@ class PubSubWriter unsigned int get_participants_matched() const { + std::unique_lock lock(mutexDiscovery_); return participant_matched_; } @@ -2208,7 +2209,7 @@ class PubSubWriter eprosima::fastdds::rtps::GUID_t datawriter_guid_; bool initialized_; bool use_domain_id_from_profile_; - std::mutex mutexDiscovery_; + mutable std::mutex mutexDiscovery_; std::condition_variable cv_; std::atomic matched_; unsigned int participant_matched_; From 99bd482e5c7aed90e81f03893b14022caf8fe853 Mon Sep 17 00:00:00 2001 From: Zakaria Talbi Lalmi Date: Mon, 8 Jun 2026 15:48:37 +0200 Subject: [PATCH 05/17] Fix vptr race in reception_timestamp_for_resent_samples. Signed-off-by: Zakaria Talbi Lalmi --- test/blackbox/common/DDSBlackboxTestsDataReader.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/blackbox/common/DDSBlackboxTestsDataReader.cpp b/test/blackbox/common/DDSBlackboxTestsDataReader.cpp index 7b3d4ae4b53..9e93cb54007 100644 --- a/test/blackbox/common/DDSBlackboxTestsDataReader.cpp +++ b/test/blackbox/common/DDSBlackboxTestsDataReader.cpp @@ -1421,6 +1421,9 @@ TEST(DDSDataReader, reception_timestamp_for_resent_samples) // Wait for the reception of all samples ASSERT_EQ(reader.block_for_all(std::chrono::seconds(5)), 3u); + // Avoid data race between the destructor and on_data_available + reader.destroy(); + // Check timestamps. reception_timestamps map is accesed by index of HelloWorld data ASSERT_EQ(reader.reception_timestamps.size(), 3u); auto reception_ts_1 = reader.reception_timestamps[1]; From 5e92a8b3d083c84d3ddb21944348aa246acab556 Mon Sep 17 00:00:00 2001 From: Zakaria Talbi Lalmi Date: Mon, 8 Jun 2026 16:42:25 +0200 Subject: [PATCH 06/17] Converted deadline counters to atomic in PubSub Writer/Reader. Signed-off-by: Zakaria Talbi Lalmi --- test/blackbox/api/dds-pim/PubSubReader.hpp | 2 +- test/blackbox/api/dds-pim/PubSubWriter.hpp | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/test/blackbox/api/dds-pim/PubSubReader.hpp b/test/blackbox/api/dds-pim/PubSubReader.hpp index c73ff33a82e..ced2b119d84 100644 --- a/test/blackbox/api/dds-pim/PubSubReader.hpp +++ b/test/blackbox/api/dds-pim/PubSubReader.hpp @@ -2577,7 +2577,7 @@ class PubSubReaderWithWaitsets : public PubSubReader eprosima::fastdds::dds::GuardCondition guard_condition_; //! Number of times deadline was missed - unsigned int times_deadline_missed_ = 0; + std::atomic times_deadline_missed_ {0}; //! The timeout for the wait operation eprosima::fastdds::dds::Duration_t timeout_; diff --git a/test/blackbox/api/dds-pim/PubSubWriter.hpp b/test/blackbox/api/dds-pim/PubSubWriter.hpp index 72e755fd8d9..640a5f82e81 100644 --- a/test/blackbox/api/dds-pim/PubSubWriter.hpp +++ b/test/blackbox/api/dds-pim/PubSubWriter.hpp @@ -20,6 +20,7 @@ #ifndef _TEST_BLACKBOX_PUBSUBWRITER_HPP_ #define _TEST_BLACKBOX_PUBSUBWRITER_HPP_ +#include #include #include #include @@ -2421,10 +2422,10 @@ class PubSubWriterWithWaitsets : public PubSubWriter eprosima::fastdds::dds::GuardCondition guard_condition_; //! The number of times deadline was missed - unsigned int times_deadline_missed_ = 0; + std::atomic times_deadline_missed_ {0}; //! The number of times liveliness was lost - unsigned int times_liveliness_lost_ = 0; + std::atomic times_liveliness_lost_ {0}; //! The timeout for the wait operation eprosima::fastdds::dds::Duration_t timeout_; From 9812b7922be61dd87a3ad5ac832955809ad377bf Mon Sep 17 00:00:00 2001 From: Zakaria Talbi Lalmi Date: Thu, 11 Jun 2026 09:16:01 +0200 Subject: [PATCH 07/17] Secure access for get_id() Signed-off-by: Zakaria Talbi Lalmi --- src/cpp/utils/SystemInfo.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/cpp/utils/SystemInfo.cpp b/src/cpp/utils/SystemInfo.cpp index b44d015d69a..3fb86873708 100644 --- a/src/cpp/utils/SystemInfo.cpp +++ b/src/cpp/utils/SystemInfo.cpp @@ -139,10 +139,12 @@ fastdds::dds::ReturnCode_t SystemInfo::get_username( return fastdds::dds::RETCODE_OK; #else uid_t user_id = geteuid(); - struct passwd* pwd = getpwuid(user_id); - if (pwd != nullptr) + struct passwd pwd {}; + struct passwd* result = nullptr; + char buf[1024]; + if (getpwuid_r(user_id, &pwd, buf, sizeof(buf), &result) == 0 && result != nullptr) { - username = pwd->pw_name; + username = pwd.pw_name; if (!username.empty()) { return fastdds::dds::RETCODE_OK; From 5c2bf582c33f30f456cbe5654f68e11a5309d519 Mon Sep 17 00:00:00 2001 From: Zakaria Talbi Lalmi Date: Thu, 11 Jun 2026 09:48:16 +0200 Subject: [PATCH 08/17] Fiz tzset_internal data race in PermissionsParser Signed-off-by: Zakaria Talbi Lalmi --- .../security/accesscontrol/PermissionsParser.cpp | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/cpp/security/accesscontrol/PermissionsParser.cpp b/src/cpp/security/accesscontrol/PermissionsParser.cpp index 9ef5335dfbd..6e248da94ce 100644 --- a/src/cpp/security/accesscontrol/PermissionsParser.cpp +++ b/src/cpp/security/accesscontrol/PermissionsParser.cpp @@ -17,10 +17,16 @@ #include #include +#include #include +#include #include #include +namespace { +std::mutex mktime_mutex; +} // namespace + #if TIXML2_MAJOR_VERSION >= 6 #define PRINTLINE(node) node->GetLineNum() #define PRINTLINEPLUSONE(node) node->GetLineNum() + 1 @@ -339,7 +345,10 @@ bool PermissionsParser::parse_validity( if (!stream.fail()) { - validity.not_before = std::mktime(&time); + { + std::lock_guard lock(mktime_mutex); + validity.not_before = std::mktime(&time); + } #endif // if _MSC_VER != 1800 tinyxml2::XMLElement* old_node = node; @@ -360,7 +369,10 @@ bool PermissionsParser::parse_validity( if (!stream.fail()) { - validity.not_after = std::mktime(&time); + { + std::lock_guard lock(mktime_mutex); + validity.not_after = std::mktime(&time); + } #endif // if _MSC_VER != 1800 returned_value = true; } From 4ffb1e52d9fabbbb82d709785933e9fe2e63e525 Mon Sep 17 00:00:00 2001 From: Zakaria Talbi Lalmi Date: Mon, 15 Jun 2026 15:57:01 +0200 Subject: [PATCH 09/17] Fix data race on MonitorServiceConsumer Signed-off-by: Zakaria Talbi Lalmi --- test/blackbox/common/DDSBlackboxTestsMonitorService.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/test/blackbox/common/DDSBlackboxTestsMonitorService.cpp b/test/blackbox/common/DDSBlackboxTestsMonitorService.cpp index 3abc191840f..d6f004ad7e8 100644 --- a/test/blackbox/common/DDSBlackboxTestsMonitorService.cpp +++ b/test/blackbox/common/DDSBlackboxTestsMonitorService.cpp @@ -686,6 +686,7 @@ class MonitorServiceConsumer : protected PubSubReader unsigned int get_participants_matched() { + std::unique_lock lock(mutexDiscovery_); return participant_matched_; } From d9409d37a6daf704e74df87d000f131fea4a2a5e Mon Sep 17 00:00:00 2001 From: Zakaria Talbi Lalmi Date: Mon, 15 Jun 2026 16:00:08 +0200 Subject: [PATCH 10/17] Fix data race on DDSBboxTestsMonitorService: get_cb_count_of Signed-off-by: Zakaria Talbi Lalmi --- test/blackbox/common/DDSBlackboxTestsMonitorService.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/blackbox/common/DDSBlackboxTestsMonitorService.cpp b/test/blackbox/common/DDSBlackboxTestsMonitorService.cpp index d6f004ad7e8..c939632c9c0 100644 --- a/test/blackbox/common/DDSBlackboxTestsMonitorService.cpp +++ b/test/blackbox/common/DDSBlackboxTestsMonitorService.cpp @@ -183,7 +183,7 @@ class MonitorServiceParticipant return statistics_part_->disable_monitor_service(); } - const uint32_t& get_cb_count( + uint32_t get_cb_count( CallbackIndex cb_idx) { return listener_.get_cb_count_of(cb_idx); @@ -460,7 +460,7 @@ class MonitorServiceParticipant std::cout << "on_sample_lost " << reader->guid() << " total_count " << status.total_count << std::endl; } - const uint32_t& get_cb_count_of( + uint32_t get_cb_count_of( CallbackIndex cb_idx) { std::unique_lock lock(mtx_); From 7ff07f8e2aaf9e912a22121e172cfc158f6d3303 Mon Sep 17 00:00:00 2001 From: Zakaria Talbi Lalmi Date: Mon, 15 Jun 2026 16:03:33 +0200 Subject: [PATCH 11/17] Fix data race on PubSubReader mock, return by value instead of reference Signed-off-by: Zakaria Talbi Lalmi --- test/blackbox/api/dds-pim/PubSubReader.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/blackbox/api/dds-pim/PubSubReader.hpp b/test/blackbox/api/dds-pim/PubSubReader.hpp index ced2b119d84..0498d2d499c 100644 --- a/test/blackbox/api/dds-pim/PubSubReader.hpp +++ b/test/blackbox/api/dds-pim/PubSubReader.hpp @@ -1924,7 +1924,7 @@ class PubSubReader return status; } - const eprosima::fastdds::dds::LivelinessChangedStatus& liveliness_changed_status() + eprosima::fastdds::dds::LivelinessChangedStatus liveliness_changed_status() { std::unique_lock lock(liveliness_mutex_); From 6a1d6a60843208efdea2b6eb40d4482940934543 Mon Sep 17 00:00:00 2001 From: Zakaria Talbi Lalmi Date: Mon, 15 Jun 2026 16:07:13 +0200 Subject: [PATCH 12/17] Fix data races in MockReceiverResource by making shared fields atomic Signed-off-by: Zakaria Talbi Lalmi --- test/unittest/transport/mock/MockReceiverResource.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/unittest/transport/mock/MockReceiverResource.h b/test/unittest/transport/mock/MockReceiverResource.h index 77260abb35c..dcb17af27ff 100644 --- a/test/unittest/transport/mock/MockReceiverResource.h +++ b/test/unittest/transport/mock/MockReceiverResource.h @@ -15,6 +15,7 @@ #ifndef MOCK_RECEIVER_STUFF_H #define MOCK_RECEIVER_STUFF_H +#include #include #include @@ -45,7 +46,7 @@ class MockReceiverResource : public ReceiverResource const Locator_t& locator); ~MockReceiverResource(); MessageReceiver* CreateMessageReceiver() override; - MockMessageReceiver* msg_receiver; + std::atomic msg_receiver; }; class MockMessageReceiver : public MessageReceiver @@ -62,7 +63,7 @@ class MockMessageReceiver : public MessageReceiver CDRMessage_t* msg) override; void setCallback( std::function cb); - octet* data; + std::atomic data; std::function callback; }; From 93e77d72dd583a7a7c112568110bc5cc09e09631 Mon Sep 17 00:00:00 2001 From: Zakaria Talbi Lalmi Date: Mon, 15 Jun 2026 16:09:49 +0200 Subject: [PATCH 13/17] Fix data races in ChainingTransportTests by making flag variables atomic Signed-off-by: Zakaria Talbi Lalmi --- .../common/BlackboxTestsTransportCustom.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/test/blackbox/common/BlackboxTestsTransportCustom.cpp b/test/blackbox/common/BlackboxTestsTransportCustom.cpp index 6cc704e1e08..ca47181704d 100644 --- a/test/blackbox/common/BlackboxTestsTransportCustom.cpp +++ b/test/blackbox/common/BlackboxTestsTransportCustom.cpp @@ -14,6 +14,7 @@ #include "BlackboxTests.hpp" +#include #include #include @@ -371,12 +372,12 @@ const std::string BuiltinTransportsTest::env_var_name_ = "FASTDDS_BUILTIN_TRANSP TEST(ChainingTransportTests, basic_test) { - bool writer_init_function_called = false; - bool writer_receive_function_called = false; - bool writer_send_function_called = false; - bool reader_init_function_called = false; - bool reader_receive_function_called = false; - bool reader_send_function_called = false; + std::atomic writer_init_function_called {false}; + std::atomic writer_receive_function_called {false}; + std::atomic writer_send_function_called {false}; + std::atomic reader_init_function_called {false}; + std::atomic reader_receive_function_called {false}; + std::atomic reader_send_function_called {false}; eprosima::fastdds::rtps::PropertyPolicy test_property_policy; test_property_policy.properties().push_back({test_property_name, test_property_value}); std::shared_ptr udp_transport = std::make_shared(); From 57f3b036219accbfce317053a0d5444169b5e835 Mon Sep 17 00:00:00 2001 From: Zakaria Talbi Lalmi Date: Mon, 15 Jun 2026 16:12:50 +0200 Subject: [PATCH 14/17] Fix data race on done flag in flush log tests Signed-off-by: Zakaria Talbi Lalmi --- test/unittest/logging/LogTests.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/unittest/logging/LogTests.cpp b/test/unittest/logging/LogTests.cpp index 714464164b9..0087fd65411 100644 --- a/test/unittest/logging/LogTests.cpp +++ b/test/unittest/logging/LogTests.cpp @@ -218,7 +218,7 @@ TEST_F(LogTests, validate_single_flush_call) the back queue will grow so large while the front one is cleared, that the test will probably timeout. */ - bool done = false; + std::atomic done {false}; int commited_before_flush = 0; // std::atomic committed = 0; // only works on msvc and icc std::atomic committed; @@ -291,7 +291,7 @@ TEST_F(LogTests, validate_multithread_flush_calls) the back queue will grow so large while the front one is cleared, that the test will probably timeout. */ - bool done = false; + std::atomic done {false}; // std::atomic committed = 0; // only works on msvc and icc std::atomic committed; committed = 0; From 0206be32f5b485cf4bb8de28d736c06b151dd805 Mon Sep 17 00:00:00 2001 From: Zakaria Talbi Lalmi Date: Mon, 15 Jun 2026 16:23:50 +0200 Subject: [PATCH 15/17] Fix data race on MockMessageReceiver callback by adding mutex protection Signed-off-by: Zakaria Talbi Lalmi --- .../transport/mock/MockReceiverResource.cpp | 17 ++++++++++------- .../transport/mock/MockReceiverResource.h | 2 ++ 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/test/unittest/transport/mock/MockReceiverResource.cpp b/test/unittest/transport/mock/MockReceiverResource.cpp index 26aa58aff64..3d50c8b2669 100644 --- a/test/unittest/transport/mock/MockReceiverResource.cpp +++ b/test/unittest/transport/mock/MockReceiverResource.cpp @@ -49,17 +49,17 @@ MockReceiverResource::~MockReceiverResource() Cleanup(); } - delete msg_receiver; + delete msg_receiver.load(); } MessageReceiver* MockReceiverResource::CreateMessageReceiver() { - if (msg_receiver == nullptr) + if (msg_receiver.load() == nullptr) { - msg_receiver = new MockMessageReceiver(); - msg_receiver->init(1024); + msg_receiver.store(new MockMessageReceiver()); + msg_receiver.load()->init(1024); } - return msg_receiver; + return msg_receiver.load(); } void MockReceiverResource::OnDataReceived( @@ -68,19 +68,21 @@ void MockReceiverResource::OnDataReceived( const Locator_t&, const Locator_t& remote) { - if (msg_receiver != nullptr) + MockMessageReceiver* recv = msg_receiver.load(); + if (recv != nullptr) { CDRMessage_t msg(0); msg.wraps = true; msg.buffer = const_cast(buf); msg.length = size; - msg_receiver->processCDRMsg(remote, &msg); + recv->processCDRMsg(remote, &msg); } } void MockMessageReceiver::setCallback( std::function cb) { + std::lock_guard lock(callback_mutex); this->callback = cb; } @@ -89,6 +91,7 @@ void MockMessageReceiver::processCDRMsg( CDRMessage_t* msg) { data = msg->buffer; + std::lock_guard lock(callback_mutex); if (callback != nullptr) { callback(); diff --git a/test/unittest/transport/mock/MockReceiverResource.h b/test/unittest/transport/mock/MockReceiverResource.h index dcb17af27ff..20a102724a7 100644 --- a/test/unittest/transport/mock/MockReceiverResource.h +++ b/test/unittest/transport/mock/MockReceiverResource.h @@ -17,6 +17,7 @@ #include #include +#include #include #include @@ -64,6 +65,7 @@ class MockMessageReceiver : public MessageReceiver void setCallback( std::function cb); std::atomic data; + std::mutex callback_mutex; std::function callback; }; From 7a182a59f79a16b731be79c2aece2706be5867c9 Mon Sep 17 00:00:00 2001 From: Zakaria Talbi Lalmi Date: Mon, 15 Jun 2026 16:27:54 +0200 Subject: [PATCH 16/17] Fix Semaphore lifetime race in UDP tests by clearing callback after wait Signed-off-by: Zakaria Talbi Lalmi --- test/unittest/transport/UDPv4Tests.cpp | 5 +++++ test/unittest/transport/UDPv6Tests.cpp | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/test/unittest/transport/UDPv4Tests.cpp b/test/unittest/transport/UDPv4Tests.cpp index 155534b8bf9..f4c6aafe674 100644 --- a/test/unittest/transport/UDPv4Tests.cpp +++ b/test/unittest/transport/UDPv4Tests.cpp @@ -240,6 +240,7 @@ TEST_F(UDPv4Tests, send_and_receive_between_ports) std::this_thread::sleep_for(std::chrono::milliseconds(1)); senderThread->join(); sem.wait(); + msg_recv->setCallback(nullptr); } TEST_F(UDPv4Tests, send_to_loopback) @@ -305,6 +306,7 @@ TEST_F(UDPv4Tests, send_to_loopback) std::this_thread::sleep_for(std::chrono::milliseconds(1)); senderThread->join(); sem.wait(); + msg_recv->setCallback(nullptr); } #endif // ifndef __APPLE__ @@ -551,6 +553,7 @@ TEST_F(UDPv4Tests, send_and_receive_between_allowed_sockets_using_localhost) std::this_thread::sleep_for(std::chrono::milliseconds(1)); senderThread->join(); sem.wait(); + msg_recv->setCallback(nullptr); } TEST_F(UDPv4Tests, send_and_receive_between_allowed_sockets_using_unicast) @@ -617,6 +620,7 @@ TEST_F(UDPv4Tests, send_and_receive_between_allowed_sockets_using_unicast) std::this_thread::sleep_for(std::chrono::milliseconds(1)); senderThread->join(); sem.wait(); + msg_recv->setCallback(nullptr); } } @@ -685,6 +689,7 @@ TEST_F(UDPv4Tests, send_and_receive_between_allowed_sockets_using_unicast_to_mul std::this_thread::sleep_for(std::chrono::milliseconds(1)); senderThread->join(); sem.wait(); + msg_recv->setCallback(nullptr); } } diff --git a/test/unittest/transport/UDPv6Tests.cpp b/test/unittest/transport/UDPv6Tests.cpp index de7302ab0aa..019ae6727b1 100644 --- a/test/unittest/transport/UDPv6Tests.cpp +++ b/test/unittest/transport/UDPv6Tests.cpp @@ -277,6 +277,7 @@ TEST_F(UDPv6Tests, send_and_receive_between_ports) std::this_thread::sleep_for(std::chrono::milliseconds(1)); senderThread->join(); sem.wait(); + msg_recv->setCallback(nullptr); } TEST_F(UDPv6Tests, send_to_loopback) @@ -342,6 +343,7 @@ TEST_F(UDPv6Tests, send_to_loopback) std::this_thread::sleep_for(std::chrono::milliseconds(1)); senderThread->join(); sem.wait(); + msg_recv->setCallback(nullptr); } #endif // ifndef __APPLE__ @@ -591,6 +593,7 @@ TEST_F(UDPv6Tests, send_and_receive_between_allowed_sockets_using_localhost) std::this_thread::sleep_for(std::chrono::milliseconds(1)); senderThread->join(); sem.wait(); + msg_recv->setCallback(nullptr); } TEST_F(UDPv6Tests, send_and_receive_between_allowed_sockets_using_unicast) @@ -657,6 +660,7 @@ TEST_F(UDPv6Tests, send_and_receive_between_allowed_sockets_using_unicast) std::this_thread::sleep_for(std::chrono::milliseconds(1)); senderThread->join(); sem.wait(); + msg_recv->setCallback(nullptr); } } @@ -724,6 +728,7 @@ TEST_F(UDPv6Tests, send_and_receive_between_allowed_sockets_using_unicast_to_mul std::this_thread::sleep_for(std::chrono::milliseconds(1)); senderThread->join(); sem.wait(); + msg_recv->setCallback(nullptr); } } From 14962658fdbc70392f4d0627302c8f83f68da461 Mon Sep 17 00:00:00 2001 From: Zakaria Talbi Lalmi Date: Mon, 15 Jun 2026 16:31:35 +0200 Subject: [PATCH 17/17] Fix data race on TestCondition::trigger_value by replacing volatile with atomic Signed-off-by: Zakaria Talbi Lalmi --- test/unittest/dds/core/condition/WaitSetImplTests.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/unittest/dds/core/condition/WaitSetImplTests.cpp b/test/unittest/dds/core/condition/WaitSetImplTests.cpp index b245a6848e8..be7c0f8805e 100644 --- a/test/unittest/dds/core/condition/WaitSetImplTests.cpp +++ b/test/unittest/dds/core/condition/WaitSetImplTests.cpp @@ -13,6 +13,7 @@ // limitations under the License. #include +#include #include #include @@ -35,7 +36,7 @@ class TestCondition : public Condition { public: - volatile bool trigger_value = false; + std::atomic trigger_value {false}; bool get_trigger_value() const override {