[15766] Update shared_mutex thirdparty to don't prioritize writers #2976

MiguelBarro · 2022-09-24T16:04:33Z

Description

The C++ standard requirements for shared_mutex do not specify if exclusive lock operation should preempt the shared lock one. Posix allows both behaviors but defaults to not preempting shared locks. STL implementations are split on this subject:

Windows, boost -> preemption (PTHREAD_RWLOCK_PREFER_WRITER_NONRECURSIVE_NP)
Linux, Mac -> not preemption ( PTHREAD_RWLOCK_PREFER_READER_NP)

In order to prevent deadlock scenarios:

out third party implementation avoids preemption.
if the framework's defaults to preemption the third party is forced.
code checks used to avoid deadlock on the preemption scenario have been removed.

Documentation: eProsima/Fast-DDS-docs#413

Contributor Checklist

Commit messages follow the project guidelines.
The code follows the style guidelines of this project.
Tests that thoroughly check the new feature have been added/Regression tests checking the bug and its fix have been added.
Any new/modified methods have been properly documented using Doxygen.
Fast DDS test suite has been run locally.
Changes are ABI compatible.
Changes are API compatible.
Documentation builds and tests pass locally.
[] NA New feature has been added to the versions.md file (if applicable).
New feature has been documented/Current behavior is correctly described in the documentation.

Reviewer Checklist

Check contributor checklist is correct.
Check CI results: changes do not issue any warning.
Check CI results: failing tests are unrelated with the changes.

MiguelBarro · 2022-09-24T16:11:14Z

@richiprosima Please test this for me 🤯

MiguelBarro · 2022-09-24T21:14:00Z

@richiprosima Please test this for me 🤯

jsan-rt · 2022-10-03T05:59:11Z

@richiprosima please test this

MiguelBarro · 2022-10-03T06:43:31Z

@richiprosima Please I have to rebase in order to fix docs ci, test this again 😅

MiguelBarro · 2022-10-03T09:06:43Z

@richiprosima Please I have to rebase in order to fix docs ci, test this again 😅

MiguelBarro · 2022-10-10T06:04:40Z

@richiprosima please test this

wade30822 · 2022-10-26T06:48:32Z

Is there any progress？

MiguelCompany · 2022-11-04T15:01:31Z

@richiprosima Please test this

jparisu · 2022-11-07T07:27:51Z

Some of the checked checklist steps look incorrect:

The code follows the style guidelines of this project.
Any new/modified methods have been properly documented using Doxygen.
Tests that thoroughly check the new feature have been added/Regression tests checking the bug and its fix have been added.
New feature has been documented/Current behavior is correctly described in the documentation.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

Signed-off-by: Miguel Barro <miguelbarro@eprosima.com>

…mework prioritizes writing. Signed-off-by: Miguel Barro <miguelbarro@eprosima.com>

Signed-off-by: Miguel Barro <miguelbarro@eprosima.com>

…texes. Now they always allow them. Signed-off-by: Miguel Barro <miguelbarro@eprosima.com>

Signed-off-by: Miguel Barro <miguelbarro@eprosima.com>

…_MUTEX value is already specified. Signed-off-by: Miguel Barro <miguelbarro@eprosima.com>

MiguelBarro · 2022-11-09T20:17:31Z

@richiprosima Please test this for me

MiguelCompany

Apart from my comment below, please investigate this failure

include/fastrtps/utils/shared_mutex.hpp

MiguelBarro · 2022-11-11T10:23:31Z

@richiprosima Please test this for me

Signed-off-by: Miguel Barro <miguelbarro@eprosima.com>

MiguelBarro · 2022-11-11T10:29:59Z

@richiprosima Please test this for me

MiguelCompany

LGTM with green CI!

Signed-off-by: Miguel Barro <miguelbarro@eprosima.com>

MiguelBarro · 2022-11-11T22:26:31Z

@richiprosima Please test this for me

MiguelCompany

LGTM

MiguelCompany · 2022-11-14T13:22:15Z

@Mergifyio backport 2.7.x 2.6.x

mergify · 2022-11-14T13:23:15Z

backport 2.7.x 2.6.x

✅ Backports have been created

#3090 [15766] Update shared_mutex thirdparty to don't prioritize writers (backport #2976) has been created for branch 2.7.x
#3091 [15766] Update shared_mutex thirdparty to don't prioritize writers (backport #2976) has been created for branch 2.6.x

* Add test for multithreaded creation of readers on a single subscriber. Signed-off-by: Miguel Company <MiguelCompany@eprosima.com> * Add DataWriter. Signed-off-by: Miguel Company <MiguelCompany@eprosima.com> * Refs 15766: Refactor of shared_mutex to select writer priority Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766: CMake update to force third party shared mutex if the framework prioritizes writing. Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766: Add atomic support for some debian distros Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766: Allow recursiveness on participant endpoint collection mutexes. Now they always allow them. Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766: linter pass Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766: Generate the config.h file when the USE_THIRDPARTY_SHARED_MUTEX value is already specified. Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766: Mandatory piggyback: avoid polution on free_pools_ collection Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766: Add some missing members to shared_lock thirdparty Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766: linter Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766. shared_mutex testing Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766. fixing gtest backward compatibility issues Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766. Add a new test to check priority is right Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766. Fixing gcc build warnings Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766. Fixing clang build warnings Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766. Make thirdparty versions always available even if none is used as eprosima::shared_mutex. Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766. Addressing reviewers comments Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766. Fixing sync issue on ProxyPool. Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> Signed-off-by: Miguel Company <MiguelCompany@eprosima.com> Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> Co-authored-by: Miguel Company <MiguelCompany@eprosima.com>

* Update shared_mutex thirdparty to not prioritize writers (#2976) * Add test for multithreaded creation of readers on a single subscriber. Signed-off-by: Miguel Company <MiguelCompany@eprosima.com> * Add DataWriter. Signed-off-by: Miguel Company <MiguelCompany@eprosima.com> * Refs 15766: Refactor of shared_mutex to select writer priority Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766: CMake update to force third party shared mutex if the framework prioritizes writing. Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766: Add atomic support for some debian distros Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766: Allow recursiveness on participant endpoint collection mutexes. Now they always allow them. Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766: linter pass Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766: Generate the config.h file when the USE_THIRDPARTY_SHARED_MUTEX value is already specified. Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766: Mandatory piggyback: avoid polution on free_pools_ collection Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766: Add some missing members to shared_lock thirdparty Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766: linter Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766. shared_mutex testing Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766. fixing gtest backward compatibility issues Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766. Add a new test to check priority is right Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766. Fixing gcc build warnings Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766. Fixing clang build warnings Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766. Make thirdparty versions always available even if none is used as eprosima::shared_mutex. Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766. Addressing reviewers comments Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766. Fixing sync issue on ProxyPool. Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> Signed-off-by: Miguel Company <MiguelCompany@eprosima.com> Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> Co-authored-by: Miguel Company <MiguelCompany@eprosima.com> * Fixing gcc warnings Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * make sure config.h is always included through fastrtps_dll.h as is expected. Do we need a foreport? Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> Signed-off-by: Miguel Company <MiguelCompany@eprosima.com> Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> Co-authored-by: MiguelBarro <45819833+MiguelBarro@users.noreply.github.com> Co-authored-by: Miguel Company <MiguelCompany@eprosima.com> Co-authored-by: Miguel Barro <miguelbarro@eprosima.com>

* Add test for multithreaded creation of readers on a single subscriber. Signed-off-by: Miguel Company <MiguelCompany@eprosima.com> * Add DataWriter. Signed-off-by: Miguel Company <MiguelCompany@eprosima.com> * Refs 15766: Refactor of shared_mutex to select writer priority Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766: CMake update to force third party shared mutex if the framework prioritizes writing. Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766: Add atomic support for some debian distros Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766: Allow recursiveness on participant endpoint collection mutexes. Now they always allow them. Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766: linter pass Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766: Generate the config.h file when the USE_THIRDPARTY_SHARED_MUTEX value is already specified. Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766: Mandatory piggyback: avoid polution on free_pools_ collection Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766: Add some missing members to shared_lock thirdparty Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766: linter Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766. shared_mutex testing Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766. fixing gtest backward compatibility issues Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766. Add a new test to check priority is right Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766. Fixing gcc build warnings Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766. Fixing clang build warnings Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766. Make thirdparty versions always available even if none is used as eprosima::shared_mutex. Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766. Addressing reviewers comments Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> * Refs 15766. Fixing sync issue on ProxyPool. Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> Signed-off-by: Miguel Company <MiguelCompany@eprosima.com> Signed-off-by: Miguel Barro <miguelbarro@eprosima.com> Co-authored-by: Miguel Company <MiguelCompany@eprosima.com>

MiguelBarro changed the title ~~Update shared_mutex thirdparty to don't prioritize writers~~ [15766] Update shared_mutex thirdparty to don't prioritize writers Oct 2, 2022

MiguelBarro force-pushed the feature/shared_mutex/writer_priority branch from 97e3f18 to afcb602 Compare October 2, 2022 15:39

MiguelBarro mentioned this pull request Oct 2, 2022

Refs 15766: update shared_mutex docs eProsima/Fast-DDS-docs#413

Merged

MiguelBarro self-assigned this Oct 2, 2022

MiguelBarro marked this pull request as ready for review October 2, 2022 16:18

MiguelBarro force-pushed the feature/shared_mutex/writer_priority branch from afcb602 to 1c149b0 Compare October 3, 2022 06:35

MiguelBarro force-pushed the feature/shared_mutex/writer_priority branch from 1c149b0 to 5af6af9 Compare October 3, 2022 09:06

jsan-rt added this to the v2.9.0 milestone Oct 3, 2022

MiguelCompany mentioned this pull request Oct 4, 2022

Deadlock in v2.6.2 #2961

Closed

1 task

MiguelBarro removed this from the v2.9.0 milestone Oct 6, 2022

fujitatomoya mentioned this pull request Nov 2, 2022

Failing to create DataReader leads to Segmentation Fault #3056

Closed

1 task

JLBuenoLopez mentioned this pull request Nov 4, 2022

Problems running SecureHelloWorldExample: subscriber will be blocked. #3054

Closed

1 task

MiguelCompany added this to the v2.8.1 milestone Nov 7, 2022

MiguelCompany and others added 8 commits November 8, 2022 08:05

Add test for multithreaded creation of readers on a single subscriber.

a8c6c21

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

Add DataWriter.

5e284f3

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

Refs 15766: Refactor of shared_mutex to select writer priority

f17a4e8

Signed-off-by: Miguel Barro <miguelbarro@eprosima.com>

Refs 15766: CMake update to force third party shared mutex if the fra…

7492731

…mework prioritizes writing. Signed-off-by: Miguel Barro <miguelbarro@eprosima.com>

Refs 15766: Add atomic support for some debian distros

b3bf9ac

Signed-off-by: Miguel Barro <miguelbarro@eprosima.com>

Refs 15766: Allow recursiveness on participant endpoint collection mu…

ced4885

…texes. Now they always allow them. Signed-off-by: Miguel Barro <miguelbarro@eprosima.com>

Refs 15766: linter pass

e0653c4

Signed-off-by: Miguel Barro <miguelbarro@eprosima.com>

Refs 15766: Generate the config.h file when the USE_THIRDPARTY_SHARED…

8bf908e

…_MUTEX value is already specified. Signed-off-by: Miguel Barro <miguelbarro@eprosima.com>

MiguelCompany requested changes Nov 11, 2022

View reviewed changes

include/fastrtps/utils/shared_mutex.hpp Show resolved Hide resolved

MiguelBarro force-pushed the feature/shared_mutex/writer_priority branch from 0474f1a to adf2351 Compare November 11, 2022 10:22

Refs 15766. Addressing reviewers comments

50388f0

Signed-off-by: Miguel Barro <miguelbarro@eprosima.com>

MiguelBarro force-pushed the feature/shared_mutex/writer_priority branch from adf2351 to 50388f0 Compare November 11, 2022 10:29

MiguelCompany previously approved these changes Nov 11, 2022

View reviewed changes

Refs 15766. Fixing sync issue on ProxyPool.

9d7259a

Signed-off-by: Miguel Barro <miguelbarro@eprosima.com>

MiguelBarro dismissed MiguelCompany’s stale review via 9d7259a November 11, 2022 22:10

MiguelCompany approved these changes Nov 14, 2022

View reviewed changes

MiguelCompany added ready-to-merge Ready to be merged. CI and changes have been reviewed and approved. doc-pending Issue or PR which is pending to be documented and removed doc-pending Issue or PR which is pending to be documented labels Nov 14, 2022

MiguelCompany merged commit 47d85d3 into master Nov 14, 2022

MiguelCompany deleted the feature/shared_mutex/writer_priority branch November 14, 2022 13:18

mergify bot mentioned this pull request Nov 14, 2022

[15766] Update shared_mutex thirdparty to don't prioritize writers (backport #2976) #3090

Merged

mergify bot mentioned this pull request Nov 14, 2022

[15766] Update shared_mutex thirdparty to don't prioritize writers (backport #2976) #3091

Merged

Mario-DL mentioned this pull request Dec 1, 2022

dds crashes when writing #2701

Closed

1 task

fujitatomoya mentioned this pull request Dec 12, 2022

Deadlock when using MultiThreadedExecutor ros2/rmw_fastrtps#650

Closed

iuhilnehc-ynos mentioned this pull request Feb 21, 2023

busy loop consumes CPU 100% ros2/rmw_fastrtps#666

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[15766] Update shared_mutex thirdparty to don't prioritize writers #2976

[15766] Update shared_mutex thirdparty to don't prioritize writers #2976

MiguelBarro commented Sep 24, 2022 •

edited by MiguelCompany

Loading

MiguelBarro commented Sep 24, 2022

MiguelBarro commented Sep 24, 2022

jsan-rt commented Oct 3, 2022

MiguelBarro commented Oct 3, 2022 •

edited

Loading

MiguelBarro commented Oct 3, 2022

MiguelBarro commented Oct 10, 2022

wade30822 commented Oct 26, 2022

MiguelCompany commented Nov 4, 2022

jparisu commented Nov 7, 2022 •

edited

Loading

MiguelBarro commented Nov 9, 2022

MiguelCompany left a comment

MiguelBarro commented Nov 11, 2022

MiguelBarro commented Nov 11, 2022

MiguelCompany left a comment

MiguelBarro commented Nov 11, 2022

MiguelCompany left a comment

MiguelCompany commented Nov 14, 2022

mergify bot commented Nov 14, 2022

[15766] Update shared_mutex thirdparty to don't prioritize writers #2976

[15766] Update shared_mutex thirdparty to don't prioritize writers #2976

Conversation

MiguelBarro commented Sep 24, 2022 • edited by MiguelCompany Loading

Description

Contributor Checklist

Reviewer Checklist

MiguelBarro commented Sep 24, 2022

MiguelBarro commented Sep 24, 2022

jsan-rt commented Oct 3, 2022

MiguelBarro commented Oct 3, 2022 • edited Loading

MiguelBarro commented Oct 3, 2022

MiguelBarro commented Oct 10, 2022

wade30822 commented Oct 26, 2022

MiguelCompany commented Nov 4, 2022

jparisu commented Nov 7, 2022 • edited Loading

MiguelBarro commented Nov 9, 2022

MiguelCompany left a comment

Choose a reason for hiding this comment

MiguelBarro commented Nov 11, 2022

MiguelBarro commented Nov 11, 2022

MiguelCompany left a comment

Choose a reason for hiding this comment

MiguelBarro commented Nov 11, 2022

MiguelCompany left a comment

Choose a reason for hiding this comment

MiguelCompany commented Nov 14, 2022

mergify bot commented Nov 14, 2022

✅ Backports have been created

MiguelBarro commented Sep 24, 2022 •

edited by MiguelCompany

Loading

MiguelBarro commented Oct 3, 2022 •

edited

Loading

jparisu commented Nov 7, 2022 •

edited

Loading