Fast-DDS icon indicating copy to clipboard operation
Fast-DDS copied to clipboard

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

Open MiguelBarro opened this issue 3 years ago • 6 comments

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: https://github.com/eProsima/Fast-DDS-docs/pull/413

Contributor Checklist

  • [x] Commit messages follow the project guidelines.
  • [x] The code follows the style guidelines of this project.
  • [x] Tests that thoroughly check the new feature have been added/Regression tests checking the bug and its fix have been added.
  • [x] Any new/modified methods have been properly documented using Doxygen.
  • [x] Fast DDS test suite has been run locally.
  • [x] Changes are ABI compatible.
  • [x] Changes are API compatible.
  • [x] Documentation builds and tests pass locally.
  • [] NA New feature has been added to the versions.md file (if applicable).
  • [x] 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 avatar Sep 24 '22 16:09 MiguelBarro

@richiprosima Please test this for me 🤯

MiguelBarro avatar Sep 24 '22 16:09 MiguelBarro

@richiprosima Please test this for me 🤯

MiguelBarro avatar Sep 24 '22 21:09 MiguelBarro

@richiprosima please test this

jsan-rt avatar Oct 03 '22 05:10 jsan-rt

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

MiguelBarro avatar Oct 03 '22 06:10 MiguelBarro

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

MiguelBarro avatar Oct 03 '22 09:10 MiguelBarro

@richiprosima please test this

MiguelBarro avatar Oct 10 '22 06:10 MiguelBarro

Is there any progress?

wade30822 avatar Oct 26 '22 06:10 wade30822

@richiprosima Please test this

MiguelCompany avatar Nov 04 '22 15:11 MiguelCompany

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.

jparisu avatar Nov 07 '22 07:11 jparisu

@richiprosima Please test this for me stan-monkey

MiguelBarro avatar Nov 08 '22 07:11 MiguelBarro

@richiprosima Please test this for me 🤯

MiguelBarro avatar Nov 08 '22 12:11 MiguelBarro

@richiprosima Please test this for me

MiguelBarro avatar Nov 08 '22 14:11 MiguelBarro

@richiprosima Please test this for me

MiguelBarro avatar Nov 09 '22 08:11 MiguelBarro

@richiprosima Please test this for me

MiguelBarro avatar Nov 09 '22 16:11 MiguelBarro

@richiprosima Please test this for me

MiguelBarro avatar Nov 09 '22 20:11 MiguelBarro

@richiprosima Please test this for me

MiguelBarro avatar Nov 11 '22 10:11 MiguelBarro

@richiprosima Please test this for me

MiguelBarro avatar Nov 11 '22 10:11 MiguelBarro

@richiprosima Please test this for me

MiguelBarro avatar Nov 11 '22 22:11 MiguelBarro

@mergifyio backport 2.7.x 2.6.x

MiguelCompany avatar Nov 14 '22 13:11 MiguelCompany

backport 2.7.x 2.6.x

✅ Backports have been created

mergify[bot] avatar Nov 14 '22 13:11 mergify[bot]