From 4303697f7b1af63f42b8b7fd0383229fe7e64757 Mon Sep 17 00:00:00 2001 From: yunhanw-google Date: Thu, 9 Dec 2021 15:26:53 -0800 Subject: [PATCH] Fix mSelfProcessedEvents init/cleanup bug (#12829) --- src/app/EventManagement.cpp | 3 +-- src/app/ReadHandler.cpp | 28 ++++++++++++++++++--------- src/app/ReadHandler.h | 8 ++++---- src/app/tests/TestReadInteraction.cpp | 17 +++++++++++++++- 4 files changed, 40 insertions(+), 16 deletions(-) diff --git a/src/app/EventManagement.cpp b/src/app/EventManagement.cpp index fd3713768ec616..1455ad49103fda 100644 --- a/src/app/EventManagement.cpp +++ b/src/app/EventManagement.cpp @@ -671,7 +671,6 @@ CHIP_ERROR EventManagement::CopyEventsSince(const TLVReader & aReader, size_t aD { EventLoadOutContext * const loadOutContext = static_cast(apContext); CHIP_ERROR err = EventIterator(aReader, aDepth, loadOutContext); - loadOutContext->mCurrentEventNumber++; if (err == CHIP_EVENT_ID_FOUND) { // checkpoint the writer @@ -693,7 +692,7 @@ CHIP_ERROR EventManagement::CopyEventsSince(const TLVReader & aReader, size_t aD loadOutContext->mFirst = false; loadOutContext->mEventCount++; } - + loadOutContext->mCurrentEventNumber++; return err; } diff --git a/src/app/ReadHandler.cpp b/src/app/ReadHandler.cpp index def836c43b9c8d..7f352043898d28 100644 --- a/src/app/ReadHandler.cpp +++ b/src/app/ReadHandler.cpp @@ -47,7 +47,12 @@ CHIP_ERROR ReadHandler::Init(Messaging::ExchangeManager * apExchangeMgr, Interac mpAttributeClusterInfoList = nullptr; mpEventClusterInfoList = nullptr; mCurrentPriority = PriorityLevel::Invalid; - mIsPrimingReports = true; + for (size_t index = 0; index < kNumPriorityLevel; index++) + { + mSelfProcessedEvents[index] = 0; + mLastScheduledEventNumber[index] = 0; + } + mIsPrimingReports = true; MoveToState(HandlerState::Initialized); mpDelegate = apDelegate; mSubscriptionId = 0; @@ -103,14 +108,19 @@ void ReadHandler::Shutdown(ShutdownOptions aOptions) mpAttributeClusterInfoList = nullptr; mpEventClusterInfoList = nullptr; mCurrentPriority = PriorityLevel::Invalid; - mIsPrimingReports = false; - mpDelegate = nullptr; - mHoldReport = false; - mDirty = false; - mActiveSubscription = false; - mIsChunkedReport = false; - mInitiatorNodeId = kUndefinedNodeId; - mHoldSync = false; + for (size_t index = 0; index < kNumPriorityLevel; index++) + { + mSelfProcessedEvents[index] = 0; + mLastScheduledEventNumber[index] = 0; + } + mIsPrimingReports = false; + mpDelegate = nullptr; + mHoldReport = false; + mDirty = false; + mActiveSubscription = false; + mIsChunkedReport = false; + mInitiatorNodeId = kUndefinedNodeId; + mHoldSync = false; } CHIP_ERROR ReadHandler::OnReadInitialRequest(System::PacketBufferHandle && aPayload) diff --git a/src/app/ReadHandler.h b/src/app/ReadHandler.h index 438ae5c59a9da9..2aa340573e157b 100644 --- a/src/app/ReadHandler.h +++ b/src/app/ReadHandler.h @@ -199,12 +199,12 @@ class ReadHandler : public Messaging::ExchangeDelegate PriorityLevel mCurrentPriority = PriorityLevel::Invalid; // The event number of the last processed event for each priority level - EventNumber mSelfProcessedEvents[kNumPriorityLevel]; + EventNumber mSelfProcessedEvents[kNumPriorityLevel] = { 0 }; // The last schedule event number snapshoted in the beginning when preparing to fill new events to reports - EventNumber mLastScheduledEventNumber[kNumPriorityLevel]; - Messaging::ExchangeManager * mpExchangeMgr = nullptr; - InteractionModelDelegate * mpDelegate = nullptr; + EventNumber mLastScheduledEventNumber[kNumPriorityLevel] = { 0 }; + Messaging::ExchangeManager * mpExchangeMgr = nullptr; + InteractionModelDelegate * mpDelegate = nullptr; // Tracks whether we're in the initial phase of receiving priming // reports, which is always true for reads and true for subscriptions diff --git a/src/app/tests/TestReadInteraction.cpp b/src/app/tests/TestReadInteraction.cpp index 11873acb523d6e..cbfca6f880f124 100644 --- a/src/app/tests/TestReadInteraction.cpp +++ b/src/app/tests/TestReadInteraction.cpp @@ -695,6 +695,7 @@ void TestReadInteraction::TestReadRoundtrip(nlTestSuite * apSuite, void * apCont readPrepareParams.mEventPathParamsListSize = 1; readPrepareParams.mpAttributePathParamsList = attributePathParams; readPrepareParams.mAttributePathParamsListSize = 2; + err = chip::app::InteractionModelEngine::GetInstance()->SendReadRequest(readPrepareParams, &delegate); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); @@ -703,6 +704,18 @@ void TestReadInteraction::TestReadRoundtrip(nlTestSuite * apSuite, void * apCont NL_TEST_ASSERT(apSuite, delegate.mNumAttributeResponse == 2); NL_TEST_ASSERT(apSuite, delegate.mGotReport); NL_TEST_ASSERT(apSuite, !delegate.mReadError); + + delegate.mGotEventResponse = false; + delegate.mNumAttributeResponse = 0; + delegate.mGotReport = false; + err = chip::app::InteractionModelEngine::GetInstance()->SendReadRequest(readPrepareParams, &delegate); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + InteractionModelEngine::GetInstance()->GetReportingEngine().Run(); + NL_TEST_ASSERT(apSuite, delegate.mGotEventResponse); + NL_TEST_ASSERT(apSuite, delegate.mNumAttributeResponse == 2); + NL_TEST_ASSERT(apSuite, delegate.mGotReport); + NL_TEST_ASSERT(apSuite, !delegate.mReadError); + // By now we should have closed all exchanges and sent all pending acks, so // there should be no queued-up things in the retransmit table. NL_TEST_ASSERT(apSuite, rm->TestGetCountRetransTable() == 0); @@ -1029,6 +1042,7 @@ void TestReadInteraction::TestSubscribeRoundtrip(nlTestSuite * apSuite, void * a NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); delegate.mGotReport = false; engine->GetReportingEngine().Run(); + NL_TEST_ASSERT(apSuite, delegate.mGotEventResponse); NL_TEST_ASSERT(apSuite, delegate.mGotReport); NL_TEST_ASSERT(apSuite, delegate.mNumAttributeResponse == 2); NL_TEST_ASSERT(apSuite, delegate.mNumSubscriptions == 1); @@ -1124,7 +1138,8 @@ void TestReadInteraction::TestSubscribeRoundtrip(nlTestSuite * apSuite, void * a engine->GetReportingEngine().Run(); NL_TEST_ASSERT(apSuite, delegate.mNumAttributeResponse == 0); - // Test multiple subscriptipn + // Test multiple subscription + delegate.mGotEventResponse = false; delegate.mNumAttributeResponse = 0; delegate.mGotReport = false; ReadPrepareParams readPrepareParams1(ctx.GetSessionBobToAlice());