diff --git a/src/app/CASEClientPool.h b/src/app/CASEClientPool.h index 15c9f2a985ff44..79e29d3ede1ddb 100644 --- a/src/app/CASEClientPool.h +++ b/src/app/CASEClientPool.h @@ -36,12 +36,14 @@ template class CASEClientPool : public CASEClientPoolDelegate { public: + ~CASEClientPool() { mClientPool.ReleaseAll(); } + CASEClient * Allocate(CASEClientInitParams params) override { return mClientPool.CreateObject(params); } void Release(CASEClient * client) override { mClientPool.ReleaseObject(client); } private: - BitMapObjectPool mClientPool; + BitMapObjectPool mClientPool; }; }; // namespace chip diff --git a/src/app/OperationalDeviceProxyPool.h b/src/app/OperationalDeviceProxyPool.h index ee920c3544efff..79b63391d9cf4b 100644 --- a/src/app/OperationalDeviceProxyPool.h +++ b/src/app/OperationalDeviceProxyPool.h @@ -44,6 +44,8 @@ template class OperationalDeviceProxyPool : public OperationalDeviceProxyPoolDelegate { public: + ~OperationalDeviceProxyPool() { mDevicePool.ReleaseAll(); } + OperationalDeviceProxy * Allocate(DeviceProxyInitParams & params, PeerId peerId) override { return mDevicePool.CreateObject(params, peerId); @@ -88,7 +90,7 @@ class OperationalDeviceProxyPool : public OperationalDeviceProxyPoolDelegate } private: - BitMapObjectPool mDevicePool; + BitMapObjectPool mDevicePool; }; }; // namespace chip diff --git a/src/lib/dnssd/minimal_mdns/Server.h b/src/lib/dnssd/minimal_mdns/Server.h index edcff644bb36eb..f3fe1ef0c8bf4b 100644 --- a/src/lib/dnssd/minimal_mdns/Server.h +++ b/src/lib/dnssd/minimal_mdns/Server.h @@ -228,8 +228,7 @@ class ServerBase // The PoolImpl impl is used as a base class because its destructor must be called after ServerBase's destructor. template -class Server : private chip::PoolImpl, +class Server : private chip::PoolImpl, public ServerBase { public: diff --git a/src/lib/dnssd/minimal_mdns/tests/CheckOnlyServer.h b/src/lib/dnssd/minimal_mdns/tests/CheckOnlyServer.h index 167b5578c9d7f9..6af8a16227fa4f 100644 --- a/src/lib/dnssd/minimal_mdns/tests/CheckOnlyServer.h +++ b/src/lib/dnssd/minimal_mdns/tests/CheckOnlyServer.h @@ -71,8 +71,7 @@ void MakePrintableName(char (&location)[N], FullQName name) } // namespace -class CheckOnlyServer : private chip::PoolImpl, +class CheckOnlyServer : private chip::PoolImpl, public ServerBase, public ParserDelegate, public TxtRecordDelegate diff --git a/src/lib/support/Pool.h b/src/lib/support/Pool.h index 15000182005e83..4a36fa28202856 100644 --- a/src/lib/support/Pool.h +++ b/src/lib/support/Pool.h @@ -162,16 +162,6 @@ 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 * @@ -204,25 +194,12 @@ enum class OnObjectPoolDestruction * @tparam T type of element to be allocated. * @tparam N a positive integer max number of elements the pool provides. */ -template +template class BitMapObjectPool : public internal::StaticAllocatorBitmap, public internal::PoolCommon { public: BitMapObjectPool() : StaticAllocatorBitmap(mData.mMemory, mUsage, N, sizeof(T)) {} - ~BitMapObjectPool() - { - switch (Action) - { - case OnObjectPoolDestruction::AutoRelease: - ReleaseAll(); - break; - case OnObjectPoolDestruction::Die: - VerifyOrDie(Allocated() == 0); - break; - case OnObjectPoolDestruction::IgnoreUnsafeDoNotUseInNewCode: - break; - } - } + ~BitMapObjectPool() { VerifyOrDie(Allocated() == 0); } template T * CreateObject(Args &&... args) @@ -289,25 +266,12 @@ class BitMapObjectPool : public internal::StaticAllocatorBitmap, public internal * * @tparam T type to be allocated. */ -template +template class HeapObjectPool : public internal::Statistics, public internal::PoolCommon { public: HeapObjectPool() {} - ~HeapObjectPool() - { - switch (Action) - { - case OnObjectPoolDestruction::AutoRelease: - ReleaseAll(); - break; - case OnObjectPoolDestruction::Die: - VerifyOrDie(Allocated() == 0); - break; - case OnObjectPoolDestruction::IgnoreUnsafeDoNotUseInNewCode: - break; - } - } + ~HeapObjectPool() { VerifyOrDie(Allocated() == 0); } template T * CreateObject(Args &&... args) @@ -373,11 +337,11 @@ class HeapObjectPool : public internal::Statistics, public internal::PoolCommon< #endif // CHIP_SYSTEM_CONFIG_POOL_USE_HEAP #if CHIP_SYSTEM_CONFIG_POOL_USE_HEAP -template -using ObjectPool = HeapObjectPool; +template +using ObjectPool = HeapObjectPool; #else // CHIP_SYSTEM_CONFIG_POOL_USE_HEAP -template -using ObjectPool = BitMapObjectPool; +template +using ObjectPool = BitMapObjectPool; #endif // CHIP_SYSTEM_CONFIG_POOL_USE_HEAP enum class ObjectPoolMem @@ -388,17 +352,17 @@ enum class ObjectPoolMem #endif // CHIP_SYSTEM_CONFIG_POOL_USE_HEAP }; -template +template class MemTypeObjectPool; -template -class MemTypeObjectPool : public BitMapObjectPool +template +class MemTypeObjectPool : public BitMapObjectPool { }; #if CHIP_SYSTEM_CONFIG_POOL_USE_HEAP -template -class MemTypeObjectPool : public HeapObjectPool +template +class MemTypeObjectPool : public HeapObjectPool { }; #endif // CHIP_SYSTEM_CONFIG_POOL_USE_HEAP diff --git a/src/lib/support/PoolWrapper.h b/src/lib/support/PoolWrapper.h index 6ec3c48503f2cd..13195ca70310df 100644 --- a/src/lib/support/PoolWrapper.h +++ b/src/lib/support/PoolWrapper.h @@ -35,6 +35,7 @@ class 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; template @@ -52,11 +53,11 @@ class PoolInterface virtual Loop ForEachActiveObjectInner(void * context, Lambda lambda) = 0; }; -template +template class PoolProxy; -template -class PoolProxy> : public PoolInterface +template +class PoolProxy> : public PoolInterface { public: static_assert(std::is_base_of::value, "Interface type is not derived from Pool type"); @@ -68,6 +69,8 @@ class PoolProxy> : public PoolIn virtual void ReleaseObject(U * element) override { Impl().ReleaseObject(static_cast(element)); } + virtual void ReleaseAll() override { Impl().ReleaseAll(); } + virtual void ResetObject(U * element, ConstructorArguments... args) override { return Impl().ResetObject(static_cast(element), std::move(args)...); @@ -80,7 +83,7 @@ class PoolProxy> : public PoolIn return Impl().ForEachActiveObject([&](T * target) { return lambda(context, static_cast(target)); }); } - virtual BitMapObjectPool & Impl() = 0; + virtual BitMapObjectPool & Impl() = 0; }; /* @@ -94,18 +97,18 @@ class PoolProxy> : public PoolIn * PoolInterface, the PoolImpl can be converted to the interface type * and passed around */ -template -class PoolImpl : public PoolProxy... +template +class PoolImpl : public PoolProxy... { public: PoolImpl() {} virtual ~PoolImpl() override {} protected: - virtual BitMapObjectPool & Impl() override { return mImpl; } + virtual BitMapObjectPool & Impl() override { return mImpl; } private: - BitMapObjectPool mImpl; + BitMapObjectPool mImpl; }; } // namespace chip diff --git a/src/transport/SecureSessionTable.h b/src/transport/SecureSessionTable.h index 9b6c9e99538ccf..8a44c52d951222 100644 --- a/src/transport/SecureSessionTable.h +++ b/src/transport/SecureSessionTable.h @@ -41,6 +41,8 @@ template mTimeSource; - BitMapObjectPool mEntries; + BitMapObjectPool mEntries; }; } // namespace Transport diff --git a/src/transport/SessionManager.cpp b/src/transport/SessionManager.cpp index 3ad613291f6ac6..f0e6078be8bbf4 100644 --- a/src/transport/SessionManager.cpp +++ b/src/transport/SessionManager.cpp @@ -95,6 +95,9 @@ void SessionManager::Shutdown() { CancelExpiryTimer(); + mSessionReleaseDelegates.ReleaseAll(); + mSessionRecoveryDelegates.ReleaseAll(); + mMessageCounterManager = nullptr; mState = State::kNotReady; diff --git a/src/transport/SessionManager.h b/src/transport/SessionManager.h index 0ddcec2babc5f9..d2d52983cf5669 100644 --- a/src/transport/SessionManager.h +++ b/src/transport/SessionManager.h @@ -272,12 +272,10 @@ class DLL_EXPORT SessionManager : public TransportMgrDelegate // TODO: This is a temporary solution to release sessions, in the near future, SessionReleaseDelegate will be // directly associated with the every SessionHolder. Then the callback function is called on over the handle // delegate directly, in order to prevent dangling handles. - BitMapObjectPool, CHIP_CONFIG_MAX_SESSION_RELEASE_DELEGATES, - OnObjectPoolDestruction::IgnoreUnsafeDoNotUseInNewCode> + BitMapObjectPool, CHIP_CONFIG_MAX_SESSION_RELEASE_DELEGATES> mSessionReleaseDelegates; - BitMapObjectPool, CHIP_CONFIG_MAX_SESSION_RECOVERY_DELEGATES, - OnObjectPoolDestruction::IgnoreUnsafeDoNotUseInNewCode> + BitMapObjectPool, CHIP_CONFIG_MAX_SESSION_RECOVERY_DELEGATES> mSessionRecoveryDelegates; TransportMgrBase * mTransportMgr = nullptr; diff --git a/src/transport/UnauthenticatedSessionTable.h b/src/transport/UnauthenticatedSessionTable.h index 3d1d9a3dedd1ff..55c1e126ec003a 100644 --- a/src/transport/UnauthenticatedSessionTable.h +++ b/src/transport/UnauthenticatedSessionTable.h @@ -88,6 +88,8 @@ template mTimeSource; - BitMapObjectPool mEntries; + BitMapObjectPool mEntries; }; } // namespace Transport diff --git a/src/transport/raw/TCP.h b/src/transport/raw/TCP.h index e2e257ba262d0b..44292b2e131592 100644 --- a/src/transport/raw/TCP.h +++ b/src/transport/raw/TCP.h @@ -276,13 +276,12 @@ class TCP : public TCPBase mConnectionsBuffer[i].Init(nullptr); } } + ~TCP() { mPendingPackets.ReleaseAll(); } private: friend class TCPTest; TCPBase::ActiveConnectionState mConnectionsBuffer[kActiveConnectionsSize]; - PoolImpl - mPendingPackets; + PoolImpl mPendingPackets; }; } // namespace Transport diff --git a/src/transport/tests/TestSessionManager.cpp b/src/transport/tests/TestSessionManager.cpp index 4febaf7d2fc8b2..db198f3fbae20f 100644 --- a/src/transport/tests/TestSessionManager.cpp +++ b/src/transport/tests/TestSessionManager.cpp @@ -202,6 +202,8 @@ void CheckMessageTest(nlTestSuite * inSuite, void * inContext) err = sessionManager.PrepareMessage(localToRemoteSession, payloadHeader, std::move(extra_large_buffer), preparedMessage); NL_TEST_ASSERT(inSuite, err == CHIP_ERROR_MESSAGE_TOO_LONG); + + sessionManager.Shutdown(); } void SendEncryptedPacketTest(nlTestSuite * inSuite, void * inContext) @@ -278,6 +280,8 @@ void SendEncryptedPacketTest(nlTestSuite * inSuite, void * inContext) NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); NL_TEST_ASSERT(inSuite, callback.ReceiveHandlerCallCount == 2); + + sessionManager.Shutdown(); } void SendBadEncryptedPacketTest(nlTestSuite * inSuite, void * inContext) @@ -392,6 +396,8 @@ void SendBadEncryptedPacketTest(nlTestSuite * inSuite, void * inContext) NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); NL_TEST_ASSERT(inSuite, callback.ReceiveHandlerCallCount == 2); + + sessionManager.Shutdown(); } void StaleConnectionDropTest(nlTestSuite * inSuite, void * inContext) @@ -459,6 +465,8 @@ void StaleConnectionDropTest(nlTestSuite * inSuite, void * inContext) CryptoContext::SessionRole::kResponder, 0); NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); NL_TEST_ASSERT(inSuite, callback.mOldConnectionDropped); + + sessionManager.Shutdown(); } // Test Suite