[dbwls99706]

Description

Deleting a DataReader while discovery removes a matched remote DataWriter can cause a null listener dereference on 2.6.x. The discovery thread checks the listener and then reads it again to invoke a callback. Between those reads, DataReaderImpl::disable() can clear it through setListener(nullptr).

I first encountered this crash while validating generated ROS 2 C++ packages for my ros2-engineering-skills project. I reproduced this during ROS 2 Humble shutdown using the stock demo_nodes_cpp talker and listener, without custom application nodes. Core dumps and AddressSanitizer reports identified the check-then-re-read paths in StatefulReader::matched_writer_remove() and the EDP::unpairWriterProxy() lambda. The new blackbox regression test also reproduces the corresponding StatelessReader path.

This PR backports three upstream changes to 2.6.x and adds a regression test for concurrent reader deletion and writer removal. It addresses the null dereference observed in #3303; it does not claim to resolve every listener lifetime race.

Changes

• Store EDP listener pointers before being used [15216] #2857, cherry-picked from 0014b017b: store EDP listener pointers locally before use.
• [15341] Protect access to reader listeners #2898, cherry-picked from 2078f7f7e: protect DataReader listener access and copy StatefulReader/StatelessReader listener pointers under mp_mutex before invoking discovery callbacks outside the lock.
• [21355] Make on_xxx_matched thread-safe #6371, cherry-picked from de70d42f6: store listener pointers in additional EDP pairing paths and serialize matched-status updates and notifications in the inner DataReader/DataWriter listeners. Conflicts were resolved for 2.6.x following the structure of the 2.14.x backport, [21355] Make on_xxx_matched thread-safe (backport #6371) #6382. Its DDSDiscovery.MatchedCallbackListenerMultithread regression test is included.
• Add DDSDiscovery.DataReaderDeletionWhileRemoteWriterIsRemoved: each of 200 iterations creates eight readers, mixing reliable and best-effort QoS, and a writer on another participant. After waiting for matching, two threads concurrently delete the writer and the readers. This exercises both StatefulReader and StatelessReader removal paths.

The cherry-picked commits preserve their original authors and sign-offs, with my sign-off added. The new regression test is a separate commit. The experimental callback gate used during diagnosis is not included.

Target branch: 2.6.x. This is a backport of existing upstream changes; no additional backport is requested here.

Related to #3303.

Validation

Local 2.6.x tests

Environment: Ubuntu 24.04, GCC 13.3, RelWithDebInfo, SECURITY=ON, and googletest release-1.11.0. The baseline is 2.6.x with the same two tests added but without the three fixes.

Test	Baseline	Backport
New deletion/unpair test, 50 fixed runs	50/50 crashed	50/50 passed (10,000 internal iterations)
New deletion/unpair test under ASan	13/20 runs reported null SEGV	50/50 passed; no ASan or LeakSanitizer reports
MatchedCallbackListenerMultithread	50/50 passed	50/50 passed
Related DDS PIM blackbox tests	Not run	222/222 passed

The 50-run baseline count comes from fixed-count execution. A separate ctest --repeat until-fail:50 run stopped on its first execution.

In 30 baseline runs under GDB, all crashed: 19 at StatelessReader.cpp:219, 9 at EDP.cpp:583, and 2 at StatefulReader.cpp:382. All followed EDP::unpairWriterProxy() called by PDP::removeWriterProxyData(). The baseline ASan reports identified null dereferences at these same locations, with no other ASan report in those runs.

The upstream matched-callback test checks matched-status consistency. It did not distinguish the baseline from the backport in this environment; the new deletion/unpair test provides the direct regression coverage for the observed null dereference.

The 222 related tests cover DDSDiscovery, DDSStatus, DDSBasic, DDSDataReader, DDSDataWriter, PubSubBasic, LivelinessQos, and DDSContentFilter. The full test suite was not run locally.

ROS 2 Humble / Fast DDS 2.6.12 reproduction

Both versions of the ros-humble-fastrtps 2.6.12 source package were rebuilt in the same CI run: unchanged and with the three backported fixes.

The reproduction runs in an Ubuntu 22.04 Docker container without external network access, with ROS_LOCALHOST_ONLY=1 (UDPv4 over loopback plus SHM). A Python launch file starts the stock talker and listener using launch.LaunchService. A separate rclpy process waits for discovery before SIGINT is sent to the launch process and forwarded to the nodes. Each non-ASan build was exercised for 30 minutes per job across four jobs.

Build	Unchanged	Backport
Non-ASan	21 / 10,838 iterations ended with a SIGSEGV exit	0 / 12,180 iterations
Non-ASan, same CPU model only (EPYC 7763)	21 / 10,838	0 / 5,734
ASan, two jobs per build	22 null SEGV reports / 5,190 iterations	0 reports / 5,313 iterations

All 12 analyzed non-ASan cores were in StatefulReader::matched_writer_remove() or the EDP::unpairWriterProxy() lambda. The baseline ASan reports identified StatefulReader.cpp:382 and EDP.cpp:583. The patched ASan runs produced no reports, including no heap-use-after-free reports. The patched non-ASan runs had no other abnormal or slow exits.

The iteration-level Fisher exact p-values were 1.3e-7 overall and 2.5e-4 for the same-CPU comparison. A job-level permutation analysis gave p = 0.0286, the smallest value possible with four jobs per build. Iterations within a job are not necessarily independent, and runner hardware varied. These results support the fix for the observed crash; zero observed failures is not proof of general lifetime safety.

Sanitizer conditions and CI status

The local blackbox ASan runs used detect_odr_violation=0 for both versions. Otherwise the binary stops at startup on an ODR report for environment_file_: SystemInfo.cpp is compiled into both the library and the blackbox executable. This disables the ODR check, not all global-variable instrumentation. Leak detection remained enabled.

The results above are from local runs and from the Humble reproduction. I have not yet run the upstream CI workflows on my fork or completed TSan validation. One note for the ASan CI job on this branch: asan.meta passes -DSANITIZER=Address, but the 2.6.x CMake files do not read that option, so the job may build without ASan instrumentation.

Compatibility, coverage, and style

• No installed headers or public API were changed.
• The defined exported symbols of libfastrtps, compared with nm -D --defined-only, are identical: 10,870 entries.
• abi-compliance-checker over the public headers reported zero binary and zero source compatibility problems. The ABI dump used optimized (-O2) builds, so this is supporting interface evidence rather than a complete proof of ABI compatibility.
• Added members are in DataReaderImpl and the inner DataReader/DataWriter listeners; the implementation classes are only forward-declared in installed headers.
• Local line coverage increased when comparing 2.6.x with the existing DDSDiscovery tests against this branch with the two new tests included: EDP.cpp 60.7% to 63.0%, StatefulReader.cpp 51.8% to 57.2%, StatelessReader.cpp 40.5% to 49.0%, DataReaderImpl.cpp 33.0% to 34.9%, and DataWriterImpl.cpp 31.4% to 32.8%. The same existing tests also showed no decrease in coverage after the fixes. Of the 76 added code lines, 59 were executed (77.6%). These measurements cover the changed source files, not the full project. I could not compare against the nightly coverage job.
• Uncrustify 0.78.1 with the eProsima C++ style reported no new differences. Existing differences in EDP.cpp, StatefulReader.cpp, and StatelessReader.cpp were left unchanged to keep the backport focused.

The counts, statistical analysis, ABI checks, and coverage figures above come from the collected execution logs and analysis scripts. The reproduction harness, backtraces, sanitizer logs, patches, and analysis are available in the evidence bundle on request.

Known limitations and changes not included

• This backport removes the repeated listener reads in the selected upstream fixes. It does not add a callback gate or make setListener() wait for callbacks already in progress. Listener lifetime synchronization beyond these changes remains outside this PR's scope.
• Other listener-access patterns outside the backported changes are not modified.
• Under LeakSanitizer, MatchedCallbackListenerMultithread reported TypeObjectFactory leaks in 3/70 baseline runs and 2/70 patched runs. Its assertions passed, but those sanitizer runs are not counted as clean passes. The observed leak was attributed to the unsynchronized TypeObjectFactory::get_instance() initialization addressed upstream by [21672] Fix data race in TypeObjectFactory::get_instance #5238. That separate fix is not included here.
• I have not run the full local test suite or built the documentation. Fork ASan and TSan validation is still pending.

Contributor Checklist

• Commit messages follow the project guidelines. Original authors and sign-offs are preserved, and my DCO sign-off is included.
• The code follows the style guidelines of this project. No new uncrustify differences; see above.
• Tests that thoroughly check the new feature have been added/Regression tests checking the bug and its fix have been added; the added tests pass locally. See the separate LeakSanitizer limitation above.
• Any new/modified methods have been properly documented using Doxygen. The matched-status mutex comments are preserved from [21355] Make on_xxx_matched thread-safe #6371; the uncommented listener_mutex_ member is preserved from [15341] Protect access to reader listeners #2898. Full Doxygen compliance has not been confirmed.
• N/A Any new configuration API has an equivalent XML API (with the corresponding XSD extension). No configuration API changes.
• Changes are backport compatible: they do NOT break ABI nor change library core behavior. No compatibility problems were detected by the checks described above; the behavior change is limited to the backported fixes.
• Changes are API compatible.
• N/A New feature has been added to the versions.md file (if applicable). Bug-fix backport only.
• N/A New feature has been documented/Current behavior is correctly described in the documentation. No new user-facing feature or configuration.
• Applicable backports have been included in the description. This PR targets 2.6.x; no additional backport is requested.

Reviewer Checklist

• The PR has a milestone assigned.
• The title and description correctly express the PR's purpose.
• Check contributor checklist is correct.
• If this is a critical bug fix, backports to the critical-only supported branches have been requested.
• Check CI results: changes do not issue any warning.
• Check CI results: failing tests are unrelated with the changes.

[cferreiragonz]

Hello @dbwls99706,

First of all, thanks for your contribution. According to our RELEASE_SUPPORT.md and our documentation, branch 2.6.x is a maintenance branch that ended standard support in July 2024. That means that this branch only receives security fixes. We encourage you to move to more recent branch in order to benefit from the latest updates of Fast DDS.

Apologies for the inconvenience.

[dbwls99706]

@cferreiragonz Thanks for the clarification.

RELEASE_SUPPORT.md and the linked documentation list “critical issues and security fixes” for the extended support period. Could this crash be considered a critical issue under that policy?

It reproduces with the stock ROS 2 Humble talker and listener during shutdown. The regression test also reproduces it during concurrent DataReader deletion and remote DataWriter removal while the participants remain active. Since ordinary entity deletion can crash the entire process, the potential impact extends beyond shutdown.

ROS 2 Humble ships Fast DDS 2.6.x, so moving to a newer branch is not a routine package update for users staying on Humble. This PR backports existing upstream fixes.

I understand if it does not meet the criteria!