Fix mSelfProcessedEvents init/cleanup bug (#12829)

project-chip · Dec 16, 2021 · 7484323 · 7484323
1 parent 5767507
commit 7484323
Show file tree

Hide file tree

Showing 4 changed files with 40 additions and 16 deletions.
diff --git 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<EventLoadOutContext *>(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
@@ -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
@@ -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
@@ -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());