diff --git a/src/app/CommandSender.cpp b/src/app/CommandSender.cpp index 1e04fb58bc75db..63053751240eb7 100644 --- a/src/app/CommandSender.cpp +++ b/src/app/CommandSender.cpp @@ -96,6 +96,9 @@ CHIP_ERROR CommandSender::SendGroupCommandRequest(const SessionHandle & session) ReturnErrorOnFailure(SendInvokeRequest()); + // Exchange is gone now, since it closed itself on successful send. + mpExchangeCtx = nullptr; + Close(); return CHIP_NO_ERROR; } diff --git a/src/app/WriteClient.cpp b/src/app/WriteClient.cpp index 106e99633b9db8..c17c63af372500 100644 --- a/src/app/WriteClient.cpp +++ b/src/app/WriteClient.cpp @@ -425,7 +425,8 @@ CHIP_ERROR WriteClient::SendWriteRequest() System::PacketBufferHandle data = mChunks.PopHead(); - if (!mChunks.IsNull() && mpExchangeCtx->IsGroupExchangeContext()) + bool isGroupWrite = mpExchangeCtx->IsGroupExchangeContext(); + if (!mChunks.IsNull() && isGroupWrite) { // Reject this request if we have more than one chunk (mChunks is not null after PopHead()), and this is a group // exchange context. @@ -434,6 +435,12 @@ CHIP_ERROR WriteClient::SendWriteRequest() // kExpectResponse is ignored by ExchangeContext in case of groupcast ReturnErrorOnFailure(mpExchangeCtx->SendMessage(MsgType::WriteRequest, std::move(data), SendMessageFlags::kExpectResponse)); + if (isGroupWrite) + { + // Exchange is closed now, since there are no group responses. Drop our + // ref to it. + mpExchangeCtx = nullptr; + } MoveToState(State::AwaitingResponse); return CHIP_NO_ERROR; } diff --git a/src/messaging/ExchangeMgr.h b/src/messaging/ExchangeMgr.h index ac796b8bd4e33b..e2ad4e8f20e388 100644 --- a/src/messaging/ExchangeMgr.h +++ b/src/messaging/ExchangeMgr.h @@ -226,7 +226,7 @@ class DLL_EXPORT ExchangeManager : public SessionMessageDelegate FabricIndex mFabricIndex = 0; - BitMapObjectPool mContextPool; + ObjectPool mContextPool; SessionManager * mSessionManager; ReliableMessageMgr mReliableMessageMgr; diff --git a/src/messaging/ReliableMessageMgr.cpp b/src/messaging/ReliableMessageMgr.cpp index e6bb6cb8f64a62..c85e3f952a6cdb 100644 --- a/src/messaging/ReliableMessageMgr.cpp +++ b/src/messaging/ReliableMessageMgr.cpp @@ -51,7 +51,7 @@ ReliableMessageMgr::RetransTableEntry::~RetransTableEntry() ec->SetMessageNotAcked(false); } -ReliableMessageMgr::ReliableMessageMgr(BitMapObjectPool & contextPool) : +ReliableMessageMgr::ReliableMessageMgr(ObjectPool & contextPool) : mContextPool(contextPool), mSystemLayer(nullptr) {} diff --git a/src/messaging/ReliableMessageMgr.h b/src/messaging/ReliableMessageMgr.h index 9538ad3f5e0c4c..ca3a9252e2f255 100644 --- a/src/messaging/ReliableMessageMgr.h +++ b/src/messaging/ReliableMessageMgr.h @@ -66,7 +66,7 @@ class ReliableMessageMgr including both successfully and failure send. */ }; - ReliableMessageMgr(BitMapObjectPool & contextPool); + ReliableMessageMgr(ObjectPool & contextPool); ~ReliableMessageMgr(); void Init(chip::System::Layer * systemLayer); @@ -177,7 +177,7 @@ class ReliableMessageMgr #endif // CHIP_CONFIG_TEST private: - BitMapObjectPool & mContextPool; + ObjectPool & mContextPool; chip::System::Layer * mSystemLayer; /* Placeholder function to run a function for all exchanges */ @@ -193,7 +193,7 @@ class ReliableMessageMgr void TicklessDebugDumpRetransTable(const char * log); // ReliableMessageProtocol Global tables for timer context - BitMapObjectPool mRetransTable; + ObjectPool mRetransTable; }; } // namespace Messaging diff --git a/src/messaging/tests/TestReliableMessageProtocol.cpp b/src/messaging/tests/TestReliableMessageProtocol.cpp index dc2118d3bb1c86..68c0ad717693f4 100644 --- a/src/messaging/tests/TestReliableMessageProtocol.cpp +++ b/src/messaging/tests/TestReliableMessageProtocol.cpp @@ -1569,6 +1569,9 @@ void CheckLostStandaloneAck(nlTestSuite * inSuite, void * inContext) // We now have just the received message waiting for an ack. NL_TEST_ASSERT(inSuite, rm->TestGetCountRetransTable() == 1); + // And receiver still has no ack pending. + NL_TEST_ASSERT(inSuite, !receiverRc->IsAckPending()); + // Reset various state so we can measure things again. mockReceiver.IsOnMessageReceivedCalled = false; mockReceiver.mReceivedPiggybackAck = false; @@ -1594,8 +1597,8 @@ void CheckLostStandaloneAck(nlTestSuite * inSuite, void * inContext) NL_TEST_ASSERT(inSuite, mockReceiver.IsOnMessageReceivedCalled); NL_TEST_ASSERT(inSuite, mockReceiver.mReceivedPiggybackAck); - // And that receiver has no ack pending. - NL_TEST_ASSERT(inSuite, !receiverRc->IsAckPending()); + // At this point all our exchanges and reliable message contexts should be + // dead, so we can't test anything about their state. // And that there are no un-acked messages left. NL_TEST_ASSERT(inSuite, rm->TestGetCountRetransTable() == 0);