Skip to content

Fix total_unread_ consistent with reader's history after get_first_untaken_info#3223

Merged
MiguelCompany merged 6 commits intomasterfrom
hotfix/16608
Jan 19, 2023
Merged

Fix total_unread_ consistent with reader's history after get_first_untaken_info#3223
MiguelCompany merged 6 commits intomasterfrom
hotfix/16608

Conversation

@MiguelCompany
Copy link
Copy Markdown
Member

@MiguelCompany MiguelCompany commented Jan 18, 2023 •
edited by EduPonz
Loading

Description

This is a rework of #3203 that avoids the segfault reported on ros2/rmw_fastrtps#659

Backports: We will update the currently created PRs (#3217, #3218, #3219) accordingly

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.
  • N/A 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.
  • 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.

Mario-DL and others added 5 commits January 18, 2023 07:16
@Mario-DL @MiguelCompany
Fix total_unread_ consistent with reader's history after get_first_un…
f1706d4 
…taken_info (#3203)

* Refs 16608: Added BlackBoxTest

Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com>

* Refs 16608: Fix. Make get_first_untaken_info() a read-only API call

Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com>

* Refs #16608: Fix tsan warning

Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com>

* Refs #16608: Applied suggested changes

Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com>

* Refs #16608: Applied second-reviewed suggested changes

Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com>

* Refs #16608: Skip test for Data-Sharing

Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com>

Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com>
@MiguelCompany
Refs #16608. Reversed unnecessary changes on DDSBlackboxTestsBasic
7a4f7d6 
Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
@MiguelCompany
Refs #16608. Not using shared pointer for PubSubReader and PubSubWriter.
72f4574 
Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
@MiguelCompany
Refs #16608. Improved test.
b9d3e01 
Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
@MiguelCompany
Refs #16608. Improved solution.
3923ee3 
Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
Mario-DL
Mario-DL previously approved these changes Jan 18, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@Mario-DL Mario-DL left a comment

Choose a reason for hiding this comment

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

LGTM

Mario-DL
Mario-DL reviewed Jan 18, 2023
View reviewed changes
@MiguelCompany
Refs #16608. Fix linters.
112af3f 
Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
Mario-DL
Mario-DL approved these changes Jan 18, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@Mario-DL Mario-DL left a comment

Choose a reason for hiding this comment

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

👍

@MiguelCompany
Copy link
Copy Markdown
Member Author

MiguelCompany commented Jan 18, 2023

@fujitatomoya Would you mind running a ROS2 CI with this one?

@EduPonz EduPonz added this to the v2.9.1 milestone Jan 18, 2023
@EduPonz EduPonz added the ci-pending PR which CI is running label Jan 18, 2023
@MiguelCompany MiguelCompany 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 Jan 18, 2023
fujitatomoya
fujitatomoya approved these changes Jan 18, 2023
View reviewed changes
@fujitatomoya
Copy link
Copy Markdown
Contributor

fujitatomoya commented Jan 18, 2023

Full CI for rmw_fastrtps:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@fujitatomoya
Copy link
Copy Markdown
Contributor

fujitatomoya commented Jan 18, 2023

  • Linux Build Status

@MiguelCompany MiguelCompany merged commit 4398841 into master Jan 19, 2023
@MiguelCompany MiguelCompany deleted the hotfix/16608 branch January 19, 2023 06:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@fujitatomoya fujitatomoya fujitatomoya approved these changes

@Mario-DL Mario-DL Mario-DL approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

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

Projects

None yet

Milestone

v2.9.1

Development

Successfully merging this pull request may close these issues.

4 participants

@MiguelCompany @fujitatomoya @Mario-DL @EduPonz