From 274719d61fba791632a0c8f710e158ac3c3e22fd Mon Sep 17 00:00:00 2001
From: Andrei Litvin <andy314@gmail.com>
Date: Wed, 14 Feb 2024 18:40:56 -0500
Subject: [PATCH] Remove PoolCommon::ResetObject (#32120)

* Remove PoolCommon: bad name, non-member method, very very limited usage

* Restyle

* Added comment about returning null on failure

* Fix compile on fixed size pools

* Restyle

* Switch to not use a cast

* Fix types ... use auto because types are messy, unsure why

* Rename for smaller diff

* Rename for smaller diff

* Update src/transport/UnauthenticatedSessionTable.h

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

---------

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
---
 src/lib/support/Pool.h                      | 16 ++--------------
 src/lib/support/PoolWrapper.h               | 12 +++---------
 src/transport/UnauthenticatedSessionTable.h | 17 +++++++++++++----
 3 files changed, 18 insertions(+), 27 deletions(-)

diff --git a/src/lib/support/Pool.h b/src/lib/support/Pool.h
index c2486f0c0e20b3..a3959fd1e868f9 100644
--- a/src/lib/support/Pool.h
+++ b/src/lib/support/Pool.h
@@ -104,18 +104,6 @@ class StaticAllocatorBitmap : public internal::StaticAllocatorBase
     std::atomic<tBitChunkType> * mUsage;
 };
 
-template <class T>
-class PoolCommon
-{
-public:
-    template <typename... Args>
-    void ResetObject(T * element, Args &&... args)
-    {
-        element->~T();
-        new (element) T(std::forward<Args>(args)...);
-    }
-};
-
 template <typename T, typename Function>
 class LambdaProxy
 {
@@ -211,7 +199,7 @@ struct HeapObjectList : HeapObjectListNode
  *  @tparam     N   a positive integer max number of elements the pool provides.
  */
 template <class T, size_t N>
-class BitMapObjectPool : public internal::StaticAllocatorBitmap, public internal::PoolCommon<T>
+class BitMapObjectPool : public internal::StaticAllocatorBitmap
 {
 public:
     BitMapObjectPool() : StaticAllocatorBitmap(mData.mMemory, mUsage, N, sizeof(T)) {}
@@ -314,7 +302,7 @@ class HeapObjectPoolExitHandling
  *  @tparam     T   type to be allocated.
  */
 template <class T>
-class HeapObjectPool : public internal::Statistics, public internal::PoolCommon<T>, public HeapObjectPoolExitHandling
+class HeapObjectPool : public internal::Statistics, public HeapObjectPoolExitHandling
 {
 public:
     HeapObjectPool() {}
diff --git a/src/lib/support/PoolWrapper.h b/src/lib/support/PoolWrapper.h
index 78b6d838245633..fd9972429d9661 100644
--- a/src/lib/support/PoolWrapper.h
+++ b/src/lib/support/PoolWrapper.h
@@ -33,10 +33,9 @@ class PoolInterface
 
     virtual ~PoolInterface() {}
 
-    virtual U * CreateObject(ConstructorArguments... args)              = 0;
-    virtual void ReleaseObject(U * element)                             = 0;
-    virtual void ReleaseAll()                                           = 0;
-    virtual void ResetObject(U * element, ConstructorArguments... args) = 0;
+    virtual U * CreateObject(ConstructorArguments... args) = 0;
+    virtual void ReleaseObject(U * element)                = 0;
+    virtual void ReleaseAll()                              = 0;
 
     template <typename Function>
     Loop ForEachActiveObject(Function && function)
@@ -82,11 +81,6 @@ class PoolProxy<T, N, M, std::tuple<U, ConstructorArguments...>> : public PoolIn
 
     void ReleaseAll() override { Impl().ReleaseAll(); }
 
-    void ResetObject(U * element, ConstructorArguments... args) override
-    {
-        return Impl().ResetObject(static_cast<T *>(element), std::move(args)...);
-    }
-
 protected:
     Loop ForEachActiveObjectInner(void * context, typename PoolInterface<U, ConstructorArguments...>::Lambda lambda) override
     {
diff --git a/src/transport/UnauthenticatedSessionTable.h b/src/transport/UnauthenticatedSessionTable.h
index 48e2025d10a487..f5ba35df533de5 100644
--- a/src/transport/UnauthenticatedSessionTable.h
+++ b/src/transport/UnauthenticatedSessionTable.h
@@ -285,17 +285,26 @@ class UnauthenticatedSessionTable
             return CHIP_NO_ERROR;
         }
 
-#if !CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
+#if CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
+        // permanent failure if heap was insufficient
+        return CHIP_ERROR_NO_MEMORY;
+#else
         entryToUse = FindLeastRecentUsedEntry();
-#endif // CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
+        VerifyOrReturnError(entryToUse != nullptr, CHIP_ERROR_NO_MEMORY);
+
+        // Drop the least recent entry to allow for a new alloc.
+        mEntries.ReleaseObject(entryToUse);
+        entryToUse = mEntries.CreateObject(sessionRole, ephemeralInitiatorNodeID, config, *this);
+
         if (entryToUse == nullptr)
         {
-            return CHIP_ERROR_NO_MEMORY;
+            // this is NOT expected: we freed an object to have space
+            return CHIP_ERROR_INTERNAL;
         }
 
-        mEntries.ResetObject(entryToUse, sessionRole, ephemeralInitiatorNodeID, config, *this);
         entry = entryToUse;
         return CHIP_NO_ERROR;
+#endif // CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
     }
 
     CHECK_RETURN_VALUE UnauthenticatedSession * FindEntry(UnauthenticatedSession::SessionRole sessionRole,