Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[17148] Improve behavior when HAVE_STRICT_REALTIME is not set. #3288

Merged
merged 5 commits into from
Feb 10, 2023
Merged

[17148] Improve behavior when HAVE_STRICT_REALTIME is not set. #3288

merged 5 commits into from
Feb 10, 2023

Conversation

MiguelCompany
Copy link
Member

@MiguelCompany MiguelCompany commented Feb 10, 2023 •
edited
Loading

Description

This PR avoids using try_lock_until when HAVE_STRICT_REALTIME has not been set (in which case it was using a 24 hours timeout).

It also fixes a bug in the custom TimedMutex, which was always using steady_clock::now for calculating the required timeout.

@Mergifyio backport 2.9.x 2.8.x 2.6.x 2.1.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.
  • Changes are ABI compatible.
  • Changes are API compatible.
  • N/A Documentation builds and tests pass locally.
  • 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.
  • Applicable backports have been included in the description.

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.

jsan-rt
jsan-rt suggested changes Feb 10, 2023
View reviewed changes
Copy link
Contributor

@jsan-rt jsan-rt left a comment

Choose a reason for hiding this comment

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

Changes look good so far but I think there are some other instances of try_lock_until that are not accounted for in DataReaderImpl and SubscriberHistory.

Our implementation of shared_mutex could also be modified to be in line with the new TimedMutex implementation since it also has timed locking capabilities.

@MiguelCompany
Copy link
Member Author

Changes look good so far but I think there are some other instances of try_lock_until that are not accounted for in DataReaderImpl and SubscriberHistory.

Changes for DataReaderImpl on 4d07531. For SubscriberHistory, it is part of the deprecated API, on which we are not deploying improvements.

Our implementation of shared_mutex could also be modified to be in line with the new TimedMutex implementation since it also has timed locking capabilities.

I had a look at it, but we are not using try_lock_until with them, and they rely on try_lock_shared_until, so I don't know whether we have anything to be done there.

@MiguelCompany
Copy link
Member Author

@richiprosima Please test this

@MiguelCompany MiguelCompany added this to the v2.10.0 milestone Feb 10, 2023
@MiguelCompany
Copy link
Member Author

@richiprosima Please test mac again to check whether the failed test is flakey

@MiguelCompany MiguelCompany merged commit 0cf43de into master Feb 10, 2023
@MiguelCompany MiguelCompany deleted the hotfix/17148/master branch February 10, 2023 15:28
@MiguelCompany
Copy link
Member Author

https://github.com/Mergifyio backport 2.9.x 2.8.x 2.6.x 2.1.x

@mergify mergify bot mentioned this pull request Feb 10, 2023
Merged
mergify bot pushed a commit that referenced this pull request Feb 10, 2023
@MiguelCompany @mergify
Improve behavior when HAVE_STRICT_REALTIME is not set (#3288)
77cc36a 
* Refs #17148. Avoid try_lock_until on RTPSParticipantImpl.

Signed-off-by: Miguel Company <[email protected]>

* Refs #17148. Avoid try_lock_until on RTPSReader.

Signed-off-by: Miguel Company <[email protected]>

* Refs #17148. Avoid try_lock_until on ResourceEvent.

Signed-off-by: Miguel Company <[email protected]>

* Refs #17148. Using right clock on custom TimedMutex.

Signed-off-by: Miguel Company <[email protected]>

* Refs #17148. Avoid try_lock_until on DataReaderImpl.

Signed-off-by: Miguel Company <[email protected]>

---------

Signed-off-by: Miguel Company <[email protected]>
(cherry picked from commit 0cf43de)
@mergify mergify bot mentioned this pull request Feb 10, 2023
Merged
mergify bot pushed a commit that referenced this pull request Feb 10, 2023
@MiguelCompany @mergify
Improve behavior when HAVE_STRICT_REALTIME is not set (#3288)
bb30f04 
* Refs #17148. Avoid try_lock_until on RTPSParticipantImpl.

Signed-off-by: Miguel Company <[email protected]>

* Refs #17148. Avoid try_lock_until on RTPSReader.

Signed-off-by: Miguel Company <[email protected]>

* Refs #17148. Avoid try_lock_until on ResourceEvent.

Signed-off-by: Miguel Company <[email protected]>

* Refs #17148. Using right clock on custom TimedMutex.

Signed-off-by: Miguel Company <[email protected]>

* Refs #17148. Avoid try_lock_until on DataReaderImpl.

Signed-off-by: Miguel Company <[email protected]>

---------

Signed-off-by: Miguel Company <[email protected]>
(cherry picked from commit 0cf43de)
@mergify mergify bot mentioned this pull request Feb 10, 2023
Merged
mergify bot pushed a commit that referenced this pull request Feb 10, 2023
@MiguelCompany @mergify
Improve behavior when HAVE_STRICT_REALTIME is not set (#3288)
34f03ac 
* Refs #17148. Avoid try_lock_until on RTPSParticipantImpl.

Signed-off-by: Miguel Company <[email protected]>

* Refs #17148. Avoid try_lock_until on RTPSReader.

Signed-off-by: Miguel Company <[email protected]>

* Refs #17148. Avoid try_lock_until on ResourceEvent.

Signed-off-by: Miguel Company <[email protected]>

* Refs #17148. Using right clock on custom TimedMutex.

Signed-off-by: Miguel Company <[email protected]>

* Refs #17148. Avoid try_lock_until on DataReaderImpl.

Signed-off-by: Miguel Company <[email protected]>

---------

Signed-off-by: Miguel Company <[email protected]>
(cherry picked from commit 0cf43de)
@mergify mergify bot mentioned this pull request Feb 10, 2023
Merged
mergify bot pushed a commit that referenced this pull request Feb 10, 2023
@MiguelCompany @mergify
Improve behavior when HAVE_STRICT_REALTIME is not set (#3288)
cfdb0f9 
* Refs #17148. Avoid try_lock_until on RTPSParticipantImpl.

Signed-off-by: Miguel Company <[email protected]>

* Refs #17148. Avoid try_lock_until on RTPSReader.

Signed-off-by: Miguel Company <[email protected]>

* Refs #17148. Avoid try_lock_until on ResourceEvent.

Signed-off-by: Miguel Company <[email protected]>

* Refs #17148. Using right clock on custom TimedMutex.

Signed-off-by: Miguel Company <[email protected]>

* Refs #17148. Avoid try_lock_until on DataReaderImpl.

Signed-off-by: Miguel Company <[email protected]>

---------

Signed-off-by: Miguel Company <[email protected]>
(cherry picked from commit 0cf43de)

# Conflicts:
#	src/cpp/fastdds/subscriber/DataReaderImpl.cpp
@mergify
Copy link
Contributor

mergify bot commented Feb 10, 2023

backport 2.9.x 2.8.x 2.6.x 2.1.x

✅ Backports have been created

EduPonz pushed a commit that referenced this pull request May 12, 2023
@mergify @MiguelCompany @jsan-rt
Improve behavior when HAVE_STRICT_REALTIME is not set (backport #3288) (
838777e 
#3292)

* Improve behavior when HAVE_STRICT_REALTIME is not set (#3288)

* Refs #17148. Avoid try_lock_until on RTPSParticipantImpl.

Signed-off-by: Miguel Company <[email protected]>

* Refs #17148. Avoid try_lock_until on RTPSReader.

Signed-off-by: Miguel Company <[email protected]>

* Refs #17148. Avoid try_lock_until on ResourceEvent.

Signed-off-by: Miguel Company <[email protected]>

* Refs #17148. Using right clock on custom TimedMutex.

Signed-off-by: Miguel Company <[email protected]>

* Refs #17148. Avoid try_lock_until on DataReaderImpl.

Signed-off-by: Miguel Company <[email protected]>

---------

Signed-off-by: Miguel Company <[email protected]>
(cherry picked from commit 0cf43de)

# Conflicts:
#	src/cpp/fastdds/subscriber/DataReaderImpl.cpp

* Fix conflicts.

Signed-off-by: Miguel Company <[email protected]>

* Add implementation to SubscriberHistory.

Signed-off-by: Miguel Company <[email protected]>

* Fixed unused variable warnings

Signed-off-by: Javier Santiago <[email protected]>

---------

Signed-off-by: Miguel Company <[email protected]>
Signed-off-by: Javier Santiago <[email protected]>
Co-authored-by: Miguel Company <[email protected]>
Co-authored-by: Javier Santiago <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jsan-rt jsan-rt jsan-rt approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v2.10.0
Development

Successfully merging this pull request may close these issues.
2 participants
@MiguelCompany @jsan-rt