Skip to content

Commit edde48f

Browse files
mergify[bot]cferreiragonz
mergify[bot]
and
cferreiragonz
authored
Fix Data Races on DDS-Pipe (backport #145) (#152)
* Fix Data Races on DDS-Pipe (#145) * Refs #21670: Protect on_data_available_lambda_ Signed-off-by: cferreiragonz <[email protected]> * Refs #21670: Set listener in RederCreation Signed-off-by: cferreiragonz <[email protected]> * Refs #21670: Separate DDS/RTPS Listeners from Base Classes Signed-off-by: cferreiragonz <[email protected]> * Refs #21670: Uncrustify Signed-off-by: cferreiragonz <[email protected]> * Refs #21670: Apply Review Signed-off-by: cferreiragonz <[email protected]> * Refs #21670: Apply Review 2 Signed-off-by: cferreiragonz <[email protected]> * Refs #21670: Apply review 3 Signed-off-by: cferreiragonz <[email protected]> * Refs #21670: Avoid using capital letters in DDS Listener Signed-off-by: cferreiragonz <[email protected]> * Refs #21670: Fix leak Signed-off-by: cferreiragonz <[email protected]> --------- Signed-off-by: cferreiragonz <[email protected]> (cherry picked from commit ee0e639) * Fix conflicts Signed-off-by: cferreiragonz <[email protected]> --------- Signed-off-by: cferreiragonz <[email protected]> Co-authored-by: Carlos Ferreira González <[email protected]> Co-authored-by: cferreiragonz <[email protected]>
1 parent 8abd633 commit edde48f

File tree

9 files changed

+288
-111
lines changed

9 files changed

+288
-111
lines changed

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

Lines changed: 64 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ namespace dds {
5454
* @warning This Participant class does not support RPC so far.
5555
* @todo TODO
5656
*/
57-
class CommonParticipant : public core::IParticipant, public fastdds::dds::DomainParticipantListener
57+
class CommonParticipant : public core::IParticipant
5858
{
5959
public:
6060

@@ -106,17 +106,56 @@ class CommonParticipant : public core::IParticipant, public fastdds::dds::Domain
106106
// LISTENER METHODS
107107
/////////////////////////
108108

109-
virtual void on_participant_discovery(
110-
fastdds::dds::DomainParticipant* participant,
111-
fastrtps::rtps::ParticipantDiscoveryInfo&& info) override;
112-
113-
virtual void on_subscriber_discovery(
114-
fastdds::dds::DomainParticipant* participant,
115-
fastrtps::rtps::ReaderDiscoveryInfo&& info) override;
116-
117-
virtual void on_publisher_discovery(
118-
fastdds::dds::DomainParticipant* participant,
119-
fastrtps::rtps::WriterDiscoveryInfo&& info) override;
109+
class DdsListener : public fastdds::dds::DomainParticipantListener
110+
{
111+
public:
112+
113+
DDSPIPE_PARTICIPANTS_DllAPI
114+
explicit DdsListener(
115+
std::shared_ptr<SimpleParticipantConfiguration> conf,
116+
std::shared_ptr<core::DiscoveryDatabase> ddb);
117+
118+
/**
119+
* @brief Override method from \c DomainParticipantListener
120+
*
121+
* This method is only used for debugging purposes.
122+
*/
123+
DDSPIPE_PARTICIPANTS_DllAPI
124+
void on_participant_discovery(
125+
fastdds::dds::DomainParticipant* participant,
126+
fastrtps::rtps::ParticipantDiscoveryInfo&& info) override;
127+
128+
/**
129+
* @brief Override method from \c DomainParticipantListener .
130+
*
131+
* This method adds to the database the discovered or modified endpoint.
132+
*/
133+
DDSPIPE_PARTICIPANTS_DllAPI
134+
void on_subscriber_discovery(
135+
fastdds::dds::DomainParticipant* participant,
136+
fastrtps::rtps::ReaderDiscoveryInfo&& info) override;
137+
138+
/**
139+
* @brief Override method from \c DomainParticipantListener .
140+
*
141+
* This method adds to the database the discovered or modified endpoint.
142+
*/
143+
DDSPIPE_PARTICIPANTS_DllAPI
144+
void on_publisher_discovery(
145+
fastdds::dds::DomainParticipant* participant,
146+
fastrtps::rtps::WriterDiscoveryInfo&& info) override;
147+
148+
protected:
149+
150+
//! Shared pointer to the configuration of the participant
151+
const std::shared_ptr<SimpleParticipantConfiguration> configuration_;
152+
//! Shared pointer to the discovery database
153+
const std::shared_ptr<core::DiscoveryDatabase> discovery_database_;
154+
155+
};
156+
157+
//! Unique pointer to the internal DDS Participant Listener
158+
std::unique_ptr<fastdds::dds::DomainParticipantListener> dds_participant_listener_;
120159

121160
protected:
122161

@@ -130,14 +169,27 @@ class CommonParticipant : public core::IParticipant, public fastdds::dds::Domain
130169
const std::shared_ptr<core::PayloadPool>& payload_pool,
131170
const std::shared_ptr<core::DiscoveryDatabase>& discovery_database);
132171

172+
/////////////////////////
173+
// VIRTUAL METHODS
174+
/////////////////////////
175+
176+
/**
177+
* @brief Virtual method that creates a listener for the internal DDS Participant.
178+
* It should be overridden if a different listener is needed.
179+
*/
180+
DDSPIPE_PARTICIPANTS_DllAPI
181+
virtual std::unique_ptr<fastdds::dds::DomainParticipantListener> create_listener_();
182+
133183
/////////////////////////
134184
// INTERNAL VIRTUAL METHODS
135185
/////////////////////////
136186

187+
DDSPIPE_PARTICIPANTS_DllAPI
137188
virtual
138189
fastdds::dds::DomainParticipantQos
139190
reckon_participant_qos_() const;
140191

192+
DDSPIPE_PARTICIPANTS_DllAPI
141193
virtual
142194
fastdds::dds::DomainParticipant*
143195
create_dds_participant_();

‎ddspipe_participants/include/ddspipe_participants/participant/dynamic_types/DynTypesParticipant.hpp‎

Lines changed: 47 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ namespace participants {
3636
*
3737
* TODO: separate these 2 participants
3838
*/
39-
class DynTypesParticipant : public rtps::SimpleParticipant, public eprosima::fastdds::dds::DomainParticipantListener
39+
class DynTypesParticipant : public rtps::SimpleParticipant
4040
{
4141
public:
4242

@@ -71,33 +71,61 @@ class DynTypesParticipant : public rtps::SimpleParticipant, public eprosima::fas
7171
std::shared_ptr<core::IReader> create_reader(
7272
const core::ITopic& topic) override;
7373

74-
DDSPIPE_PARTICIPANTS_DllAPI
75-
void on_type_discovery(
76-
eprosima::fastdds::dds::DomainParticipant* participant,
77-
const eprosima::fastrtps::rtps::SampleIdentity& request_sample_id,
78-
const eprosima::fastrtps::string_255& topic,
79-
const eprosima::fastrtps::types::TypeIdentifier* identifier,
80-
const eprosima::fastrtps::types::TypeObject* object,
81-
eprosima::fastrtps::types::DynamicType_ptr dyn_type) override;
82-
83-
DDSPIPE_PARTICIPANTS_DllAPI
84-
virtual void on_type_information_received(
85-
eprosima::fastdds::dds::DomainParticipant* participant,
86-
const eprosima::fastrtps::string_255 topic_name,
87-
const eprosima::fastrtps::string_255 type_name,
88-
const eprosima::fastrtps::types::TypeInformation& type_information) override;
74+
class DynTypesRtpsListener : public rtps::CommonParticipant::RtpsListener,
75+
public eprosima::fastdds::dds::DomainParticipantListener
76+
{
77+
public:
78+
79+
DDSPIPE_PARTICIPANTS_DllAPI
80+
explicit DynTypesRtpsListener(
81+
std::shared_ptr<ParticipantConfiguration> conf,
82+
std::shared_ptr<core::DiscoveryDatabase> ddb,
83+
std::shared_ptr<InternalReader> internal_reader);
84+
85+
DDSPIPE_PARTICIPANTS_DllAPI
86+
void on_type_discovery(
87+
eprosima::fastdds::dds::DomainParticipant* participant,
88+
const eprosima::fastrtps::rtps::SampleIdentity& request_sample_id,
89+
const eprosima::fastrtps::string_255& topic,
90+
const eprosima::fastrtps::types::TypeIdentifier* identifier,
91+
const eprosima::fastrtps::types::TypeObject* object,
92+
eprosima::fastrtps::types::DynamicType_ptr dyn_type) override;
93+
94+
DDSPIPE_PARTICIPANTS_DllAPI
95+
virtual void on_type_information_received(
96+
eprosima::fastdds::dds::DomainParticipant* participant,
97+
const eprosima::fastrtps::string_255 topic_name,
98+
const eprosima::fastrtps::string_255 type_name,
99+
const eprosima::fastrtps::types::TypeInformation& type_information) override;
100+
101+
//! Type Object Reader getter
102+
inline std::shared_ptr<InternalReader> type_object_reader() const
103+
{
104+
return type_object_reader_;
105+
}
106+
107+
protected:
108+
109+
void internal_notify_type_object_(
110+
eprosima::fastrtps::types::DynamicType_ptr dynamic_type);
111+
112+
//! Copy of Type Object Internal Reader
113+
std::shared_ptr<InternalReader> type_object_reader_;
114+
115+
};
89116

90117
protected:
91118

92-
void internal_notify_type_object_(
93-
eprosima::fastrtps::types::DynamicType_ptr dynamic_type);
94-
95119
void initialize_internal_dds_participant_();
96120

97121
eprosima::fastdds::dds::DomainParticipant* dds_participant_;
98122

99123
//! Type Object Internal Reader
100124
std::shared_ptr<InternalReader> type_object_reader_;
125+
126+
//! Override method from \c CommonParticipant to create the internal RTPS participant listener
127+
std::unique_ptr<fastrtps::rtps::RTPSParticipantListener> create_listener_() override;
128+
101129
};
102130

103131
} /* namespace participants */

‎ddspipe_participants/include/ddspipe_participants/participant/rtps/CommonParticipant.hpp‎

Lines changed: 59 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@ namespace rtps {
5050
*/
5151
class CommonParticipant
5252
: public core::IParticipant
53-
, public fastrtps::rtps::RTPSParticipantListener
5453
{
5554
public:
5655

@@ -121,35 +120,56 @@ class CommonParticipant
121120
// RTPS LISTENER METHODS
122121
/////////////////////////
123122

124-
/**
125-
* @brief Override method from \c RTPSParticipantListener .
126-
*
127-
* This method only is for debugging purposes.
128-
*/
129-
DDSPIPE_PARTICIPANTS_DllAPI
130-
virtual void onParticipantDiscovery(
131-
fastrtps::rtps::RTPSParticipant* participant,
132-
fastrtps::rtps::ParticipantDiscoveryInfo&& info) override;
133-
134-
/**
135-
* @brief Override method from \c RTPSParticipantListener .
136-
*
137-
* This method adds to database the endpoint discovered or modified.
138-
*/
139-
DDSPIPE_PARTICIPANTS_DllAPI
140-
virtual void onReaderDiscovery(
141-
fastrtps::rtps::RTPSParticipant* participant,
142-
fastrtps::rtps::ReaderDiscoveryInfo&& info) override;
143-
144-
/**
145-
* @brief Override method from \c RTPSParticipantListener .
146-
*
147-
* This method adds to database the endpoint discovered or modified.
148-
*/
149-
DDSPIPE_PARTICIPANTS_DllAPI
150-
virtual void onWriterDiscovery(
151-
fastrtps::rtps::RTPSParticipant* participant,
152-
fastrtps::rtps::WriterDiscoveryInfo&& info) override;
123+
class RtpsListener : public fastrtps::rtps::RTPSParticipantListener
124+
{
125+
public:
126+
127+
DDSPIPE_PARTICIPANTS_DllAPI
128+
explicit RtpsListener(
129+
std::shared_ptr<ParticipantConfiguration> conf,
130+
std::shared_ptr<core::DiscoveryDatabase> ddb);
131+
132+
/**
133+
* @brief Override method from \c RTPSParticipantListener .
134+
*
135+
* This method is only used for debugging purposes.
136+
*/
137+
DDSPIPE_PARTICIPANTS_DllAPI
138+
virtual void onParticipantDiscovery(
139+
fastrtps::rtps::RTPSParticipant* participant,
140+
fastrtps::rtps::ParticipantDiscoveryInfo&& info) override;
141+
142+
/**
143+
* @brief Override method from \c RTPSParticipantListener .
144+
*
145+
* This method adds to database the endpoint discovered or modified.
146+
*/
147+
DDSPIPE_PARTICIPANTS_DllAPI
148+
virtual void onReaderDiscovery(
149+
fastrtps::rtps::RTPSParticipant* participant,
150+
fastrtps::rtps::ReaderDiscoveryInfo&& info) override;
151+
152+
/**
153+
* @brief Override method from \c RTPSParticipantListener .
154+
*
155+
* This method adds to database the endpoint discovered or modified.
156+
*/
157+
DDSPIPE_PARTICIPANTS_DllAPI
158+
virtual void onWriterDiscovery(
159+
fastrtps::rtps::RTPSParticipant* participant,
160+
fastrtps::rtps::WriterDiscoveryInfo&& info) override;
161+
162+
protected:
163+
164+
//! Shared pointer to the configuration of the participant
165+
const std::shared_ptr<ParticipantConfiguration> configuration_;
166+
//! Shared pointer to the discovery database
167+
const std::shared_ptr<core::DiscoveryDatabase> discovery_database_;
168+
169+
};
170+
171+
//! Unique pointer to the internal RTPS Participant Listener
172+
std::unique_ptr<fastrtps::rtps::RTPSParticipantListener> rtps_participant_listener_;
153173

154174
//////////////////
155175
// STATIC METHODS
@@ -207,6 +227,15 @@ class CommonParticipant
207227
static fastrtps::rtps::RTPSParticipantAttributes reckon_participant_attributes_(
208228
const ParticipantConfiguration* participant_configuration);
209229

230+
/**
231+
* @brief Virtual method that creates a listener for the internal RTPS Participant.
232+
* It should be overridden if a different listener is needed.
233+
* This method must be called after the RTPS Participant is created, otherwise no listener will be set.
234+
* @return A unique pointer to an RTPS Participant Listener.
235+
*/
236+
DDSPIPE_PARTICIPANTS_DllAPI
237+
virtual std::unique_ptr<fastrtps::rtps::RTPSParticipantListener> create_listener_();
238+
210239
/////
211240
// VARIABLES
212241

0 commit comments

Comments
 (0)