Skip to content

[23923] Fix RTPSParticipantAttributes internal data races#6370

Merged
MiguelCompany merged 26 commits intomasterfrom
fix_rtps_attr_data_race
May 4, 2026
Merged

[23923] Fix RTPSParticipantAttributes internal data races#6370
MiguelCompany merged 26 commits intomasterfrom
fix_rtps_attr_data_race

Conversation

@cferreiragonz
Copy link
Copy Markdown
Contributor

@cferreiragonz cferreiragonz commented Apr 16, 2026

Description

This PR solves a bunch of data races caused by the not-thread-safe access to RTPSParticipantAttributes via its main getter get_attributes(). This method returns a const reference to the object, which can be concurrently modified from other functions like update_attributes() causing a read-write access data race.

The proposed fix should modify the original method to return a copy of the class attribute with the mutex locked, making the access safe. However, this would imply an API break. Thus, a new method has been created to be used internally, leaving the get_attributes() method untouched and marked to be fixed in the next major release.

Related PR:

@Mergifyio backport 3.4.x 3.2.x 2.14.x

Contributor Checklist

  • Commit messages follow the project guidelines.
  • The code follows the style guidelines of this project.
  • N/A: 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
  • N/A: Any new/modified methods have been properly documented using Doxygen.
  • N/A: Any new configuration API has an equivalent XML API (with the corresponding XSD extension)
  • Changes are backport compatible: they do NOT break ABI nor change library core behavior.
  • Changes are API compatible.
  • N/A New feature has been added to the versions.md file (if applicable).
  • N/A New feature has been documented/Current behavior is correctly described in the documentation.
  • N/A Applicable backports have been included in the description.

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.

juanlofer-eprosima and others added 9 commits April 16, 2026 12:40
Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com>
Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com>
Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com>
Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com>
Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com>
Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com>
Signed-off-by: Carlos Ferreira González <carlosferreira@eprosima.com>
Signed-off-by: Carlos Ferreira González <carlosferreira@eprosima.com>
Signed-off-by: Carlos Ferreira González <carlosferreira@eprosima.com>
@cferreiragonz cferreiragonz added this to the v3.7.0 milestone Apr 16, 2026
@github-actions github-actions Bot added the ci-pending PR which CI is running label Apr 16, 2026
Signed-off-by: Carlos Ferreira González <carlosferreira@eprosima.com>
Signed-off-by: Carlos Ferreira González <carlosferreira@eprosima.com>
@cferreiragonz cferreiragonz requested review from richiprosima and removed request for richiprosima April 17, 2026 08:14
@cferreiragonz cferreiragonz mentioned this pull request Apr 17, 2026
16 tasks
Copy link
Copy Markdown
Contributor

@raulojeda22 raulojeda22 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit

Comment thread include/fastdds/rtps/participant/RTPSParticipant.hpp Outdated
Signed-off-by: Carlos Ferreira González <carlosferreira@eprosima.com>
raulojeda22
raulojeda22 previously approved these changes Apr 20, 2026
Copy link
Copy Markdown
Contributor

@raulojeda22 raulojeda22 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@cferreiragonz cferreiragonz requested review from richiprosima and removed request for richiprosima April 20, 2026 12:20
Signed-off-by: Carlos Ferreira González <carlosferreira@eprosima.com>
Signed-off-by: Carlos Ferreira González <carlosferreira@eprosima.com>
Signed-off-by: Carlos Ferreira González <carlosferreira@eprosima.com>
Signed-off-by: Carlos Ferreira González <carlosferreira@eprosima.com>
@cferreiragonz cferreiragonz removed the request for review from raulojeda22 April 27, 2026 07:45
Signed-off-by: Carlos Ferreira González <carlosferreira@eprosima.com>
raulojeda22
raulojeda22 previously approved these changes Apr 28, 2026
Copy link
Copy Markdown
Contributor

@raulojeda22 raulojeda22 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with green CI

Signed-off-by: Carlos Ferreira González <carlosferreira@eprosima.com>
raulojeda22
raulojeda22 previously approved these changes Apr 29, 2026
Copy link
Copy Markdown
Contributor

@raulojeda22 raulojeda22 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with green CI

Signed-off-by: Carlos Ferreira González <carlosferreira@eprosima.com>
raulojeda22
raulojeda22 previously approved these changes Apr 30, 2026
Copy link
Copy Markdown
Contributor

@raulojeda22 raulojeda22 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with green CI

@cferreiragonz
Copy link
Copy Markdown
Contributor Author

259 Data Races reported by ASAN, none of them related with update_attributes. RTM IMHO

@cferreiragonz cferreiragonz added ready-to-merge Ready to be merged. CI and changes have been reviewed and approved. and removed ci-pending PR which CI is running labels May 4, 2026
Copy link
Copy Markdown
Member

@MiguelCompany MiguelCompany left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one nit. It does not require to rerun CI.

Comment thread include/fastdds/rtps/participant/RTPSParticipant.hpp Outdated
Signed-off-by: Carlos Ferreira González <carlosferreira@eprosima.com>
@MiguelCompany MiguelCompany merged commit 7dd4b4d into master May 4, 2026
5 checks passed
@MiguelCompany MiguelCompany deleted the fix_rtps_attr_data_race branch May 4, 2026 08:39
@cferreiragonz cferreiragonz modified the milestones: v3.7.0, v3.6.2 May 4, 2026
@cferreiragonz cferreiragonz mentioned this pull request May 4, 2026
1 task
@cferreiragonz cferreiragonz linked an issue May 4, 2026 that may be closed by this pull request
1 task
@cferreiragonz
Copy link
Copy Markdown
Contributor Author

@Mergifyio backport 3.4.x 3.2.x 2.14.x

@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented May 6, 2026

backport 3.4.x 3.2.x 2.14.x

✅ Backports have been created

Details

Cherry-pick of 7dd4b4d has failed:

On branch mergify/bp/3.4.x/pr-6370
Your branch is up to date with 'origin/3.4.x'.

You are currently cherry-picking commit 7dd4b4d1.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   include/fastdds/rtps/attributes/RTPSParticipantAttributes.hpp
	modified:   include/fastdds/rtps/participant/RTPSParticipant.hpp
	modified:   src/cpp/fastdds/builtin/type_lookup_service/TypeLookupManager.cpp
	modified:   src/cpp/fastdds/builtin/type_lookup_service/TypeLookupReplyListener.cpp
	modified:   src/cpp/fastdds/builtin/type_lookup_service/TypeLookupRequestListener.cpp
	modified:   src/cpp/fastdds/domain/DomainParticipantImpl.cpp
	modified:   src/cpp/rtps/builtin/BuiltinProtocols.cpp
	modified:   src/cpp/rtps/builtin/discovery/endpoint/EDPSimple.cpp
	modified:   src/cpp/rtps/builtin/discovery/endpoint/EDPStatic.cpp
	modified:   src/cpp/rtps/builtin/discovery/participant/PDP.cpp
	modified:   src/cpp/rtps/builtin/discovery/participant/PDPClient.cpp
	modified:   src/cpp/rtps/builtin/discovery/participant/PDPListener.cpp
	modified:   src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp
	modified:   src/cpp/rtps/builtin/discovery/participant/PDPServerListener.cpp
	modified:   src/cpp/rtps/builtin/discovery/participant/PDPSimple.cpp
	modified:   src/cpp/rtps/builtin/discovery/participant/simple/PDPStatelessWriter.cpp
	modified:   src/cpp/rtps/builtin/liveliness/WLP.cpp
	modified:   src/cpp/rtps/flowcontrol/FlowControllerFactory.cpp
	modified:   src/cpp/rtps/flowcontrol/FlowControllerImpl.hpp
	modified:   src/cpp/rtps/participant/RTPSParticipant.cpp
	modified:   src/cpp/rtps/participant/RTPSParticipantImpl.hpp
	modified:   src/cpp/rtps/reader/BaseReader.cpp
	modified:   src/cpp/rtps/reader/StatefulReader.cpp
	modified:   src/cpp/rtps/security/SecurityManager.cpp
	modified:   src/cpp/rtps/security/SecurityManager.h
	modified:   src/cpp/rtps/security/accesscontrol/AccessControl.h
	modified:   src/cpp/rtps/security/authentication/Authentication.h
	modified:   src/cpp/security/accesscontrol/Permissions.cpp
	modified:   src/cpp/security/accesscontrol/Permissions.h
	modified:   src/cpp/security/authentication/PKIDH.cpp
	modified:   src/cpp/security/authentication/PKIDH.h
	modified:   test/mock/rtps/RTPSParticipant/fastdds/rtps/participant/RTPSParticipant.hpp
	modified:   test/mock/rtps/RTPSParticipantAttributes/fastdds/rtps/attributes/RTPSParticipantAttributes.hpp
	modified:   test/mock/rtps/RTPSParticipantImpl/rtps/participant/RTPSParticipantImpl.hpp
	modified:   test/mock/rtps/SecurityPluginFactory/rtps/security/MockAccessControlPlugin.h
	modified:   test/mock/rtps/SecurityPluginFactory/rtps/security/MockAuthenticationPlugin.h
	modified:   test/unittest/dds/participant/ParticipantTests.cpp
	modified:   test/unittest/rtps/security/SecurityTests.hpp
	modified:   test/unittest/security/accesscontrol/AccessControlTests.cpp
	modified:   test/unittest/security/authentication/AuthenticationPluginTests.hpp
	modified:   test/unittest/security/authentication/BuiltinPKIDHTests.cpp

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   include/fastdds/dds/publisher/DataWriter.hpp
	both modified:   src/cpp/rtps/participant/RTPSParticipantImpl.cpp

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

Cherry-pick of 7dd4b4d has failed:

On branch mergify/bp/3.2.x/pr-6370
Your branch is up to date with 'origin/3.2.x'.

You are currently cherry-picking commit 7dd4b4d17.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   include/fastdds/rtps/attributes/RTPSParticipantAttributes.hpp
	modified:   include/fastdds/rtps/participant/RTPSParticipant.hpp
	modified:   src/cpp/fastdds/builtin/type_lookup_service/TypeLookupManager.cpp
	modified:   src/cpp/fastdds/builtin/type_lookup_service/TypeLookupReplyListener.cpp
	modified:   src/cpp/fastdds/builtin/type_lookup_service/TypeLookupRequestListener.cpp
	modified:   src/cpp/fastdds/domain/DomainParticipantImpl.cpp
	modified:   src/cpp/rtps/builtin/BuiltinProtocols.cpp
	modified:   src/cpp/rtps/builtin/discovery/endpoint/EDPSimple.cpp
	modified:   src/cpp/rtps/builtin/discovery/endpoint/EDPStatic.cpp
	modified:   src/cpp/rtps/builtin/discovery/participant/PDP.cpp
	modified:   src/cpp/rtps/builtin/discovery/participant/PDPClient.cpp
	modified:   src/cpp/rtps/builtin/discovery/participant/PDPListener.cpp
	modified:   src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp
	modified:   src/cpp/rtps/builtin/discovery/participant/PDPServerListener.cpp
	modified:   src/cpp/rtps/builtin/discovery/participant/PDPSimple.cpp
	modified:   src/cpp/rtps/builtin/discovery/participant/simple/PDPStatelessWriter.cpp
	modified:   src/cpp/rtps/builtin/liveliness/WLP.cpp
	modified:   src/cpp/rtps/flowcontrol/FlowControllerFactory.cpp
	modified:   src/cpp/rtps/flowcontrol/FlowControllerImpl.hpp
	modified:   src/cpp/rtps/participant/RTPSParticipant.cpp
	modified:   src/cpp/rtps/participant/RTPSParticipantImpl.hpp
	modified:   src/cpp/rtps/reader/BaseReader.cpp
	modified:   src/cpp/rtps/reader/StatefulReader.cpp
	modified:   src/cpp/rtps/security/SecurityManager.cpp
	modified:   src/cpp/rtps/security/SecurityManager.h
	modified:   src/cpp/rtps/security/accesscontrol/AccessControl.h
	modified:   src/cpp/rtps/security/authentication/Authentication.h
	modified:   src/cpp/security/accesscontrol/Permissions.cpp
	modified:   src/cpp/security/accesscontrol/Permissions.h
	modified:   src/cpp/security/authentication/PKIDH.cpp
	modified:   src/cpp/security/authentication/PKIDH.h
	modified:   test/mock/rtps/RTPSParticipant/fastdds/rtps/participant/RTPSParticipant.hpp
	modified:   test/mock/rtps/RTPSParticipantAttributes/fastdds/rtps/attributes/RTPSParticipantAttributes.hpp
	modified:   test/mock/rtps/RTPSParticipantImpl/rtps/participant/RTPSParticipantImpl.hpp
	modified:   test/mock/rtps/SecurityPluginFactory/rtps/security/MockAccessControlPlugin.h
	modified:   test/mock/rtps/SecurityPluginFactory/rtps/security/MockAuthenticationPlugin.h
	modified:   test/unittest/dds/participant/ParticipantTests.cpp
	modified:   test/unittest/rtps/security/SecurityTests.hpp
	modified:   test/unittest/security/accesscontrol/AccessControlTests.cpp
	modified:   test/unittest/security/authentication/AuthenticationPluginTests.hpp
	modified:   test/unittest/security/authentication/BuiltinPKIDHTests.cpp

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   include/fastdds/dds/publisher/DataWriter.hpp
	both modified:   src/cpp/rtps/participant/RTPSParticipantImpl.cpp

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

Cherry-pick of 7dd4b4d has failed:

On branch mergify/bp/2.14.x/pr-6370
Your branch is up to date with 'origin/2.14.x'.

You are currently cherry-picking commit 7dd4b4d17.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   include/fastdds/rtps/security/accesscontrol/AccessControl.h
	modified:   include/fastdds/rtps/security/authentication/Authentication.h
	modified:   src/cpp/rtps/participant/RTPSParticipant.cpp
	modified:   src/cpp/rtps/security/SecurityManager.h
	modified:   src/cpp/security/accesscontrol/Permissions.cpp
	modified:   src/cpp/security/accesscontrol/Permissions.h
	modified:   src/cpp/security/authentication/PKIDH.cpp
	modified:   src/cpp/security/authentication/PKIDH.h
	modified:   test/mock/rtps/RTPSParticipantImpl/rtps/participant/RTPSParticipantImpl.h
	modified:   test/mock/rtps/SecurityPluginFactory/rtps/security/MockAccessControlPlugin.h
	modified:   test/mock/rtps/SecurityPluginFactory/rtps/security/MockAuthenticationPlugin.h
	modified:   test/unittest/rtps/security/SecurityTests.hpp
	modified:   test/unittest/security/accesscontrol/AccessControlTests.cpp
	modified:   test/unittest/security/authentication/AuthenticationPluginTests.hpp
	modified:   test/unittest/security/authentication/BuiltinPKIDHTests.cpp

Unmerged paths:
  (use "git add/rm <file>..." as appropriate to mark resolution)
	both modified:   include/fastdds/dds/publisher/DataWriter.hpp
	both modified:   include/fastdds/rtps/attributes/RTPSParticipantAttributes.h
	both modified:   include/fastdds/rtps/participant/RTPSParticipant.h
	deleted by us:   src/cpp/fastdds/builtin/type_lookup_service/TypeLookupManager.cpp
	deleted by us:   src/cpp/fastdds/builtin/type_lookup_service/TypeLookupReplyListener.cpp
	deleted by us:   src/cpp/fastdds/builtin/type_lookup_service/TypeLookupRequestListener.cpp
	both modified:   src/cpp/fastdds/domain/DomainParticipantImpl.cpp
	both modified:   src/cpp/rtps/builtin/BuiltinProtocols.cpp
	both modified:   src/cpp/rtps/builtin/discovery/endpoint/EDPSimple.cpp
	both modified:   src/cpp/rtps/builtin/discovery/endpoint/EDPStatic.cpp
	both modified:   src/cpp/rtps/builtin/discovery/participant/PDP.cpp
	both modified:   src/cpp/rtps/builtin/discovery/participant/PDPClient.cpp
	both modified:   src/cpp/rtps/builtin/discovery/participant/PDPListener.cpp
	both modified:   src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp
	both modified:   src/cpp/rtps/builtin/discovery/participant/PDPServerListener.cpp
	both modified:   src/cpp/rtps/builtin/discovery/participant/PDPSimple.cpp
	both modified:   src/cpp/rtps/builtin/discovery/participant/simple/PDPStatelessWriter.cpp
	both modified:   src/cpp/rtps/builtin/liveliness/WLP.cpp
	both modified:   src/cpp/rtps/flowcontrol/FlowControllerFactory.cpp
	both modified:   src/cpp/rtps/flowcontrol/FlowControllerImpl.hpp
	both modified:   src/cpp/rtps/participant/RTPSParticipantImpl.cpp
	both modified:   src/cpp/rtps/participant/RTPSParticipantImpl.h
	deleted by us:   src/cpp/rtps/reader/BaseReader.cpp
	both modified:   src/cpp/rtps/reader/StatefulReader.cpp
	both modified:   src/cpp/rtps/security/SecurityManager.cpp
	both modified:   test/mock/rtps/RTPSParticipant/fastdds/rtps/participant/RTPSParticipant.h
	both modified:   test/mock/rtps/RTPSParticipantAttributes/fastdds/rtps/attributes/RTPSParticipantAttributes.h
	both modified:   test/unittest/dds/participant/ParticipantTests.cpp

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fast dds pro ready-to-merge Ready to be merged. CI and changes have been reviewed and approved.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Filter remote locators crash

4 participants