Catch exception when constructing a payload pool Node (#6460) (#6462)

* Refs #25475. Regression test.



* Refs #25475. Capture exception when constructing PayloadNode.



* Refs #25475. Bonus: avoid using sizeof(NodeInfo).



* Refs #25475. Fix uncrustify.



---------


(cherry picked from commit 31ece6f)

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

Author: mergify[bot]

src/cpp/rtps/history/TopicPayloadPool.cpp

@@ -189,7 +189,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)
     {

src/cpp/rtps/history/TopicPayloadPool.hpp

@@ -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)

test/unittest/rtps/history/TopicPayloadPoolTests.cpp

@@ -16,6 +16,7 @@
 
 #include <rtps/history/TopicPayloadPool.hpp>
 
+#include <limits>
 #include <tuple>
 
 using namespace eprosima::fastrtps::rtps;
@@ -488,6 +489,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));
+
+    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);
+    }
+    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.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