Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion src/cpp/rtps/history/TopicPayloadPool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,14 @@ TopicPayloadPool::PayloadNode* TopicPayloadPool::allocate(
TopicPayloadPool::PayloadNode* TopicPayloadPool::do_allocate(
uint32_t size)
{
PayloadNode* payload = new (std::nothrow) PayloadNode(size);
PayloadNode* payload = nullptr;
try
{
payload = new PayloadNode(size);
}
catch (const std::bad_alloc&)
{
}

if (payload != nullptr)
{
Expand Down
2 changes: 1 addition & 1 deletion src/cpp/rtps/history/TopicPayloadPool.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ class TopicPayloadPool : public ITopicPayloadPool
}
else
{
buffer = (octet*)calloc(size + sizeof(NodeInfo) - 1, sizeof(octet));
buffer = (octet*)calloc(size + data_offset, sizeof(octet));
}

if (buffer == nullptr)
Expand Down
44 changes: 44 additions & 0 deletions test/unittest/rtps/history/TopicPayloadPoolTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include <rtps/history/TopicPayloadPool.hpp>
#include <fastdds/rtps/common/CacheChange.hpp>

#include <limits>
#include <tuple>

using namespace eprosima::fastdds::rtps;
Expand Down Expand Up @@ -483,6 +484,49 @@ TEST(TopicPayloalPoolTests, dynamic_reusable_memory_zero_size)
do_dynamic_topic_payload_pool_zero_size_test(config);
}

//! Regression test for redmine issue #25475
//! When the allocation of a new payload node fails, the pool must report the
//! failure gracefully (returning false) instead of letting the std::bad_alloc
//! thrown by the PayloadNode constructor propagate out of get_payload.
//! Might require to be run in a constrained environment (i.e. container with limited memory)
//! to actually trigger the allocation failure, but will not fail if the allocation succeeds.
TEST(TopicPayloalPoolTests, bad_alloc_on_get_payload_is_handled)
{
// An infinite history so that the pool is allowed to allocate on demand
// (otherwise allocate() would short-circuit on the maximum pool size).
PoolConfig config{ DYNAMIC_RESERVE_MEMORY_MODE, 128, 0, 0 };

std::unique_ptr<ITopicPayloadPool> pool = TopicPayloadPool::get(config);
ASSERT_NE(pool, nullptr);
pool->reserve_history(config, false);

// Request a payload so large that the underlying calloc is expected to fail,
// making the PayloadNode constructor throw std::bad_alloc.
constexpr uint32_t huge_size = (std::numeric_limits<uint32_t>::max)();

CacheChange_t change;
bool result = true;

// The pool must never let the exception escape.
EXPECT_NO_THROW(result = pool->get_payload(huge_size, change.serializedPayload));

if (result)
{
// On a platform able to satisfy such a large allocation, release it.
EXPECT_EQ(huge_size, change.serializedPayload.max_size);
pool->release_payload(change.serializedPayload);
}
else
{
// On allocation failure the payload must be left in a clean state.
EXPECT_EQ(change.serializedPayload.data, nullptr);
EXPECT_EQ(change.serializedPayload.max_size, 0u);
EXPECT_EQ(change.serializedPayload.payload_owner, nullptr);
}

pool->release_history(config, false);
}

#ifdef INSTANTIATE_TEST_SUITE_P
#define GTEST_INSTANTIATE_TEST_MACRO(x, y, z) INSTANTIATE_TEST_SUITE_P(x, y, z)
#else
Expand Down
Loading