Skip to content

Commit 8eaf047

Browse files
authored
Fixed segfault in 'RepeaterDataFilter.cpp' when forwarding dispose/unregister samples with a null check (#186)
* [#24620] Fixed segfault in 'RepeaterDataFilter.cpp' when forwarding dispose/unregister samples Signed-off-by: danipiza <dpizarrogallego@gmail.com> * [#24620] Applied uncrustify Signed-off-by: danipiza <dpizarrogallego@gmail.com> * [#24607] Documented future API approach Signed-off-by: danipiza <dpizarrogallego@gmail.com> --------- Signed-off-by: danipiza <dpizarrogallego@gmail.com>
1 parent 0fe11b8 commit 8eaf047

1 file changed

Lines changed: 36 additions & 2 deletions

File tree

ddspipe_participants/src/cpp/writer/dds/filter/RepeaterDataFilter.cpp

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,46 @@ bool RepeaterDataFilter::evaluate(
3838

3939
auto write_data = std::static_pointer_cast<RepeaterWriteData>(sample_info.user_write_data);
4040

41+
if (!write_data)
42+
{
43+
// The origin information is carried through WriteParams::user_write_data, which is only attached to
44+
// ALIVE samples (see dds::CommonWriter::fill_to_send_data_)
45+
// Dispose and unregister samples are sent through DataWriter::dispose / DataWriter::unregister_instance,
46+
// whose public API does not allow attaching WriteParams, so user_write_data is null for them.
47+
// In that case the origin participant is unknown and the sample cannot be filtered out by origin:
48+
// let it through (it already passed the SelfDataFilter check above,
49+
// so it is not sent back to the writer's own participant)
50+
//
51+
// TODO: This null check is a workaround.
52+
// The intended future fix is to stop relying on RepeaterDataFilter and avoid echoing
53+
// samples back to their source, and instead expand the DDS-Pipe writer API so that, in repeater mode,
54+
// the origin endpoint GUID is passed explicitly into the dispose/unregister (and write) calls:
55+
// - dds::CommonWriter::write_nts_ already knows the origin (the RtpsPayloadData::source_guid it
56+
// currently copies into RepeaterWriteData). Its dispatch helpers
57+
// (core::PayloadPoolMediator::dispose() / unregister_instance() / write()) would take that GUID as
58+
// an explicit argument instead of smuggling it through WriteParams::user_write_data.
59+
// - A new Fast DDS DataWriter dispose/unregister/write API would receive that origin GUID and exclude
60+
// the originating participant from the dispatch directly, rather than having the RTPS layer run a
61+
// per-reader content filter to drop the sample for that reader.
62+
// With the source excluded at dispatch time, the repeater no longer needs RepeaterDataFilter at all to
63+
// prevent resending the signal back to its source, so this filter (and this null branch) could be
64+
// removed. This also covers dispose/unregister, whose origin is currently lost because their API
65+
// carries no WriteParams.
66+
logDebug(
67+
REPEATER_DATA_FILTER,
68+
"Sample without origin information (e.g. dispose/unregister); "
69+
"considering it relevant for reader GUID " << reader_guid << ".");
70+
return true;
71+
}
72+
4173
bool is_relevant = write_data->last_writer_guid_prefix != reader_guid.guidPrefix;
4274

4375
logDebug(
4476
REPEATER_DATA_FILTER,
45-
"Evaluating whether sample with origin writer GUID prefix " << write_data->last_writer_guid_prefix <<
46-
" is relevant for reader GUID " << reader_guid << "? " << (is_relevant ? "TRUE" : "FALSE"));
77+
"Evaluating whether sample with origin writer GUID prefix "
78+
<< write_data->last_writer_guid_prefix
79+
<< " is relevant for reader GUID " << reader_guid
80+
<< "? " << (is_relevant ? "TRUE" : "FALSE"));
4781

4882
return is_relevant;
4983
}

0 commit comments

Comments
 (0)