[20658] Fix leak in `SecurityManager::participant_volatile_message_secure_writer_` #4673

MiguelCompany · 2024-04-09T14:01:59Z

Description

This is a fix for #4553, which reported an increase in memory consumption when the partition of a secure DataWriter is changed. The consumption came from the history of the participant_volatile_message_secure_writer_ not being correctly cleared.

This PR:

adds a black-box test reproducing the behavior depicted in [20658] TopicPayloadPool memory leak problem #4553
converts volatile_writer_ in the security tests into a StrictMock, and adds expectations for the added change to be released
Fixes [20658] TopicPayloadPool memory leak problem #4553 by making SecurityManager implement rtps::WriterListener and removing changes from history when they are acknowledged (i.e. making participant_volatile_message_secure_writer_ truly behave as volatile)

@Mergifyio backport 2.13.x 2.10.x 2.6.x

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.
N/A Any new configuration API has an equivalent XML API (with the corresponding XSD extension)
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.
Applicable backports have been included in the description.

Reviewer Checklist

The PR has a milestone assigned.
The title and description correctly express the PR's purpose.
Check contributor checklist is correct.
Check CI results: changes do not issue any warning.
Check CI results: failing tests are unrelated with the changes.

fujitatomoya

adding the listener of security manager to remove the history change once all readers are acknowledged, lgtm.

thirdparty/fastcdr

EduPonz

Please revert the Fast CDR submodule update

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

MiguelCompany · 2024-04-23T10:23:10Z

Please revert the Fast CDR submodule update

I rebased and removed the submodule update from the commit where it was done

EduPonz · 2024-04-24T05:11:40Z

@MiguelCompany I'm afraid Ubuntu and macOS CIs show failures in BlackboxTests_DDS_PIM.Security/Security.BuiltinAuthenticationAndAccessAndCryptoPlugin_Permissions_validation_toggle_partition. Can you please check it?

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

EduPonz · 2024-04-25T05:00:33Z

@Mergifyio backport 2.13.x 2.10.x 2.6.x

EduPonz · 2024-04-25T05:00:58Z

CI errors unrelated to the PR

mergify · 2024-04-25T05:01:18Z

backport 2.13.x 2.10.x 2.6.x

✅ Backports have been created

#4724 [20658] Fix leak in SecurityManager::participant_volatile_message_secure_writer_ (backport #4673) has been created for branch 2.13.x
#4725 [20658] Fix leak in SecurityManager::participant_volatile_message_secure_writer_ (backport #4673) has been created for branch 2.10.x
#4726 [20658] Fix leak in SecurityManager::participant_volatile_message_secure_writer_ (backport #4673) has been created for branch 2.6.x

…er_ (#4673) * Refs #20658. Add blackbox test. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Refs #20658. Add expectations to unit test. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Refs #20658. Fix issue. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Refs #20658. Improve regression test. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> --------- Signed-off-by: Miguel Company <miguelcompany@eprosima.com> (cherry picked from commit 14ee8ef)

…ter_` (#4673) (#4725) * Fix leak in SecurityManager::participant_volatile_message_secure_writer_ (#4673) * Refs #20658. Add blackbox test. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Refs #20658. Add expectations to unit test. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Refs #20658. Fix issue. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Refs #20658. Improve regression test. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> --------- Signed-off-by: Miguel Company <miguelcompany@eprosima.com> (cherry picked from commit 14ee8ef) * Fix SecurityTest unittests memory error (#4750) Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com> --------- Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com> Co-authored-by: Miguel Company <miguelcompany@eprosima.com> Co-authored-by: Mario Domínguez López <116071334+Mario-DL@users.noreply.github.com>

…ter_` (#4673) (#4724) * Fix leak in SecurityManager::participant_volatile_message_secure_writer_ (#4673) * Refs #20658. Add blackbox test. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Refs #20658. Add expectations to unit test. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Refs #20658. Fix issue. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Refs #20658. Improve regression test. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> --------- Signed-off-by: Miguel Company <miguelcompany@eprosima.com> (cherry picked from commit 14ee8ef) * Fix SecurityTest unittests memory error (#4750) Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com> --------- Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com> Co-authored-by: Miguel Company <miguelcompany@eprosima.com> Co-authored-by: Mario Domínguez López <116071334+Mario-DL@users.noreply.github.com>

…ter_` (#4673) (#4726) * Fix leak in SecurityManager::participant_volatile_message_secure_writer_ (#4673) * Refs #20658. Add blackbox test. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Refs #20658. Add expectations to unit test. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Refs #20658. Fix issue. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Refs #20658. Improve regression test. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> --------- Signed-off-by: Miguel Company <miguelcompany@eprosima.com> (cherry picked from commit 14ee8ef) * Fix SecurityTest unittests memory error (#4750) Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com> --------- Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com> Co-authored-by: Miguel Company <miguelcompany@eprosima.com> Co-authored-by: Mario Domínguez López <116071334+Mario-DL@users.noreply.github.com>

MiguelCompany added this to the v2.14.1 milestone Apr 9, 2024

MiguelCompany mentioned this pull request Apr 9, 2024

[20658] TopicPayloadPool memory leak problem #4553

Closed

1 task

MiguelCompany force-pushed the bugfix/20658 branch from b960714 to 3112b9b Compare April 9, 2024 14:06

fujitatomoya approved these changes Apr 15, 2024

View reviewed changes

thirdparty/fastcdr Outdated Show resolved Hide resolved

EduPonz suggested changes Apr 23, 2024

View reviewed changes

MiguelCompany added 3 commits April 23, 2024 12:21

Refs #20658. Add blackbox test.

90dca12

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

Refs #20658. Add expectations to unit test.

1103492

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

Refs #20658. Fix issue.

5492ac7

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

MiguelCompany force-pushed the bugfix/20658 branch from 3112b9b to 5492ac7 Compare April 23, 2024 10:22

EduPonz self-requested a review April 23, 2024 10:27

github-actions bot added the ci-pending PR which CI is running label Apr 23, 2024

EduPonz added to-do and removed ci-pending PR which CI is running labels Apr 24, 2024

Refs #20658. Improve regression test.

118ae0a

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

EduPonz removed the to-do label Apr 24, 2024

EduPonz requested review from EduPonz and removed request for EduPonz April 24, 2024 08:56

EduPonz approved these changes Apr 24, 2024

View reviewed changes

github-actions bot added the ci-pending PR which CI is running label Apr 24, 2024

EduPonz self-requested a review April 24, 2024 13:40

EduPonz merged commit 14ee8ef into master Apr 25, 2024
10 of 21 checks passed

EduPonz deleted the bugfix/20658 branch April 25, 2024 05:01

mergify bot mentioned this pull request Apr 25, 2024

[20658] Fix leak in SecurityManager::participant_volatile_message_secure_writer_ (backport #4673) #4724

Merged

12 tasks

This was referenced Apr 25, 2024

[20658] Fix leak in SecurityManager::participant_volatile_message_secure_writer_ (backport #4673) #4725

Merged

[20658] Fix leak in SecurityManager::participant_volatile_message_secure_writer_ (backport #4673) #4726

Merged

Mario-DL mentioned this pull request Apr 30, 2024

[20945] Fix SecurityTest unittests memory error #4750

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[20658] Fix leak in `SecurityManager::participant_volatile_message_secure_writer_` #4673

[20658] Fix leak in `SecurityManager::participant_volatile_message_secure_writer_` #4673

MiguelCompany commented Apr 9, 2024 •

edited by EduPonz

Loading

fujitatomoya left a comment

EduPonz left a comment

MiguelCompany commented Apr 23, 2024

EduPonz commented Apr 24, 2024

EduPonz commented Apr 25, 2024

EduPonz commented Apr 25, 2024

mergify bot commented Apr 25, 2024 •

edited

Loading

[20658] Fix leak in SecurityManager::participant_volatile_message_secure_writer_ #4673

[20658] Fix leak in SecurityManager::participant_volatile_message_secure_writer_ #4673

Conversation

MiguelCompany commented Apr 9, 2024 • edited by EduPonz Loading

Description

Contributor Checklist

Reviewer Checklist

fujitatomoya left a comment

Choose a reason for hiding this comment

EduPonz left a comment

Choose a reason for hiding this comment

MiguelCompany commented Apr 23, 2024

EduPonz commented Apr 24, 2024

EduPonz commented Apr 25, 2024

EduPonz commented Apr 25, 2024

mergify bot commented Apr 25, 2024 • edited Loading

✅ Backports have been created

[20658] Fix leak in `SecurityManager::participant_volatile_message_secure_writer_` #4673

[20658] Fix leak in `SecurityManager::participant_volatile_message_secure_writer_` #4673

MiguelCompany commented Apr 9, 2024 •

edited by EduPonz

Loading

mergify bot commented Apr 25, 2024 •

edited

Loading