diff --git a/src/messaging/ExchangeContext.cpp b/src/messaging/ExchangeContext.cpp index 281834f5d42f82..3fc9ae28b5be19 100644 --- a/src/messaging/ExchangeContext.cpp +++ b/src/messaging/ExchangeContext.cpp @@ -212,9 +212,9 @@ void ExchangeContext::Abort() Release(); } -void ExchangeContext::Reset() +void ExchangeContextDeletor::Release(ExchangeContext * ec) { - *this = ExchangeContext(); + ec->mExchangeMgr->ReleaseContext(ec); } ExchangeMessageDispatch * ExchangeContext::GetMessageDispatch() @@ -227,15 +227,12 @@ ExchangeMessageDispatch * ExchangeContext::GetMessageDispatch() return nullptr; } -ExchangeContext * ExchangeContext::Alloc(ExchangeManager * em, uint16_t ExchangeId, SecureSessionHandle session, bool Initiator, - ExchangeDelegateBase * delegate) +ExchangeContext::ExchangeContext(ExchangeManager * em, uint16_t ExchangeId, SecureSessionHandle session, bool Initiator, + ExchangeDelegateBase * delegate) { - VerifyOrDie(mExchangeMgr == nullptr && GetReferenceCount() == 0); + VerifyOrDie(mExchangeMgr == nullptr); - Reset(); - Retain(); - mExchangeMgr = em; - em->IncrementContextsInUse(); + mExchangeMgr = em; mExchangeId = ExchangeId; mSecureSession = session; mFlags.Set(Flags::kFlagInitiator, Initiator); @@ -248,28 +245,22 @@ ExchangeContext * ExchangeContext::Alloc(ExchangeManager * em, uint16_t Exchange SetAutoRequestAck(true); #if defined(CHIP_EXCHANGE_CONTEXT_DETAIL_LOGGING) - ChipLogDetail(ExchangeManager, "ec++ id: %d, inUse: %d, addr: 0x%x", (this - em->mContextPool.begin()), em->GetContextsInUse(), - this); + ChipLogDetail(ExchangeManager, "ec++ id: %d", ExchangeId); #endif SYSTEM_STATS_INCREMENT(chip::System::Stats::kExchangeMgr_NumContexts); - - return this; } -void ExchangeContext::Free() +ExchangeContext::~ExchangeContext() { VerifyOrDie(mExchangeMgr != nullptr && GetReferenceCount() == 0); // Ideally, in this scenario, the retransmit table should // be clear of any outstanding messages for this context and // the boolean parameter passed to DoClose() should not matter. - ExchangeManager * em = mExchangeMgr; DoClose(false); mExchangeMgr = nullptr; - em->DecrementContextsInUse(); - if (mExchangeACL != nullptr) { chip::Platform::Delete(mExchangeACL); @@ -277,8 +268,7 @@ void ExchangeContext::Free() } #if defined(CHIP_EXCHANGE_CONTEXT_DETAIL_LOGGING) - ChipLogDetail(ExchangeManager, "ec-- id: %d [%04" PRIX16 "], inUse: %d, addr: 0x%x", (this - em->mContextPool.begin()), - mExchangeId, em->GetContextsInUse(), this); + ChipLogDetail(ExchangeManager, "ec-- id: %d", mExchangeId); #endif SYSTEM_STATS_DECREMENT(chip::System::Stats::kExchangeMgr_NumContexts); } diff --git a/src/messaging/ExchangeContext.h b/src/messaging/ExchangeContext.h index d58e140c6528fd..fe4e0a4455092b 100644 --- a/src/messaging/ExchangeContext.h +++ b/src/messaging/ExchangeContext.h @@ -54,8 +54,7 @@ class ExchangeContextDeletor * It defines methods for encoding and communicating CHIP messages within an ExchangeContext * over various transport mechanisms, for example, TCP, UDP, or CHIP Reliable Messaging. */ -class DLL_EXPORT ExchangeContext : public ReliableMessageContext, - public ReferenceCounted +class DLL_EXPORT ExchangeContext : public ReliableMessageContext, public ReferenceCounted { friend class ExchangeManager; friend class ExchangeContextDeletor; @@ -63,6 +62,11 @@ class DLL_EXPORT ExchangeContext : public ReliableMessageContext, public: typedef uint32_t Timeout; // Type used to express the timeout in this ExchangeContext, in milliseconds + ExchangeContext(ExchangeManager * em, uint16_t ExchangeId, SecureSessionHandle session, bool Initiator, + ExchangeDelegateBase * delegate); + + ~ExchangeContext(); + /** * Determine whether the context is the initiator of the exchange. * @@ -170,11 +174,6 @@ class DLL_EXPORT ExchangeContext : public ReliableMessageContext, SecureSessionHandle mSecureSession; // The connection state uint16_t mExchangeId; // Assigned exchange ID. - ExchangeContext * Alloc(ExchangeManager * em, uint16_t ExchangeId, SecureSessionHandle session, bool Initiator, - ExchangeDelegateBase * delegate); - void Free(); - void Reset(); - /** * Determine whether a response is currently expected for a message that was sent over * this exchange. While this is true, attempts to send other messages that expect a response @@ -216,10 +215,5 @@ class DLL_EXPORT ExchangeContext : public ReliableMessageContext, void DoClose(bool clearRetransTable); }; -inline void ExchangeContextDeletor::Release(ExchangeContext * obj) -{ - obj->Free(); -} - } // namespace Messaging } // namespace chip diff --git a/src/messaging/ExchangeMgr.cpp b/src/messaging/ExchangeMgr.cpp index 0c5fe415f0edb6..210238c29905a1 100644 --- a/src/messaging/ExchangeMgr.cpp +++ b/src/messaging/ExchangeMgr.cpp @@ -75,8 +75,6 @@ CHIP_ERROR ExchangeManager::Init(SecureSessionMgr * sessionMgr) mNextExchangeId = GetRandU16(); mNextKeyId = 0; - mContextsInUse = 0; - for (auto & handler : UMHandlerPool) { // Mark all handlers as unallocated. This handles both initial @@ -98,11 +96,11 @@ CHIP_ERROR ExchangeManager::Shutdown() { mReliableMessageMgr.Shutdown(); - for (auto & ec : mContextPool) - { - // ExchangeContext leaked - assert(ec.GetReferenceCount() == 0); - } + mContextPool.ForEachActiveObject([](auto * ec) { + // There should be no active object in the pool + assert(false); + return true; + }); if (mSessionMgr != nullptr) { @@ -117,7 +115,7 @@ CHIP_ERROR ExchangeManager::Shutdown() ExchangeContext * ExchangeManager::NewContext(SecureSessionHandle session, ExchangeDelegateBase * delegate) { - return AllocContext(mNextExchangeId++, session, true, delegate); + return mContextPool.CreateObject(this, mNextExchangeId++, session, true, delegate); } CHIP_ERROR ExchangeManager::RegisterUnsolicitedMessageHandlerForProtocol(Protocols::Id protocolId, ExchangeDelegateBase * delegate) @@ -146,23 +144,6 @@ void ExchangeManager::OnReceiveError(CHIP_ERROR error, const Transport::PeerAddr ChipLogError(ExchangeManager, "Accept FAILED, err = %s", ErrorStr(error)); } -ExchangeContext * ExchangeManager::AllocContext(uint16_t ExchangeId, SecureSessionHandle session, bool Initiator, - ExchangeDelegateBase * delegate) -{ - CHIP_FAULT_INJECT(FaultInjection::kFault_AllocExchangeContext, return nullptr); - - for (auto & ec : mContextPool) - { - if (ec.GetReferenceCount() == 0) - { - return ec.Alloc(this, ExchangeId, session, Initiator, delegate); - } - } - - ChipLogError(ExchangeManager, "Alloc ctxt FAILED"); - return nullptr; -} - CHIP_ERROR ExchangeManager::RegisterUMH(Protocols::Id protocolId, int16_t msgType, ExchangeDelegateBase * delegate) { UnsolicitedMessageHandler * umh = UMHandlerPool; @@ -224,22 +205,28 @@ void ExchangeManager::OnMessageReceived(const PacketHeader & packetHeader, const payloadHeader.GetProtocolID()); // Search for an existing exchange that the message applies to. If a match is found... - for (auto & ec : mContextPool) - { - if (ec.GetReferenceCount() > 0 && ec.MatchExchange(session, packetHeader, payloadHeader)) + bool found = false; + mContextPool.ForEachActiveObject([&](auto * ec) { + if (ec->MatchExchange(session, packetHeader, payloadHeader)) { // Found a matching exchange. Set flag for correct subsequent CRMP // retransmission timeout selection. - if (!ec.HasRcvdMsgFromPeer()) + if (!ec->HasRcvdMsgFromPeer()) { - ec.SetMsgRcvdFromPeer(true); + ec->SetMsgRcvdFromPeer(true); } // Matched ExchangeContext; send to message handler. - ec.HandleMessage(packetHeader, payloadHeader, source, std::move(msgBuf)); - - ExitNow(err = CHIP_NO_ERROR); + ec->HandleMessage(packetHeader, payloadHeader, source, std::move(msgBuf)); + found = true; + return false; } + return true; + }); + + if (found) + { + ExitNow(err = CHIP_NO_ERROR); } // Search for an unsolicited message handler if it marked as being sent by an initiator. Since we didn't @@ -289,17 +276,16 @@ void ExchangeManager::OnMessageReceived(const PacketHeader & packetHeader, const // If rcvd msg is from initiator then this exchange is created as not Initiator. // If rcvd msg is not from initiator then this exchange is created as Initiator. // TODO: Figure out which channel to use for the received message - ec = AllocContext(payloadHeader.GetExchangeID(), session, !payloadHeader.IsInitiator(), nullptr); + ec = mContextPool.CreateObject(this, payloadHeader.GetExchangeID(), session, !payloadHeader.IsInitiator(), nullptr); } else { - ec = AllocContext(payloadHeader.GetExchangeID(), session, false, matchingUMH->Delegate); + ec = mContextPool.CreateObject(this, payloadHeader.GetExchangeID(), session, false, matchingUMH->Delegate); } VerifyOrExit(ec != nullptr, err = CHIP_ERROR_NO_MEMORY); - ChipLogDetail(ExchangeManager, "ec pos: %d, id: %d, Delegate: 0x%x", ec - mContextPool.begin(), ec->GetExchangeId(), - ec->GetDelegate()); + ChipLogDetail(ExchangeManager, "ec id: %d, Delegate: 0x%x", ec->GetExchangeId(), ec->GetDelegate()); ec->HandleMessage(packetHeader, payloadHeader, source, std::move(msgBuf)); @@ -330,14 +316,14 @@ void ExchangeManager::OnConnectionExpired(SecureSessionHandle session, SecureSes mDelegate->OnConnectionExpired(session, this); } - for (auto & ec : mContextPool) - { - if (ec.GetReferenceCount() > 0 && ec.mSecureSession == session) + mContextPool.ForEachActiveObject([&](auto * ec) { + if (ec->mSecureSession == session) { - ec.Close(); + ec->Close(); // Continue iterate because there can be multiple contexts associated with the connection. } - } + return true; + }); } void ExchangeManager::OnMessageReceived(const Transport::PeerAddress & source, System::PacketBufferHandle msgBuf) @@ -359,37 +345,18 @@ void ExchangeManager::OnMessageReceived(const Transport::PeerAddress & source, S void ExchangeManager::CloseAllContextsForDelegate(const ExchangeDelegateBase * delegate) { - for (auto & ec : mContextPool) - { - if (ec.GetReferenceCount() == 0 || ec.GetDelegate() != delegate) + mContextPool.ForEachActiveObject([&](auto * ec) { + if (ec->GetDelegate() == delegate) { - continue; + // Make sure to null out the delegate before closing the context, so + // we don't notify the delegate that the context is closing. We + // have to do this, because the delegate might be partially + // destroyed by this point. + ec->SetDelegate(nullptr); + ec->Close(); } - - // Make sure to null out the delegate before closing the context, so - // we don't notify the delegate that the context is closing. We - // have to do this, because the delegate might be partially - // destroyed by this point. - ec.SetDelegate(nullptr); - ec.Close(); - } -} - -void ExchangeManager::IncrementContextsInUse() -{ - mContextsInUse++; -} - -void ExchangeManager::DecrementContextsInUse() -{ - if (mContextsInUse >= 1) - { - mContextsInUse--; - } - else - { - ChipLogError(ExchangeManager, "No context in use, decrement failed"); - } + return true; + }); } } // namespace Messaging diff --git a/src/messaging/ExchangeMgr.h b/src/messaging/ExchangeMgr.h index 930d233e643469..8393fee57be568 100644 --- a/src/messaging/ExchangeMgr.h +++ b/src/messaging/ExchangeMgr.h @@ -31,6 +31,7 @@ #include #include #include +#include #include #include @@ -100,6 +101,8 @@ class DLL_EXPORT ExchangeManager : public SecureSessionMgrDelegate, public Trans */ ExchangeContext * NewContext(SecureSessionHandle session, ExchangeDelegateBase * delegate); + void ReleaseContext(ExchangeContext * ec) { mContextPool.ReleaseObject(ec); } + /** * Register an unsolicited message handler for a given protocol identifier. This handler would be * invoked for all messages of the given protocol. @@ -182,9 +185,6 @@ class DLL_EXPORT ExchangeManager : public SecureSessionMgrDelegate, public Trans */ void CloseAllContextsForDelegate(const ExchangeDelegateBase * delegate); - void IncrementContextsInUse(); - void DecrementContextsInUse(); - void SetDelegate(ExchangeMgrDelegate * delegate) { mDelegate = delegate; } SecureSessionMgr * GetSessionMgr() const { return mSessionMgr; } @@ -194,7 +194,6 @@ class DLL_EXPORT ExchangeManager : public SecureSessionMgrDelegate, public Trans Transport::AdminId GetAdminId() { return mAdminId; } uint16_t GetNextKeyId() { return ++mNextKeyId; } - size_t GetContextsInUse() const { return mContextsInUse; } private: enum class State @@ -234,14 +233,10 @@ class DLL_EXPORT ExchangeManager : public SecureSessionMgrDelegate, public Trans Transport::AdminId mAdminId = 0; - std::array mContextPool; - size_t mContextsInUse; + BitMapObjectPool mContextPool; UnsolicitedMessageHandler UMHandlerPool[CHIP_CONFIG_MAX_UNSOLICITED_MESSAGE_HANDLERS]; - ExchangeContext * AllocContext(uint16_t ExchangeId, SecureSessionHandle session, bool Initiator, - ExchangeDelegateBase * delegate); - CHIP_ERROR RegisterUMH(Protocols::Id protocolId, int16_t msgType, ExchangeDelegateBase * delegate); CHIP_ERROR UnregisterUMH(Protocols::Id protocolId, int16_t msgType); diff --git a/src/messaging/ReliableMessageMgr.cpp b/src/messaging/ReliableMessageMgr.cpp index 40c2cdb28bae53..f60c3797f909e6 100644 --- a/src/messaging/ReliableMessageMgr.cpp +++ b/src/messaging/ReliableMessageMgr.cpp @@ -39,7 +39,7 @@ namespace Messaging { ReliableMessageMgr::RetransTableEntry::RetransTableEntry() : rc(nullptr), nextRetransTimeTick(0), sendCount(0) {} -ReliableMessageMgr::ReliableMessageMgr(std::array & contextPool) : +ReliableMessageMgr::ReliableMessageMgr(BitMapObjectPool & contextPool) : mContextPool(contextPool), mSystemLayer(nullptr), mSessionMgr(nullptr), mCurrentTimerExpiry(0), mTimerIntervalShift(CHIP_CONFIG_RMP_TIMER_DEFAULT_PERIOD_SHIFT) {} diff --git a/src/messaging/ReliableMessageMgr.h b/src/messaging/ReliableMessageMgr.h index b66a2ba2a955b0..10480e6f0a7c42 100644 --- a/src/messaging/ReliableMessageMgr.h +++ b/src/messaging/ReliableMessageMgr.h @@ -31,6 +31,7 @@ #include #include +#include #include #include #include @@ -66,7 +67,7 @@ class ReliableMessageMgr }; public: - ReliableMessageMgr(std::array & contextPool); + ReliableMessageMgr(BitMapObjectPool & contextPool); ~ReliableMessageMgr(); void Init(chip::System::Layer * systemLayer, SecureSessionMgr * sessionMgr); @@ -223,7 +224,7 @@ class ReliableMessageMgr void TestSetIntervalShift(uint16_t value) { mTimerIntervalShift = value; } private: - std::array & mContextPool; + BitMapObjectPool & mContextPool; chip::System::Layer * mSystemLayer; SecureSessionMgr * mSessionMgr; uint64_t mTimeStampBase; // ReliableMessageProtocol timer base value to add offsets to evaluate timeouts @@ -234,10 +235,10 @@ class ReliableMessageMgr template void ExecuteForAllContext(Function function) { - for (auto & ec : mContextPool) - { - function(ec.GetReliableMessageContext()); - } + mContextPool.ForEachActiveObject([&](auto * ec) { + function(ec->GetReliableMessageContext()); + return true; + }); } void TicklessDebugDumpRetransTable(const char * log);