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.

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

Incorrect payload pool release in EDPSimple destructor [13114] #2333

Closed
ghost opened this issue Nov 30, 2021 · 2 comments · Fixed by #2335
Closed

Incorrect payload pool release in EDPSimple destructor [13114] #2333

ghost opened this issue Nov 30, 2021 · 2 comments · Fixed by #2335

Comments

@ghost
Copy link

ghost commented Nov 30, 2021

There is a bug in EDPSimple::~EDPSimple() which attempts to release more memory than it was allocated:

EDPUtils::release_payload_pool(sec_pub_reader_payload_pool_, publications_secure_writer_.second->m_att, true);

Such behavior leads to integer overflow in PreallocatedReallocTopicPayloadPool::release_history():

    bool release_history(
            const PoolConfig& config,
            bool is_reader) override
    {
        {
            std::lock_guard<std::mutex> lock(mutex_);
            minimum_pool_size_ -= config.initial_size;
        }

        return TopicPayloadPool::release_history(config, is_reader);
    }

Next time, on participant creation, the payload pool tries to allocate gigantic amount of memory and gets killed by the OS.

System information

  • Fast-RTPS version: 2.4.1
  • OS: Ubuntu 20.04

Additional context

This code change helps to solve the issue:

diff --git a/src/cpp/rtps/builtin/discovery/endpoint/EDPSimple.cpp b/src/cpp/rtps/builtin/discovery/endpoint/EDPSimple.cpp
index 294310a61..2000eb4a8 100644
--- a/src/cpp/rtps/builtin/discovery/endpoint/EDPSimple.cpp
+++ b/src/cpp/rtps/builtin/discovery/endpoint/EDPSimple.cpp
@@ -82,7 +82,7 @@ EDPSimple::~EDPSimple()
     if (this->publications_secure_reader_.first != nullptr)
     {
         this->mp_RTPSParticipant->deleteUserEndpoint(publications_secure_reader_.first);
-        EDPUtils::release_payload_pool(sec_pub_reader_payload_pool_, publications_secure_writer_.second->m_att, true);
+        EDPUtils::release_payload_pool(sec_pub_reader_payload_pool_, publications_secure_reader_.second->m_att, true);
         delete(publications_secure_reader_.second);
     }
 
@MiguelCompany MiguelCompany changed the title Incorrect payload pool release in EDPSimple destructor. Incorrect payload pool release in EDPSimple destructor [13114] Dec 1, 2021
@MiguelCompany
Copy link
Member

@oleksandr-hryshchuk Thanks for the catch! I introduced your fix, plus a refactor to avoid copy-paste mistakes in the future, on #2335.

Please check on your side.

@ghost
Copy link
Author

ghost commented Dec 1, 2021

@MiguelCompany, your fix works for me, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant