Refs #21670: Apply Review 2

Signed-off-by: cferreiragonz <carlosferreira@eprosima.com>

Author: cferreiragonz

ddspipe_participants/include/ddspipe_participants/participant/dds/CommonParticipant.hpp

@@ -153,17 +153,8 @@ class CommonParticipant : public core::IParticipant
                 const fastdds::dds::PublicationBuiltinTopicData& info,
                 bool& /*should_be_ignored*/) override;
 
-        //! Getter for GUID of the participant
-        const fastdds::rtps::GUID_t& guid() const;
-
-        //! Setter for GUID of the participant
-        void guid(
-                const fastdds::rtps::GUID_t& guid);
-
     protected:
 
-        //! GUID of the participant
-        fastdds::rtps::GUID_t guid_;
         //! Shared pointer to the configuration of the participant
         const std::shared_ptr<SimpleParticipantConfiguration> configuration_;
         //! Shared pointer to the discovery database

ddspipe_participants/src/cpp/participant/dds/CommonParticipant.cpp

@@ -96,9 +96,6 @@ void CommonParticipant::init()
     {
         throw utils::InitializationException(STR_ENTRY << "Error enabling DDS Participant " << id() << ".");
     }
-
-    // Set the GUID of the participant in the DDS Listener
-    static_cast<DDSListener*>(dds_participant_listener_.get())->guid(dds_participant_->guid());
 }
 
 core::types::ParticipantId CommonParticipant::id() const noexcept
@@ -265,13 +262,13 @@ void CommonParticipant::DDSListener::on_participant_discovery(
 }
 
 void CommonParticipant::DDSListener::on_data_reader_discovery(
-        fastdds::dds::DomainParticipant*,
+        fastdds::dds::DomainParticipant* participant,
         fastdds::rtps::ReaderDiscoveryStatus reason,
         const fastdds::dds::SubscriptionBuiltinTopicData& info,
         bool& /*should_be_ignored*/)
 {
     // If reader is from other participant, store it in discovery database
-    if (detail::come_from_same_participant_(info.guid, guid()))
+    if (detail::come_from_same_participant_(info.guid, participant->guid()))
     {
         // Come from this participant, do nothing
         return;
@@ -315,13 +312,13 @@ void CommonParticipant::DDSListener::on_data_reader_discovery(
 }
 
 void CommonParticipant::DDSListener::on_data_writer_discovery(
-        fastdds::dds::DomainParticipant*,
+        fastdds::dds::DomainParticipant* participant,
         fastdds::rtps::WriterDiscoveryStatus reason,
         const fastdds::dds::PublicationBuiltinTopicData& info,
         bool& /*should_be_ignored*/)
 {
     // If writer is from other participant, store it in discovery database
-    if (detail::come_from_same_participant_(info.guid, guid()))
+    if (detail::come_from_same_participant_(info.guid, participant->guid()))
     {
         // Come from this participant, do nothing
         return;
@@ -364,17 +361,6 @@ void CommonParticipant::DDSListener::on_data_writer_discovery(
     }
 }
 
-const fastdds::rtps::GUID_t& CommonParticipant::DDSListener::guid() const
-{
-    return guid_;
-}
-
-void CommonParticipant::DDSListener::guid(
-        const fastdds::rtps::GUID_t& guid)
-{
-    guid_ = guid;
-}
-
 CommonParticipant::CommonParticipant(
         const std::shared_ptr<SimpleParticipantConfiguration>& participant_configuration,
         const std::shared_ptr<core::PayloadPool>& payload_pool,

ddspipe_participants/src/cpp/reader/dds/CommonReader.cpp

@@ -86,6 +86,18 @@ void CommonReader::init()
                       participant_id_ << " in topic " << topic_ << ".");
     }
 
+    // Subscriber is created with autoenable set to false, so we need to enable the reader manually.
+    // This is done just to ensure that the reader is not registered before any other method modifying the reader pointer
+    // is called, opening a window for potential data races. Although Fast DDS ensures that this cannot happen, this
+    // procedure protects against future bad practices introducing the aforementioned data races.
+    if (fastdds::dds::RETCODE_OK != reader_->enable())
+    {
+        dds_subscriber_->delete_datareader(reader_);
+        throw utils::InitializationException(
+                  utils::Formatter() << "Error enabling DataReader for Participant " <<
+                      participant_id_ << " in topic " << topic_ << ".");
+    }
+
 }
 
 void CommonReader::on_data_available(
@@ -221,6 +233,7 @@ fastdds::dds::SubscriberQos CommonReader::reckon_subscriber_qos_() const
     {
         qos.partition().push_back("*");
     }
+    qos.entity_factory().autoenable_created_entities = false;
     return qos;
 }
 

ddspipe_participants/src/cpp/reader/rtps/CommonReader.cpp

@@ -107,6 +107,7 @@ void CommonReader::internal_entities_creation_(
     // Create CommonReader
     // Listener must be set in creation as no callbacks should be missed
     // It is safe to do so here as object is already created and callbacks do not require anything set in this method
+    // Also, no data races can ocurr as no callback will be called until this reader is registered
     rtps_reader_ = fastdds::rtps::RTPSDomain::createRTPSReader(
         rtps_participant_,
         non_const_reader_attributes,