From e6ef7e9f7ec4c8f85f6f55a9091e9bdd7ec0c01e Mon Sep 17 00:00:00 2001 From: Damian Krolik Date: Fri, 29 Oct 2021 19:28:01 +0200 Subject: [PATCH] [app] Forget exchange context when SubscribeResponse is received SubscribeResponse is the last action in the Subscribe transaction and ExchangeContext::HandleMessage automatically closes exchanges that have no pending requests nor responses. ReadClient still kept the pointer to the exchange after receiving SubscribeResponse, so it would attempt to abort already closed exchange if the subscription was cancelled after receiving the SubscribeResponse and before receving a subsequent Report message. --- src/app/ReadClient.cpp | 4 +++ src/app/tests/TestReadInteraction.cpp | 52 +++++++++++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/src/app/ReadClient.cpp b/src/app/ReadClient.cpp index 67fb6b783c87c9..a77794af73f7d9 100644 --- a/src/app/ReadClient.cpp +++ b/src/app/ReadClient.cpp @@ -295,6 +295,10 @@ CHIP_ERROR ReadClient::OnMessageReceived(Messaging::ExchangeContext * apExchange { VerifyOrExit(apExchangeContext == mpExchangeCtx, err = CHIP_ERROR_INCORRECT_STATE); err = ProcessSubscribeResponse(std::move(aPayload)); + // Forget the context as SUBSCRIBE RESPONSE is the last message in SUBSCRIBE transaction and + // ExchangeContext::HandleMessage automatically closes a context if no other messages need to + // be sent or received. + mpExchangeCtx = nullptr; SuccessOrExit(err); } else diff --git a/src/app/tests/TestReadInteraction.cpp b/src/app/tests/TestReadInteraction.cpp index f53a58eae0cb21..f5db2d8e039437 100644 --- a/src/app/tests/TestReadInteraction.cpp +++ b/src/app/tests/TestReadInteraction.cpp @@ -266,6 +266,7 @@ class TestReadInteraction static void TestProcessSubscribeRequest(nlTestSuite * apSuite, void * apContext); static void TestReadRoundtrip(nlTestSuite * apSuite, void * apContext); static void TestSubscribeRoundtrip(nlTestSuite * apSuite, void * apContext); + static void TestSubscribeEarlyShutdown(nlTestSuite * apSuite, void * apContext); static void TestSubscribeInvalidAttributePathRoundtrip(nlTestSuite * apSuite, void * apContext); static void TestReadInvalidAttributePathRoundtrip(nlTestSuite * apSuite, void * apContext); @@ -1037,6 +1038,56 @@ void TestReadInteraction::TestSubscribeRoundtrip(nlTestSuite * apSuite, void * a engine->Shutdown(); } +// Verify that subscription can be shut down just after receiving SUBSCRIBE RESPONSE, +// before receiving any subsequent REPORT DATA. +void TestReadInteraction::TestSubscribeEarlyShutdown(nlTestSuite * apSuite, void * apContext) +{ + TestContext & ctx = *static_cast(apContext); + Messaging::ReliableMessageMgr * rm = ctx.GetExchangeManager().GetReliableMessageMgr(); + InteractionModelEngine & engine = *InteractionModelEngine::GetInstance(); + MockInteractionModelApp delegate; + + // Initialize Interaction Model Engine + NL_TEST_ASSERT(apSuite, rm->TestGetCountRetransTable() == 0); + NL_TEST_ASSERT(apSuite, engine.Init(&ctx.GetExchangeManager(), &delegate) == CHIP_NO_ERROR); + + // Subscribe to the attribute + AttributePathParams attributePathParams; + attributePathParams.mNodeId = chip::kTestDeviceNodeId; + attributePathParams.mEndpointId = kTestEndpointId; + attributePathParams.mClusterId = kTestClusterId; + attributePathParams.mFieldId = 1; + attributePathParams.mListIndex = 0; + attributePathParams.mFlags.Set(AttributePathParams::Flags::kFieldIdValid); + + ReadPrepareParams readPrepareParams(ctx.GetSessionBobToAlice()); + readPrepareParams.mpAttributePathParamsList = &attributePathParams; + readPrepareParams.mAttributePathParamsListSize = 1; + readPrepareParams.mMinIntervalFloorSeconds = 2; + readPrepareParams.mMaxIntervalCeilingSeconds = 5; + readPrepareParams.mKeepSubscriptions = false; + + printf("Send subscribe request message to Node: %" PRIu64 "\n", chip::kTestDeviceNodeId); + NL_TEST_ASSERT(apSuite, engine.SendSubscribeRequest(readPrepareParams, &delegate) == CHIP_NO_ERROR); + + engine.GetReportingEngine().Run(); + NL_TEST_ASSERT(apSuite, delegate.mGotReport); + NL_TEST_ASSERT(apSuite, delegate.mNumAttributeResponse == 1); + NL_TEST_ASSERT(apSuite, delegate.mNumSubscriptions == 1); + NL_TEST_ASSERT(apSuite, delegate.mpReadHandler != nullptr); + + // Shutdown the subscription + uint64_t subscriptionId = 0; + delegate.mpReadHandler->GetSubscriptionId(subscriptionId); + + NL_TEST_ASSERT(apSuite, subscriptionId != 0); + NL_TEST_ASSERT(apSuite, engine.ShutdownSubscription(subscriptionId) == CHIP_NO_ERROR); + + // Cleanup + NL_TEST_ASSERT(apSuite, rm->TestGetCountRetransTable() == 0); + engine.Shutdown(); +} + void TestReadInteraction::TestSubscribeInvalidAttributePathRoundtrip(nlTestSuite * apSuite, void * apContext) { TestContext & ctx = *static_cast(apContext); @@ -1106,6 +1157,7 @@ const nlTest sTests[] = NL_TEST_DEF("TestProcessSubscribeResponse", chip::app::TestReadInteraction::TestProcessSubscribeResponse), NL_TEST_DEF("TestProcessSubscribeRequest", chip::app::TestReadInteraction::TestProcessSubscribeRequest), NL_TEST_DEF("TestSubscribeRoundtrip", chip::app::TestReadInteraction::TestSubscribeRoundtrip), + NL_TEST_DEF("TestSubscribeEarlyShutdown", chip::app::TestReadInteraction::TestSubscribeEarlyShutdown), NL_TEST_DEF("TestSubscribeInvalidAttributePathRoundtrip", chip::app::TestReadInteraction::TestSubscribeInvalidAttributePathRoundtrip), NL_TEST_DEF("TestReadInvalidAttributePathRoundtrip", chip::app::TestReadInteraction::TestReadInvalidAttributePathRoundtrip), NL_TEST_SENTINEL()