diff --git a/src/app/CASESessionManager.h b/src/app/CASESessionManager.h index bf771945348875..65253af176c766 100644 --- a/src/app/CASESessionManager.h +++ b/src/app/CASESessionManager.h @@ -103,7 +103,9 @@ class CASESessionManager : public SessionReleaseDelegate, public Dnssd::Resolver OperationalDeviceProxy * FindSession(SessionHandle session); void ReleaseSession(OperationalDeviceProxy * device); - BitMapObjectPool mActiveSessions; + BitMapObjectPool + mActiveSessions; CASESessionManagerConfig mConfig; }; diff --git a/src/app/InteractionModelEngine.h b/src/app/InteractionModelEngine.h index 85a2d5eaebf7c1..1d10ee2a417a60 100644 --- a/src/app/InteractionModelEngine.h +++ b/src/app/InteractionModelEngine.h @@ -281,7 +281,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 mCommandHandlerObjs; - BitMapObjectPool mTimedHandlers; + BitMapObjectPool + 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]; diff --git a/src/controller/CHIPDeviceControllerFactory.cpp b/src/controller/CHIPDeviceControllerFactory.cpp index df423e1b4620f3..95bb7f69a1de97 100644 --- a/src/controller/CHIPDeviceControllerFactory.cpp +++ b/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) { diff --git a/src/credentials/GroupDataProviderImpl.h b/src/credentials/GroupDataProviderImpl.h index 638816fe8eeecf..e650ffd64fc517 100644 --- a/src/credentials/GroupDataProviderImpl.h +++ b/src/credentials/GroupDataProviderImpl.h @@ -138,7 +138,7 @@ class GroupDataProviderImpl : public GroupDataProvider BitMapObjectPool mEndpointIterators; BitMapObjectPool mAllStatesIterators; BitMapObjectPool mFabricStatesIterators; - BitMapObjectPool mKeySetIterators; + BitMapObjectPool mKeySetIterators; }; } // namespace Credentials diff --git a/src/lib/dnssd/minimal_mdns/Server.h b/src/lib/dnssd/minimal_mdns/Server.h index d471db071b18de..22910c8cc1fb19 100644 --- a/src/lib/dnssd/minimal_mdns/Server.h +++ b/src/lib/dnssd/minimal_mdns/Server.h @@ -228,7 +228,8 @@ 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 6af8a16227fa4f..167b5578c9d7f9 100644 --- a/src/lib/dnssd/minimal_mdns/tests/CheckOnlyServer.h +++ b/src/lib/dnssd/minimal_mdns/tests/CheckOnlyServer.h @@ -71,7 +71,8 @@ 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 679ea1f9e37fb2..1b4e77253b6563 100644 --- a/src/lib/support/Pool.h +++ b/src/lib/support/Pool.h @@ -22,6 +22,7 @@ #pragma once +#include #include #include @@ -161,6 +162,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 * @@ -193,14 +204,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 +template class BitMapObjectPool : public internal::StaticAllocatorBitmap, public internal::PoolCommon { 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 @@ -268,15 +289,24 @@ 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() { - // 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 @@ -343,11 +373,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 @@ -358,17 +388,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 db08761a9b15ec..6ec3c48503f2cd 100644 --- a/src/lib/support/PoolWrapper.h +++ b/src/lib/support/PoolWrapper.h @@ -52,11 +52,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"); @@ -80,7 +80,7 @@ class PoolProxy> : public PoolInter return Impl().ForEachActiveObject([&](T * target) { return lambda(context, static_cast(target)); }); } - virtual BitMapObjectPool & Impl() = 0; + virtual BitMapObjectPool & Impl() = 0; }; /* @@ -94,18 +94,18 @@ class PoolProxy> : public PoolInter * 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/lib/support/tests/TestPool.cpp b/src/lib/support/tests/TestPool.cpp index c9242d054cad1b..b78e684abf8866 100644 --- a/src/lib/support/tests/TestPool.cpp +++ b/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) diff --git a/src/transport/SecureSessionTable.h b/src/transport/SecureSessionTable.h index d40e6626b6789a..ba5df93f44e59d 100644 --- a/src/transport/SecureSessionTable.h +++ b/src/transport/SecureSessionTable.h @@ -124,7 +124,7 @@ class SecureSessionTable private: Time::TimeSource mTimeSource; - BitMapObjectPool mEntries; + BitMapObjectPool mEntries; }; } // namespace Transport diff --git a/src/transport/SessionManager.h b/src/transport/SessionManager.h index 6e8ec58a7fbd7e..88ff08e1d26dbb 100644 --- a/src/transport/SessionManager.h +++ b/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, CHIP_CONFIG_MAX_SESSION_CREATION_DELEGATES> + BitMapObjectPool, 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, CHIP_CONFIG_MAX_SESSION_RELEASE_DELEGATES> + BitMapObjectPool, CHIP_CONFIG_MAX_SESSION_RELEASE_DELEGATES, + OnObjectPoolDestruction::IgnoreUnsafeDoNotUseInNewCode> mSessionReleaseDelegates; - BitMapObjectPool, CHIP_CONFIG_MAX_SESSION_RECOVERY_DELEGATES> + BitMapObjectPool, CHIP_CONFIG_MAX_SESSION_RECOVERY_DELEGATES, + OnObjectPoolDestruction::IgnoreUnsafeDoNotUseInNewCode> mSessionRecoveryDelegates; TransportMgrBase * mTransportMgr = nullptr; diff --git a/src/transport/UnauthenticatedSessionTable.h b/src/transport/UnauthenticatedSessionTable.h index 032f19e2a919f1..3d1d9a3dedd1ff 100644 --- a/src/transport/UnauthenticatedSessionTable.h +++ b/src/transport/UnauthenticatedSessionTable.h @@ -223,7 +223,7 @@ class UnauthenticatedSessionTable } Time::TimeSource mTimeSource; - BitMapObjectPool mEntries; + BitMapObjectPool mEntries; }; } // namespace Transport diff --git a/src/transport/raw/TCP.h b/src/transport/raw/TCP.h index 16cc64c5b6cc88..e5295186c7b3a5 100644 --- a/src/transport/raw/TCP.h +++ b/src/transport/raw/TCP.h @@ -280,7 +280,9 @@ class TCP : public TCPBase private: friend class TCPTest; TCPBase::ActiveConnectionState mConnectionsBuffer[kActiveConnectionsSize]; - PoolImpl mPendingPackets; + PoolImpl + mPendingPackets; }; } // namespace Transport