ObjectPool: Use Platform::New and rename modes (#13088)

#### Problem

- Heap allocations should use `Platform::New` rather than raw `new`.
- `ObjectPoolMem::kStatic` could be misleading, since the pool memory is
  part of the enclosing context, not static unless also declared `static`.

#### Change overview

- Use `Platform::New` and `Platform::Delete`.
- Rename `ObjectPoolMem::kStatic` → `kInline` and `kDynamic` → `kHeap`.

#### Testing

CI

Author: kpschoedel

src/lib/dnssd/minimal_mdns/Server.h

@@ -228,7 +228,7 @@ class ServerBase
 
 // The PoolImpl impl is used as a base class because its destructor must be called after ServerBase's destructor.
 template <size_t kCount>
-class Server : private chip::PoolImpl<ServerBase::EndpointInfo, kCount, chip::ObjectPoolMem::kStatic,
+class Server : private chip::PoolImpl<ServerBase::EndpointInfo, kCount, chip::ObjectPoolMem::kInline,
                                       ServerBase::EndpointInfoPoolType::Interface>,
                public ServerBase
 {

src/lib/dnssd/minimal_mdns/tests/CheckOnlyServer.h

@@ -71,7 +71,7 @@ void MakePrintableName(char (&location)[N], FullQName name)
 
 } // namespace
 
-class CheckOnlyServer : private chip::PoolImpl<ServerBase::EndpointInfo, 0, chip::ObjectPoolMem::kStatic,
+class CheckOnlyServer : private chip::PoolImpl<ServerBase::EndpointInfo, 0, chip::ObjectPoolMem::kInline,
                                                ServerBase::EndpointInfoPoolType::Interface>,
                         public ServerBase,
                         public ParserDelegate,

src/lib/support/Pool.cpp

@@ -151,7 +151,7 @@ Loop HeapObjectList::ForEachNode(void * context, Lambda lambda)
             if (p->mObject == nullptr)
             {
                 p->Remove();
-                delete p;
+                Platform::Delete(p);
             }
             p = next;
         }

src/lib/support/Pool.h

@@ -22,6 +22,7 @@
 
 #pragma once
 
+#include <lib/support/CHIPMem.h>
 #include <lib/support/CodeUtils.h>
 #include <system/SystemConfig.h>
 
@@ -276,10 +277,10 @@ class HeapObjectPool : public internal::Statistics, public internal::PoolCommon<
     template <typename... Args>
     T * CreateObject(Args &&... args)
     {
-        T * object = new T(std::forward<Args>(args)...);
+        T * object = Platform::New<T>(std::forward<Args>(args)...);
         if (object != nullptr)
         {
-            auto node = new internal::HeapObjectListNode();
+            auto node = Platform::New<internal::HeapObjectListNode>();
             if (node != nullptr)
             {
                 node->mObject = object;
@@ -300,7 +301,7 @@ class HeapObjectPool : public internal::Statistics, public internal::PoolCommon<
             {
                 // Note that the node is not removed here; that is deferred until the end of the next pool iteration.
                 node->mObject = nullptr;
-                delete object;
+                Platform::Delete(object);
                 DecreaseUsage();
             }
         }
@@ -336,28 +337,39 @@ class HeapObjectPool : public internal::Statistics, public internal::PoolCommon<
 
 #endif // CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
 
+/**
+ * Specify ObjectPool storage allocation.
+ */
 enum class ObjectPoolMem
 {
-    kStatic,
+    /**
+     * Use storage inside the containing scope for both objects and pool management state.
+     */
+    kInline,
 #if CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
-    kDynamic,
-    kDefault = kDynamic
+    /**
+     * Allocate objects from the heap, with only pool management state in the containing scope.
+     *
+     * For this case, the ObjectPool size parameter is ignored.
+     */
+    kHeap,
+    kDefault = kHeap
 #else  // CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
-    kDefault = kStatic
+    kDefault = kInline
 #endif // CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
 };
 
 template <typename T, size_t N, ObjectPoolMem P = ObjectPoolMem::kDefault>
 class ObjectPool;
 
 template <typename T, size_t N>
-class ObjectPool<T, N, ObjectPoolMem::kStatic> : public BitMapObjectPool<T, N>
+class ObjectPool<T, N, ObjectPoolMem::kInline> : public BitMapObjectPool<T, N>
 {
 };
 
 #if CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
 template <typename T, size_t N>
-class ObjectPool<T, N, ObjectPoolMem::kDynamic> : public HeapObjectPool<T>
+class ObjectPool<T, N, ObjectPoolMem::kHeap> : public HeapObjectPool<T>
 {
 };
 #endif // CHIP_SYSTEM_CONFIG_POOL_USE_HEAP

src/lib/support/tests/TestPool.cpp

@@ -62,20 +62,20 @@ void TestReleaseNull(nlTestSuite * inSuite, void * inContext)
 
 void TestReleaseNullStatic(nlTestSuite * inSuite, void * inContext)
 {
-    TestReleaseNull<uint32_t, 10, ObjectPoolMem::kStatic>(inSuite, inContext);
+    TestReleaseNull<uint32_t, 10, ObjectPoolMem::kInline>(inSuite, inContext);
 }
 
 #if CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
 void TestReleaseNullDynamic(nlTestSuite * inSuite, void * inContext)
 {
-    TestReleaseNull<uint32_t, 10, ObjectPoolMem::kDynamic>(inSuite, inContext);
+    TestReleaseNull<uint32_t, 10, ObjectPoolMem::kHeap>(inSuite, inContext);
 }
 #endif // CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
 
 template <typename T, size_t N, ObjectPoolMem P>
 void TestCreateReleaseObject(nlTestSuite * inSuite, void * inContext)
 {
-    ObjectPool<uint32_t, N, ObjectPoolMem::kStatic> pool;
+    ObjectPool<uint32_t, N, ObjectPoolMem::kInline> pool;
     uint32_t * obj[N];
 
     NL_TEST_ASSERT(inSuite, pool.Allocated() == 0);
@@ -104,9 +104,9 @@ void TestCreateReleaseObject(nlTestSuite * inSuite, void * inContext)
 void TestCreateReleaseObjectStatic(nlTestSuite * inSuite, void * inContext)
 {
     constexpr const size_t kSize = 100;
-    TestCreateReleaseObject<uint32_t, kSize, ObjectPoolMem::kStatic>(inSuite, inContext);
+    TestCreateReleaseObject<uint32_t, kSize, ObjectPoolMem::kInline>(inSuite, inContext);
 
-    ObjectPool<uint32_t, kSize, ObjectPoolMem::kStatic> pool;
+    ObjectPool<uint32_t, kSize, ObjectPoolMem::kInline> pool;
     uint32_t * obj[kSize];
 
     for (size_t i = 0; i < kSize; ++i)
@@ -144,7 +144,7 @@ void TestCreateReleaseObjectStatic(nlTestSuite * inSuite, void * inContext)
 #if CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
 void TestCreateReleaseObjectDynamic(nlTestSuite * inSuite, void * inContext)
 {
-    TestCreateReleaseObject<uint32_t, 100, ObjectPoolMem::kDynamic>(inSuite, inContext);
+    TestCreateReleaseObject<uint32_t, 100, ObjectPoolMem::kHeap>(inSuite, inContext);
 }
 #endif // CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
 
@@ -201,13 +201,13 @@ void TestCreateReleaseStruct(nlTestSuite * inSuite, void * inContext)
 
 void TestCreateReleaseStructStatic(nlTestSuite * inSuite, void * inContext)
 {
-    TestCreateReleaseStruct<ObjectPoolMem::kStatic>(inSuite, inContext);
+    TestCreateReleaseStruct<ObjectPoolMem::kInline>(inSuite, inContext);
 }
 
 #if CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
 void TestCreateReleaseStructDynamic(nlTestSuite * inSuite, void * inContext)
 {
-    TestCreateReleaseStruct<ObjectPoolMem::kDynamic>(inSuite, inContext);
+    TestCreateReleaseStruct<ObjectPoolMem::kHeap>(inSuite, inContext);
 }
 #endif // CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
 
@@ -334,13 +334,13 @@ void TestForEachActiveObject(nlTestSuite * inSuite, void * inContext)
 
 void TestForEachActiveObjectStatic(nlTestSuite * inSuite, void * inContext)
 {
-    TestForEachActiveObject<ObjectPoolMem::kStatic>(inSuite, inContext);
+    TestForEachActiveObject<ObjectPoolMem::kInline>(inSuite, inContext);
 }
 
 #if CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
 void TestForEachActiveObjectDynamic(nlTestSuite * inSuite, void * inContext)
 {
-    TestForEachActiveObject<ObjectPoolMem::kDynamic>(inSuite, inContext);
+    TestForEachActiveObject<ObjectPoolMem::kHeap>(inSuite, inContext);
 }
 #endif // CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
 
@@ -397,23 +397,24 @@ void TestPoolInterface(nlTestSuite * inSuite, void * inContext)
 
 void TestPoolInterfaceStatic(nlTestSuite * inSuite, void * inContext)
 {
-    TestPoolInterface<ObjectPoolMem::kStatic>(inSuite, inContext);
+    TestPoolInterface<ObjectPoolMem::kInline>(inSuite, inContext);
 }
 
 #if CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
 void TestPoolInterfaceDynamic(nlTestSuite * inSuite, void * inContext)
 {
-    TestPoolInterface<ObjectPoolMem::kDynamic>(inSuite, inContext);
+    TestPoolInterface<ObjectPoolMem::kHeap>(inSuite, inContext);
 }
 #endif // CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
 
 int Setup(void * inContext)
 {
-    return SUCCESS;
+    return ::chip::Platform::MemoryInit() == CHIP_NO_ERROR ? SUCCESS : FAILURE;
 }
 
 int Teardown(void * inContext)
 {
+    ::chip::Platform::MemoryShutdown();
     return SUCCESS;
 }
 

src/system/tests/TestSystemTimer.cpp

@@ -442,6 +442,11 @@ static int TestSetup(void * aContext)
 {
     TestContext & lContext = *reinterpret_cast<TestContext *>(aContext);
 
+    if (::chip::Platform::MemoryInit() != CHIP_NO_ERROR)
+    {
+        return FAILURE;
+    }
+
 #if CHIP_SYSTEM_CONFIG_USE_LWIP && LWIP_VERSION_MAJOR == 2 && LWIP_VERSION_MINOR == 0
     static sys_mbox_t * sLwIPEventQueue = NULL;
 
@@ -471,6 +476,7 @@ static int TestTeardown(void * aContext)
     tcpip_finish(NULL, NULL);
 #endif // CHIP_SYSTEM_CONFIG_USE_LWIP && (LWIP_VERSION_MAJOR == 2) && (LWIP_VERSION_MINOR == 0)
 
+    ::chip::Platform::MemoryShutdown();
     return (SUCCESS);
 }
 

src/transport/raw/TCP.h

@@ -281,7 +281,7 @@ class TCP : public TCPBase
 private:
     friend class TCPTest;
     TCPBase::ActiveConnectionState mConnectionsBuffer[kActiveConnectionsSize];
-    PoolImpl<PendingPacket, kPendingPacketSize, ObjectPoolMem::kStatic, PendingPacketPoolType::Interface> mPendingPackets;
+    PoolImpl<PendingPacket, kPendingPacketSize, ObjectPoolMem::kInline, PendingPacketPoolType::Interface> mPendingPackets;
 };
 
 } // namespace Transport