diff --git a/src/app/InteractionModelEngine.cpp b/src/app/InteractionModelEngine.cpp index 68c9ae8a1129ef..bd9219b949da55 100644 --- a/src/app/InteractionModelEngine.cpp +++ b/src/app/InteractionModelEngine.cpp @@ -742,7 +742,7 @@ bool InteractionModelEngine::EnsureResourceForSubscription(FabricIndex aFabricIn return true; } -bool InteractionModelEngine::CanEstablishReadTransaction(const ReadHandler * apReadHandler) +CHIP_ERROR InteractionModelEngine::CanEstablishReadTransaction(const ReadHandler * apReadHandler) { #if CHIP_SYSTEM_CONFIG_POOL_USE_HEAP && !CHIP_CONFIG_IM_FORCE_FABRIC_QUOTA_CHECK #if CONFIG_IM_BUILD_FOR_UNIT_TEST @@ -759,6 +759,15 @@ bool InteractionModelEngine::CanEstablishReadTransaction(const ReadHandler * apR size_t activeReadHandlersOnCurrentFabric = 0; // It is safe to use & here since this function will be called on current stack. + if (apReadHandler->GetAttributePathCount() > kReservedPathsPerReadRequest || + apReadHandler->GetEventPathCount() > kReservedPathsPerReadRequest || + apReadHandler->GetDataVersionFilterCount() > kReservedPathsPerReadRequest) + { + // Doesn't matter how many other reads are in progress; we are not + // going to handle this request. + return (allowUnlimited ? CHIP_NO_ERROR : CHIP_IM_GLOBAL_STATUS(PathsExhausted)); + } + mReadHandlers.ForEachActiveObject([&](ReadHandler * handler) { if (handler->GetAccessingFabricIndex() == currentFabricIndex && handler->IsType(ReadHandler::InteractionType::Read)) { @@ -770,13 +779,10 @@ bool InteractionModelEngine::CanEstablishReadTransaction(const ReadHandler * apR // The incoming read handler here is also counted above. if (activeReadHandlersOnCurrentFabric > kReservedReadHandlersPerFabricForReadRequests) { - return allowUnlimited; + return (allowUnlimited ? CHIP_NO_ERROR : CHIP_IM_GLOBAL_STATUS(Busy)); } - return (apReadHandler->GetAttributePathCount() <= kReservedPathsPerReadRequest && - apReadHandler->GetEventPathCount() <= kReservedPathsPerReadRequest && - apReadHandler->GetDataVersionFilterCount() <= kReservedPathsPerReadRequest) || - allowUnlimited; + return CHIP_NO_ERROR; } void InteractionModelEngine::RemoveReadClient(ReadClient * apReadClient) diff --git a/src/app/InteractionModelEngine.h b/src/app/InteractionModelEngine.h index bf344511a7780a..cf825f32682044 100644 --- a/src/app/InteractionModelEngine.h +++ b/src/app/InteractionModelEngine.h @@ -244,10 +244,15 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler, * kReservedPathsPerReadRequest. This function will check if the given ReadHandler will exceed the limitations for the accessing * fabric. * + * If CHIP_NO_ERROR is returned, it's OK to proceed with the read. + * + * Otherwise the CHIP_ERROR encodes an interaction model status that needs + * to turn into a Status Response to the client. + * * TODO: (#17418) We are now reserving resources for read requests, could be changed to similar algorithm for read resources * minimas. */ - bool CanEstablishReadTransaction(const ReadHandler * apReadHandler); + CHIP_ERROR CanEstablishReadTransaction(const ReadHandler * apReadHandler); /** * Select the oldest (and the one that exceeds the per subscription resource minimum if there are any) read handler on the diff --git a/src/app/ReadHandler.cpp b/src/app/ReadHandler.cpp index c02d9d222f6e1a..ee4be7d293fa64 100644 --- a/src/app/ReadHandler.cpp +++ b/src/app/ReadHandler.cpp @@ -384,8 +384,9 @@ CHIP_ERROR ReadHandler::ProcessReadRequest(System::PacketBufferHandle && aPayloa } ReturnErrorOnFailure(err); - // Ensure the read transaction doesn't exceed the resources dedicated to read transactions. - VerifyOrReturnError(InteractionModelEngine::GetInstance()->CanEstablishReadTransaction(this), CHIP_ERROR_NO_MEMORY); + // Ensure the read transaction doesn't exceed the resources dedicated to + // read transactions. + ReturnErrorOnFailure(InteractionModelEngine::GetInstance()->CanEstablishReadTransaction(this)); bool isFabricFiltered; ReturnErrorOnFailure(readRequestParser.GetIsFabricFiltered(&isFabricFiltered)); diff --git a/src/controller/tests/data_model/TestRead.cpp b/src/controller/tests/data_model/TestRead.cpp index 8bc04d723a6460..a680dd63f25cea 100644 --- a/src/controller/tests/data_model/TestRead.cpp +++ b/src/controller/tests/data_model/TestRead.cpp @@ -188,6 +188,9 @@ class TestReadInteraction : public app::ReadHandler::ApplicationCallback static void TestReadSubscribeAttributeResponseWithCache(nlTestSuite * apSuite, void * apContext); static void TestReadHandler_KillOverQuotaSubscriptions(nlTestSuite * apSuite, void * apContext); static void TestReadHandler_KillOldestSubscriptions(nlTestSuite * apSuite, void * apContext); + static void TestReadHandler_TwoParallelReads(nlTestSuite * apSuite, void * apContext); + static void TestReadHandler_TooManyPaths(nlTestSuite * apSuite, void * apContext); + static void TestReadHandler_TwoParallelReadsSecondTooManyPaths(nlTestSuite * apSuite, void * apContext); private: static constexpr uint16_t kTestMinInterval = 33; @@ -1823,6 +1826,168 @@ void TestReadInteraction::TestReadHandler_KillOldestSubscriptions(nlTestSuite * app::InteractionModelEngine::GetInstance()->SetPathPoolCapacityForSubscriptions(-1); } +void TestReadInteraction::TestReadHandler_TwoParallelReads(nlTestSuite * apSuite, void * apContext) +{ + using namespace chip::app; + + TestContext & ctx = *static_cast(apContext); + + chip::Messaging::ReliableMessageMgr * rm = ctx.GetExchangeManager().GetReliableMessageMgr(); + // Shouldn't have anything in the retransmit table when starting the test. + NL_TEST_ASSERT(apSuite, rm->TestGetCountRetransTable() == 0); + + auto * engine = app::InteractionModelEngine::GetInstance(); + engine->SetForceHandlerQuota(true); + + app::ReadPrepareParams readPrepareParams(ctx.GetSessionBobToAlice()); + // Read full wildcard paths, repeat twice to ensure chunking. + chip::app::AttributePathParams attributePathParams[2]; + readPrepareParams.mpAttributePathParamsList = attributePathParams; + readPrepareParams.mAttributePathParamsListSize = ArraySize(attributePathParams); + + { + MockInteractionModelApp delegate1; + NL_TEST_ASSERT(apSuite, delegate1.mNumAttributeResponse == 0); + NL_TEST_ASSERT(apSuite, !delegate1.mReadError); + app::ReadClient readClient1(InteractionModelEngine::GetInstance(), &ctx.GetExchangeManager(), delegate1, + ReadClient::InteractionType::Read); + + MockInteractionModelApp delegate2; + NL_TEST_ASSERT(apSuite, delegate2.mNumAttributeResponse == 0); + NL_TEST_ASSERT(apSuite, !delegate2.mReadError); + ReadClient readClient2(InteractionModelEngine::GetInstance(), &ctx.GetExchangeManager(), delegate2, + ReadClient::InteractionType::Read); + + CHIP_ERROR err = readClient1.SendRequest(readPrepareParams); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + err = readClient2.SendRequest(readPrepareParams); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + ctx.DrainAndServiceIO(); + + NL_TEST_ASSERT(apSuite, delegate1.mNumAttributeResponse != 0); + NL_TEST_ASSERT(apSuite, !delegate1.mReadError); + + NL_TEST_ASSERT(apSuite, delegate2.mNumAttributeResponse == 0); + NL_TEST_ASSERT(apSuite, delegate2.mReadError); + + StatusIB status(delegate2.mError); + NL_TEST_ASSERT(apSuite, status.mStatus == Protocols::InteractionModel::Status::Busy); + } + + NL_TEST_ASSERT(apSuite, engine->GetNumActiveReadClients() == 0); + NL_TEST_ASSERT(apSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0); + engine->SetForceHandlerQuota(false); +} + +// Needs to be larger than our plausible path pool. +constexpr size_t sTooLargePathCount = 200; + +void TestReadInteraction::TestReadHandler_TooManyPaths(nlTestSuite * apSuite, void * apContext) +{ + using namespace chip::app; + + TestContext & ctx = *static_cast(apContext); + + chip::Messaging::ReliableMessageMgr * rm = ctx.GetExchangeManager().GetReliableMessageMgr(); + // Shouldn't have anything in the retransmit table when starting the test. + NL_TEST_ASSERT(apSuite, rm->TestGetCountRetransTable() == 0); + + auto * engine = InteractionModelEngine::GetInstance(); + engine->SetForceHandlerQuota(true); + + ReadPrepareParams readPrepareParams(ctx.GetSessionBobToAlice()); + // Needs to be larger than our plausible path pool. + chip::app::AttributePathParams attributePathParams[sTooLargePathCount]; + readPrepareParams.mpAttributePathParamsList = attributePathParams; + readPrepareParams.mAttributePathParamsListSize = ArraySize(attributePathParams); + + { + MockInteractionModelApp delegate; + NL_TEST_ASSERT(apSuite, delegate.mNumAttributeResponse == 0); + NL_TEST_ASSERT(apSuite, !delegate.mReadError); + ReadClient readClient(InteractionModelEngine::GetInstance(), &ctx.GetExchangeManager(), delegate, + ReadClient::InteractionType::Read); + + CHIP_ERROR err = readClient.SendRequest(readPrepareParams); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + ctx.DrainAndServiceIO(); + + NL_TEST_ASSERT(apSuite, delegate.mNumAttributeResponse == 0); + NL_TEST_ASSERT(apSuite, delegate.mReadError); + + StatusIB status(delegate.mError); + NL_TEST_ASSERT(apSuite, status.mStatus == Protocols::InteractionModel::Status::PathsExhausted); + } + + NL_TEST_ASSERT(apSuite, engine->GetNumActiveReadClients() == 0); + NL_TEST_ASSERT(apSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0); + engine->SetForceHandlerQuota(false); +} + +void TestReadInteraction::TestReadHandler_TwoParallelReadsSecondTooManyPaths(nlTestSuite * apSuite, void * apContext) +{ + using namespace chip::app; + + TestContext & ctx = *static_cast(apContext); + + chip::Messaging::ReliableMessageMgr * rm = ctx.GetExchangeManager().GetReliableMessageMgr(); + // Shouldn't have anything in the retransmit table when starting the test. + NL_TEST_ASSERT(apSuite, rm->TestGetCountRetransTable() == 0); + + auto * engine = InteractionModelEngine::GetInstance(); + engine->SetForceHandlerQuota(true); + + { + MockInteractionModelApp delegate1; + NL_TEST_ASSERT(apSuite, delegate1.mNumAttributeResponse == 0); + NL_TEST_ASSERT(apSuite, !delegate1.mReadError); + ReadClient readClient1(InteractionModelEngine::GetInstance(), &ctx.GetExchangeManager(), delegate1, + ReadClient::InteractionType::Read); + + MockInteractionModelApp delegate2; + NL_TEST_ASSERT(apSuite, delegate2.mNumAttributeResponse == 0); + NL_TEST_ASSERT(apSuite, !delegate2.mReadError); + ReadClient readClient2(InteractionModelEngine::GetInstance(), &ctx.GetExchangeManager(), delegate2, + ReadClient::InteractionType::Read); + + ReadPrepareParams readPrepareParams1(ctx.GetSessionBobToAlice()); + // Read full wildcard paths, repeat twice to ensure chunking. + chip::app::AttributePathParams attributePathParams1[2]; + readPrepareParams1.mpAttributePathParamsList = attributePathParams1; + readPrepareParams1.mAttributePathParamsListSize = ArraySize(attributePathParams1); + + CHIP_ERROR err = readClient1.SendRequest(readPrepareParams1); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + ReadPrepareParams readPrepareParams2(ctx.GetSessionBobToAlice()); + // Read full wildcard paths, repeat twice to ensure chunking. + chip::app::AttributePathParams attributePathParams2[sTooLargePathCount]; + readPrepareParams2.mpAttributePathParamsList = attributePathParams2; + readPrepareParams2.mAttributePathParamsListSize = ArraySize(attributePathParams2); + + err = readClient2.SendRequest(readPrepareParams2); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + ctx.DrainAndServiceIO(); + + NL_TEST_ASSERT(apSuite, delegate1.mNumAttributeResponse != 0); + NL_TEST_ASSERT(apSuite, !delegate1.mReadError); + + NL_TEST_ASSERT(apSuite, delegate2.mNumAttributeResponse == 0); + NL_TEST_ASSERT(apSuite, delegate2.mReadError); + + StatusIB status(delegate2.mError); + NL_TEST_ASSERT(apSuite, status.mStatus == Protocols::InteractionModel::Status::PathsExhausted); + } + + NL_TEST_ASSERT(apSuite, engine->GetNumActiveReadClients() == 0); + NL_TEST_ASSERT(apSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0); + engine->SetForceHandlerQuota(false); +} + // clang-format off const nlTest sTests[] = { @@ -1845,6 +2010,9 @@ const nlTest sTests[] = NL_TEST_DEF("TestReadSubscribeAttributeResponseWithCache", TestReadInteraction::TestReadSubscribeAttributeResponseWithCache), NL_TEST_DEF("TestReadHandler_KillOverQuotaSubscriptions", TestReadInteraction::TestReadHandler_KillOverQuotaSubscriptions), NL_TEST_DEF("TestReadHandler_KillOldestSubscriptions", TestReadInteraction::TestReadHandler_KillOldestSubscriptions), + NL_TEST_DEF("TestReadHandler_TwoParallelReads", TestReadInteraction::TestReadHandler_TwoParallelReads), + NL_TEST_DEF("TestReadHandler_TooManyPaths", TestReadInteraction::TestReadHandler_TooManyPaths), + NL_TEST_DEF("TestReadHandler_TwoParallelReadsSecondTooManyPaths", TestReadInteraction::TestReadHandler_TwoParallelReadsSecondTooManyPaths), NL_TEST_SENTINEL() }; // clang-format on