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

Fix segfault when creating two participant with same fixed id [18051] #3443

Merged
merged 11 commits into from
Apr 13, 2023
Merged

Fix segfault when creating two participant with same fixed id [18051] #3443

merged 11 commits into from
Apr 13, 2023

Conversation

richiware
Copy link
Member

@richiware richiware commented Apr 10, 2023 •
edited by MiguelCompany
Loading

"

Description

Second participant creation fails, but when its DomainParticipantImpl will be removed from the factory, the code remove the one of the first participant. This is because they share the same GUID.

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; the added tests pass locally
  • Any new/modified methods have been properly documented using Doxygen.
  • Changes are ABI compatible.
  • 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.
  • Check contributor checklist is correct.
  • Check CI results: changes do not issue any warning.
  • Check CI results: failing tests are unrelated with the changes.
    "

@richiware
Refs #18050. Add regression test
5118b56 
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
@richiware
Refs #18050. Fix segfault
4482d40 
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
@richiware richiware changed the title Fix segfault when creating two participant with same fixed id Fix segfault when creating two participant with same fixed id [18051] Apr 10, 2023
@MiguelCompany MiguelCompany added this to the v2.10.2 milestone Apr 10, 2023
MiguelCompany
MiguelCompany requested changes Apr 10, 2023
View reviewed changes
test/unittest/dds/participant/ParticipantTests.cpp Outdated Show resolved Hide resolved
test/unittest/dds/participant/ParticipantTests.cpp Outdated Show resolved Hide resolved
test/unittest/dds/participant/ParticipantTests.cpp Show resolved Hide resolved
src/cpp/rtps/RTPSDomain.cpp Outdated Show resolved Hide resolved
src/cpp/rtps/RTPSDomain.cpp Outdated Show resolved Hide resolved
richiware and others added 3 commits April 10, 2023 16:10
@richiware @MiguelCompany
Apply suggestions from code review
2c0302e 
Co-authored-by: Miguel Company <miguelcompany@eprosima.com>
@richiware
Refs #18050. Apply suggestions
5355856 
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
@richiware
Refs #18050. improvement regression test
c52b494 
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
MiguelCompany
MiguelCompany requested changes Apr 10, 2023
View reviewed changes
src/cpp/rtps/RTPSDomainImpl.hpp Outdated Show resolved Hide resolved
src/cpp/fastdds/domain/DomainParticipantImpl.cpp Show resolved Hide resolved
src/cpp/fastdds/domain/DomainParticipantImpl.cpp Outdated Show resolved Hide resolved
test/unittest/dds/participant/ParticipantTests.cpp Outdated Show resolved Hide resolved
richiware and others added 2 commits April 11, 2023 07:34
@richiware @MiguelCompany
Apply suggestions from code review
7897564 
Co-authored-by: Miguel Company <miguelcompany@eprosima.com>
@richiware
Refs #18051. Apply suggestions.
3dcf51d 
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
MiguelCompany
MiguelCompany previously approved these changes Apr 11, 2023
View reviewed changes
@MiguelCompany
Copy link
Member

@richiprosima Please test this

@MiguelCompany MiguelCompany added the ci-pending PR which CI is running label Apr 11, 2023
@richiware
Refs #18051. Fix mock
eceabf0 
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
@MiguelCompany
Copy link
Member

@richiprosima Please test this

MiguelCompany
MiguelCompany previously approved these changes Apr 11, 2023
View reviewed changes
Copy link
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.

LGTM with green CI

@richiware
Refs #18051. Fix statistics mock
3b12f56 
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
@richiware
Copy link
Member Author

@richiprosima Please test this

@richiware
Refs #18051. Fix uncrustify
3055676 
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
@richiware
Copy link
Member Author

@richiprosima Please test this

MiguelCompany
MiguelCompany previously approved these changes Apr 11, 2023
View reviewed changes
Copy link
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.

LGTM with green CI

@richiware
Refs #18051. Fix memory leak
3ab84f9 
Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
@richiware
Copy link
Member Author

@richiprosima Please test this

MiguelCompany
MiguelCompany approved these changes Apr 12, 2023
View reviewed changes
Copy link
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.

LGTM with green CI

@MiguelCompany
Copy link
Member

@richiprosima Please test windows

@richiware
Copy link
Member Author

@richiprosima Please test windows

@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 Apr 12, 2023
@richiware richiware merged commit 3a168ed into master Apr 13, 2023
@richiware richiware deleted the bugfix/18050 branch April 13, 2023 05:32
@JLBuenoLopez
Copy link
Contributor

@Mergifyio backport 2.9.x 2.6.x

@mergify
Copy link
Contributor

mergify bot commented Jun 27, 2023 •
edited
Loading

backport 2.9.x 2.6.x

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request Jun 27, 2023
@richiware @mergify
Fix segfault when creating two participant with same fixed id (#3443)
59094af 
* Refs #18050. Add regression test

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

* Refs #18050. Fix segfault

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

* Apply suggestions from code review

Co-authored-by: Miguel Company <miguelcompany@eprosima.com>

* Refs #18050. Apply suggestions

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

* Refs #18050. improvement regression test

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

* Apply suggestions from code review

Co-authored-by: Miguel Company <miguelcompany@eprosima.com>

* Refs #18051. Apply suggestions.

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

* Refs #18051. Fix mock

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

* Refs #18051. Fix statistics mock

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

* Refs #18051. Fix uncrustify

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

* Refs #18051. Fix memory leak

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

---------

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
Co-authored-by: Miguel Company <miguelcompany@eprosima.com>
(cherry picked from commit 3a168ed)

# Conflicts:
#	src/cpp/rtps/RTPSDomain.cpp
#	src/cpp/rtps/RTPSDomainImpl.hpp
mergify bot pushed a commit that referenced this pull request Jun 27, 2023
@richiware @mergify
Fix segfault when creating two participant with same fixed id (#3443)
c326230 
* Refs #18050. Add regression test

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

* Refs #18050. Fix segfault

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

* Apply suggestions from code review

Co-authored-by: Miguel Company <miguelcompany@eprosima.com>

* Refs #18050. Apply suggestions

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

* Refs #18050. improvement regression test

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

* Apply suggestions from code review

Co-authored-by: Miguel Company <miguelcompany@eprosima.com>

* Refs #18051. Apply suggestions.

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

* Refs #18051. Fix mock

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

* Refs #18051. Fix statistics mock

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

* Refs #18051. Fix uncrustify

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

* Refs #18051. Fix memory leak

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

---------

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>
Co-authored-by: Miguel Company <miguelcompany@eprosima.com>
(cherry picked from commit 3a168ed)

# Conflicts:
#	src/cpp/fastdds/domain/DomainParticipantImpl.cpp
#	src/cpp/rtps/RTPSDomain.cpp
#	src/cpp/rtps/RTPSDomainImpl.hpp
This was referenced Jun 27, 2023
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MiguelCompany MiguelCompany MiguelCompany approved these changes

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.10.2
Development

Successfully merging this pull request may close these issues.
3 participants
@richiware @MiguelCompany @JLBuenoLopez