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>
project-chip · Feb 14, 2024 · 274719d · 274719d
1 parent 2619da7
commit 274719d
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 27 deletions.
diff --git 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
@@ -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
@@ -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,