Catch or release ObjectPool leaks

src/app/CASESessionManager.h

@@ -103,7 +103,9 @@ class CASESessionManager : public SessionReleaseDelegate, public Dnssd::Resolver
     OperationalDeviceProxy * FindSession(SessionHandle session);
     void ReleaseSession(OperationalDeviceProxy * device);
 
-    BitMapObjectPool<OperationalDeviceProxy, CHIP_CONFIG_CONTROLLER_MAX_ACTIVE_DEVICES> mActiveSessions;
+    BitMapObjectPool<OperationalDeviceProxy, CHIP_CONFIG_CONTROLLER_MAX_ACTIVE_DEVICES,
+                     OnObjectPoolDestruction::IgnoreUnsafeDoNotUseInNewCode>
+        mActiveSessions;
 
     CASESessionManagerConfig mConfig;
 };

src/app/InteractionModelEngine.h

@@ -280,7 +280,8 @@ class InteractionModelEngine : public Messaging::ExchangeDelegate, public Comman
     // TODO(#8006): investgate if we can disable some IM functions on some compact accessories.
     // TODO(#8006): investgate if we can provide more flexible object management on devices with more resources.
     BitMapObjectPool<CommandHandler, CHIP_IM_MAX_NUM_COMMAND_HANDLER> mCommandHandlerObjs;
-    BitMapObjectPool<TimedHandler, CHIP_IM_MAX_NUM_TIMED_HANDLER> mTimedHandlers;
+    BitMapObjectPool<TimedHandler, CHIP_IM_MAX_NUM_TIMED_HANDLER, OnObjectPoolDestruction::IgnoreUnsafeDoNotUseInNewCode>
+        mTimedHandlers;
     ReadClient mReadClients[CHIP_IM_MAX_NUM_READ_CLIENT];
     ReadHandler mReadHandlers[CHIP_IM_MAX_NUM_READ_HANDLER];
     WriteClient mWriteClients[CHIP_IM_MAX_NUM_WRITE_CLIENT];

src/controller/CHIPDeviceControllerFactory.cpp

@@ -223,6 +223,15 @@ CHIP_ERROR DeviceControllerSystemState::Shutdown()
     // Shut down the interaction model
     app::InteractionModelEngine::GetInstance()->Shutdown();
 
+    // Shut down the TransportMgr. This holds Inet::UDPEndPoints so it must be shut down
+    // before PlatformMgr().Shutdown() shuts down Inet.
+    if (mTransportMgr != nullptr)
+    {
+        mTransportMgr->Close();
+        chip::Platform::Delete(mTransportMgr);
+        mTransportMgr = nullptr;
+    }
+
 #if CONFIG_DEVICE_LAYER
     //
     // We can safely call PlatformMgr().Shutdown(), which like DeviceController::Shutdown(),
@@ -247,11 +256,6 @@ CHIP_ERROR DeviceControllerSystemState::Shutdown()
 
     mSystemLayer = nullptr;
     mInetLayer   = nullptr;
-    if (mTransportMgr != nullptr)
-    {
-        chip::Platform::Delete(mTransportMgr);
-        mTransportMgr = nullptr;
-    }
 
     if (mMessageCounterManager != nullptr)
     {

src/credentials/GroupDataProviderImpl.h

@@ -138,7 +138,7 @@ class GroupDataProviderImpl : public GroupDataProvider
     BitMapObjectPool<GroupMappingIteratorImpl, kIteratorsMax> mEndpointIterators;
     BitMapObjectPool<AllStatesIterator, kIteratorsMax> mAllStatesIterators;
     BitMapObjectPool<FabricStatesIterator, kIteratorsMax> mFabricStatesIterators;
-    BitMapObjectPool<KeySetIteratorImpl, kIteratorsMax> mKeySetIterators;
+    BitMapObjectPool<KeySetIteratorImpl, kIteratorsMax, OnObjectPoolDestruction::IgnoreUnsafeDoNotUseInNewCode> mKeySetIterators;
 };
 
 } // namespace Credentials

src/lib/support/Pool.h

@@ -22,6 +22,7 @@
 
 #pragma once
 
+#include <lib/support/CodeUtils.h>
 #include <system/SystemConfig.h>
 
 #include <atomic>
@@ -166,6 +167,16 @@ struct HeapObjectList : HeapObjectListNode
 
 } // namespace internal
 
+/**
+ * Action taken if objects remain allocated when a pool is destroyed.
+ */
+enum class OnObjectPoolDestruction
+{
+    AutoRelease,                   ///< Release any objects still allocated.
+    Die,                           ///< Abort if any objects remain allocated.
+    IgnoreUnsafeDoNotUseInNewCode, ///< Do nothing; keep historical behaviour until leaks are fixed.
+};
+
 /**
  * @class ObjectPool
  *
@@ -198,14 +209,24 @@ struct HeapObjectList : HeapObjectListNode
  *  @tparam     T   type of element to be allocated.
  *  @tparam     N   a positive integer max number of elements the pool provides.
  */
-template <class T, size_t N>
+template <class T, size_t N, OnObjectPoolDestruction Action = OnObjectPoolDestruction::Die>
 class BitMapObjectPool : public internal::StaticAllocatorBitmap, public internal::PoolCommon<T>
 {
 public:
     BitMapObjectPool() : StaticAllocatorBitmap(mData.mMemory, mUsage, N, sizeof(T)) {}
     ~BitMapObjectPool()
     {
-        // ReleaseAll();
+        switch (Action)
+        {
+        case OnObjectPoolDestruction::AutoRelease:
+            ReleaseAll();
+            break;
+        case OnObjectPoolDestruction::Die:
+            VerifyOrDie(Allocated() == 0);
+            break;
+        case OnObjectPoolDestruction::IgnoreUnsafeDoNotUseInNewCode:
+            break;
+        }
     }
 
     template <typename... Args>
@@ -273,15 +294,24 @@ class BitMapObjectPool : public internal::StaticAllocatorBitmap, public internal
  *
  *  @tparam     T   type to be allocated.
  */
-template <class T>
+template <class T, OnObjectPoolDestruction Action = OnObjectPoolDestruction::Die>
 class HeapObjectPool : public internal::Statistics, public internal::PoolCommon<T>
 {
 public:
     HeapObjectPool() {}
     ~HeapObjectPool()
     {
-        // TODO(#11880): Release all active objects (or verify that none are active) when destroying the pool.
-        // ReleaseAll();
+        switch (Action)
+        {
+        case OnObjectPoolDestruction::AutoRelease:
+            ReleaseAll();
+            break;
+        case OnObjectPoolDestruction::Die:
+            VerifyOrDie(Allocated() == 0);
+            break;
+        case OnObjectPoolDestruction::IgnoreUnsafeDoNotUseInNewCode:
+            break;
+        }
     }
 
     template <typename... Args>
@@ -348,11 +378,11 @@ class HeapObjectPool : public internal::Statistics, public internal::PoolCommon<
 #endif // CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
 
 #if CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
-template <typename T, unsigned int N>
-using ObjectPool = HeapObjectPool<T>;
+template <typename T, unsigned int N, OnObjectPoolDestruction Action = OnObjectPoolDestruction::Die>
+using ObjectPool = HeapObjectPool<T, Action>;
 #else  // CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
-template <typename T, unsigned int N>
-using ObjectPool = BitMapObjectPool<T, N>;
+template <typename T, unsigned int N, OnObjectPoolDestruction Action = OnObjectPoolDestruction::Die>
+using ObjectPool = BitMapObjectPool<T, N, Action>;
 #endif // CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
 
 enum class ObjectPoolMem
@@ -363,17 +393,17 @@ enum class ObjectPoolMem
 #endif // CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
 };
 
-template <typename T, size_t N, ObjectPoolMem P>
+template <typename T, size_t N, ObjectPoolMem P, OnObjectPoolDestruction Action = OnObjectPoolDestruction::Die>
 class MemTypeObjectPool;
 
-template <typename T, size_t N>
-class MemTypeObjectPool<T, N, ObjectPoolMem::kStatic> : public BitMapObjectPool<T, N>
+template <typename T, size_t N, OnObjectPoolDestruction Action>
+class MemTypeObjectPool<T, N, ObjectPoolMem::kStatic, Action> : public BitMapObjectPool<T, N, Action>
 {
 };
 
 #if CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
-template <typename T, size_t N>
-class MemTypeObjectPool<T, N, ObjectPoolMem::kDynamic> : public HeapObjectPool<T>
+template <typename T, size_t N, OnObjectPoolDestruction Action>
+class MemTypeObjectPool<T, N, ObjectPoolMem::kDynamic, Action> : public HeapObjectPool<T, Action>
 {
 };
 #endif // CHIP_SYSTEM_CONFIG_POOL_USE_HEAP

src/lib/support/PoolWrapper.h

@@ -52,11 +52,11 @@ class PoolInterface
     virtual Loop ForEachActiveObjectInner(void * context, Lambda lambda) = 0;
 };
 
-template <class T, size_t N, typename Interface>
+template <class T, size_t N, OnObjectPoolDestruction A, typename Interface>
 class PoolProxy;
 
-template <class T, size_t N, typename U, typename... ConstructorArguments>
-class PoolProxy<T, N, std::tuple<U, ConstructorArguments...>> : public PoolInterface<U, ConstructorArguments...>
+template <class T, size_t N, OnObjectPoolDestruction A, typename U, typename... ConstructorArguments>
+class PoolProxy<T, N, A, std::tuple<U, ConstructorArguments...>> : public PoolInterface<U, ConstructorArguments...>
 {
 public:
     static_assert(std::is_base_of<U, T>::value, "Interface type is not derived from Pool type");
@@ -83,7 +83,7 @@ class PoolProxy<T, N, std::tuple<U, ConstructorArguments...>> : public PoolInter
         return Impl().ForEachActiveObject([&](T * target) { return lambda(context, static_cast<U *>(target)); });
     }
 
-    virtual BitMapObjectPool<T, N> & Impl() = 0;
+    virtual BitMapObjectPool<T, N, A> & Impl() = 0;
 };
 
 /*
@@ -97,18 +97,18 @@ class PoolProxy<T, N, std::tuple<U, ConstructorArguments...>> : public PoolInter
  *                     PoolInterface<U, ConstructorArguments...>, the PoolImpl can be converted to the interface type
  *                     and passed around
  */
-template <class T, size_t N, typename... Interfaces>
-class PoolImpl : public PoolProxy<T, N, Interfaces>...
+template <class T, size_t N, OnObjectPoolDestruction A, typename... Interfaces>
+class PoolImpl : public PoolProxy<T, N, A, Interfaces>...
 {
 public:
     PoolImpl() {}
     virtual ~PoolImpl() override {}
 
 protected:
-    virtual BitMapObjectPool<T, N> & Impl() override { return mImpl; }
+    virtual BitMapObjectPool<T, N, A> & Impl() override { return mImpl; }
 
 private:
-    BitMapObjectPool<T, N> mImpl;
+    BitMapObjectPool<T, N, A> mImpl;
 };
 
 } // namespace chip

src/lib/support/tests/TestPool.cpp

@@ -136,6 +136,8 @@ void TestCreateReleaseObjectStatic(nlTestSuite * inSuite, void * inContext)
     NL_TEST_ASSERT(inSuite, GetNumObjectsInUse(pool) == kSize);
     NL_TEST_ASSERT(inSuite, pool.Allocated() == kSize);
     NL_TEST_ASSERT(inSuite, pool.Exhausted());
+
+    pool.ReleaseAll();
 }
 
 #if CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
@@ -325,6 +327,8 @@ void TestForEachActiveObject(nlTestSuite * inSuite, void * inContext)
     }
     NL_TEST_ASSERT(inSuite, count >= kSize / 2);
     NL_TEST_ASSERT(inSuite, count <= kSize);
+
+    pool.ReleaseAll();
 }
 
 void TestForEachActiveObjectStatic(nlTestSuite * inSuite, void * inContext)

src/transport/SecureSessionTable.h

@@ -124,7 +124,7 @@ class SecureSessionTable
 
 private:
     Time::TimeSource<kTimeSource> mTimeSource;
-    BitMapObjectPool<SecureSession, kMaxSessionCount> mEntries;
+    BitMapObjectPool<SecureSession, kMaxSessionCount, OnObjectPoolDestruction::IgnoreUnsafeDoNotUseInNewCode> mEntries;
 };
 
 } // namespace Transport

src/transport/SessionManager.h

@@ -286,16 +286,19 @@ class DLL_EXPORT SessionManager : public TransportMgrDelegate
     State mState;                                                                         // < Initialization state of the object
 
     SessionMessageDelegate * mCB = nullptr;
-    BitMapObjectPool<std::reference_wrapper<SessionCreationDelegate>, CHIP_CONFIG_MAX_SESSION_CREATION_DELEGATES>
+    BitMapObjectPool<std::reference_wrapper<SessionCreationDelegate>, CHIP_CONFIG_MAX_SESSION_CREATION_DELEGATES,
+                     OnObjectPoolDestruction::IgnoreUnsafeDoNotUseInNewCode>
         mSessionCreationDelegates;
 
     // TODO: This is a temporary solution to release sessions, in the near future, SessionReleaseDelegate will be
     //       directly associated with the every SessionHandle. Then the callback function is called on over the handle
     //       delegate directly, in order to prevent dangling handles.
-    BitMapObjectPool<std::reference_wrapper<SessionReleaseDelegate>, CHIP_CONFIG_MAX_SESSION_RELEASE_DELEGATES>
+    BitMapObjectPool<std::reference_wrapper<SessionReleaseDelegate>, CHIP_CONFIG_MAX_SESSION_RELEASE_DELEGATES,
+                     OnObjectPoolDestruction::IgnoreUnsafeDoNotUseInNewCode>
         mSessionReleaseDelegates;
 
-    BitMapObjectPool<std::reference_wrapper<SessionRecoveryDelegate>, CHIP_CONFIG_MAX_SESSION_RECOVERY_DELEGATES>
+    BitMapObjectPool<std::reference_wrapper<SessionRecoveryDelegate>, CHIP_CONFIG_MAX_SESSION_RECOVERY_DELEGATES,
+                     OnObjectPoolDestruction::IgnoreUnsafeDoNotUseInNewCode>
         mSessionRecoveryDelegates;
 
     TransportMgrBase * mTransportMgr                                   = nullptr;

src/transport/UnauthenticatedSessionTable.h

@@ -223,7 +223,7 @@ class UnauthenticatedSessionTable
     }
 
     Time::TimeSource<Time::Source::kSystem> mTimeSource;
-    BitMapObjectPool<UnauthenticatedSession, kMaxSessionCount> mEntries;
+    BitMapObjectPool<UnauthenticatedSession, kMaxSessionCount, OnObjectPoolDestruction::IgnoreUnsafeDoNotUseInNewCode> mEntries;
 };
 
 } // namespace Transport

src/transport/raw/TCP.h

@@ -280,7 +280,9 @@ class TCP : public TCPBase
 private:
     friend class TCPTest;
     TCPBase::ActiveConnectionState mConnectionsBuffer[kActiveConnectionsSize];
-    PoolImpl<PendingPacket, kPendingPacketSize, PendingPacketPoolType::Interface> mPendingPackets;
+    PoolImpl<PendingPacket, kPendingPacketSize, OnObjectPoolDestruction::IgnoreUnsafeDoNotUseInNewCode,
+             PendingPacketPoolType::Interface>
+        mPendingPackets;
 };
 
 } // namespace Transport