Fix the status codes returned from failed Read interactions.

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)

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

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));

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<TestContext *>(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<TestContext *>(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<TestContext *>(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