Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 16 additions & 3 deletions src/app/InteractionModelEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -168,8 +168,15 @@ class AutoReleaseSubscriptionInfoIterator
{
public:
AutoReleaseSubscriptionInfoIterator(SubscriptionResumptionStorage::SubscriptionInfoIterator * iterator) : mIterator(iterator){};
~AutoReleaseSubscriptionInfoIterator() { mIterator->Release(); }
~AutoReleaseSubscriptionInfoIterator()
{
if (mIterator)
{
mIterator->Release();
}
}

explicit operator bool() const { return mIterator != nullptr; }
SubscriptionResumptionStorage::SubscriptionInfoIterator * operator->() const { return mIterator; }

private:
Expand Down Expand Up @@ -777,6 +784,7 @@ Protocols::InteractionModel::Status InteractionModelEngine::OnReadInitialRequest
{
SubscriptionResumptionStorage::SubscriptionInfo subscriptionInfo;
auto * iterator = mpSubscriptionResumptionStorage->IterateSubscriptions();
VerifyOrReturnError(nullptr != iterator, Status::ResourceExhausted);

while (iterator->Next(subscriptionInfo))
{
Expand Down Expand Up @@ -2098,9 +2106,11 @@ CHIP_ERROR InteractionModelEngine::ResumeSubscriptions()
// future improvements: https://github.com/project-chip/connectedhomeip/issues/25439

SubscriptionResumptionStorage::SubscriptionInfo subscriptionInfo;
auto * iterator = mpSubscriptionResumptionStorage->IterateSubscriptions();
mNumOfSubscriptionsToResume = 0;
uint16_t minInterval = 0;

auto * iterator = mpSubscriptionResumptionStorage->IterateSubscriptions();
VerifyOrReturnError(nullptr != iterator, CHIP_ERROR_NO_MEMORY);
while (iterator->Next(subscriptionInfo))
{
mNumOfSubscriptionsToResume++;
Expand Down Expand Up @@ -2137,6 +2147,7 @@ void InteractionModelEngine::ResumeSubscriptionsTimerCallback(System::Layer * ap
#endif // CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
SubscriptionResumptionStorage::SubscriptionInfo subscriptionInfo;
AutoReleaseSubscriptionInfoIterator iterator(imEngine->mpSubscriptionResumptionStorage->IterateSubscriptions());
VerifyOrReturn(iterator, ChipLogError(InteractionModel, "Failed to allocate subscription resumption iterator"));
while (iterator->Next(subscriptionInfo))
{
// If subscription happens between reboot and this timer callback, it's already live and should skip resumption
Expand Down Expand Up @@ -2209,7 +2220,9 @@ bool InteractionModelEngine::HasSubscriptionsToResume()

// Look through persisted subscriptions and see if any aren't already in mReadHandlers pool
SubscriptionResumptionStorage::SubscriptionInfo subscriptionInfo;
auto * iterator = mpSubscriptionResumptionStorage->IterateSubscriptions();
auto * iterator = mpSubscriptionResumptionStorage->IterateSubscriptions();
// Conservatively assume there may be subscriptions to resume if we cannot get an iterator.
VerifyOrReturnValue(iterator != nullptr, true);
bool foundSubscriptionToResume = false;
while (iterator->Next(subscriptionInfo))
{
Expand Down
3 changes: 3 additions & 0 deletions src/app/SubscriptionResumptionStorage.h
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,9 @@ class SubscriptionResumptionStorage
* Iterate through persisted subscriptions
*
* @return A valid iterator on success. Use CommonIterator accessor to retrieve SubscriptionInfo
*
* Warning: Implementations using fixed-size pools may return nullptr if the iterator pool is exhausted.
* Callers should check for nullptr before using the iterator.
*/
virtual SubscriptionInfoIterator * IterateSubscriptions() = 0;

Expand Down
51 changes: 51 additions & 0 deletions src/app/tests/TestInteractionModelEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ class TestInteractionModelEngine : public chip::Test::AppContext
void TestSubjectHasActiveSubscriptionSubWithCAT();
void TestSubscriptionResumptionTimer();
void TestDecrementNumSubscriptionsToResume();
void TestHasSubscriptionsToResumeHandlesNullIterator();
void TestFabricHasAtLeastOneActiveSubscription();
void TestFabricHasAtLeastOneActiveSubscriptionWithMixedStates();
static int GetAttributePathListLength(SingleLinkedListNode<AttributePathParams> * apattributePathParamsList);
Expand Down Expand Up @@ -830,5 +831,55 @@ TEST_F_FROM_FIXTURE(TestInteractionModelEngine, TestFabricHasAtLeastOneActiveSub
EXPECT_FALSE(engine->FabricHasAtLeastOneActiveSubscription(fabricIndex));
}

#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS

// Mock SubscriptionResumptionStorage that always returns nullptr from IterateSubscriptions(). Used to test null-check handling in
// the InteractionModelEngine.
class NullIteratorSubscriptionResumptionStorage : public chip::app::SubscriptionResumptionStorage
{
public:
SubscriptionInfoIterator * IterateSubscriptions() override { return nullptr; }
CHIP_ERROR Save(SubscriptionInfo & subscriptionInfo) override { return CHIP_NO_ERROR; }
CHIP_ERROR Delete(chip::NodeId nodeId, chip::FabricIndex fabricIndex, chip::SubscriptionId subscriptionId) override
{
return CHIP_NO_ERROR;
}
CHIP_ERROR DeleteAll(chip::FabricIndex fabricIndex) override { return CHIP_NO_ERROR; }
};

// Test verifies that ResumeSubscriptions handles a nullptr iterator gracefully by returning CHIP_ERROR_NO_MEMORY and not crashing
TEST_F(TestInteractionModelEngine, TestResumeSubscriptionsHandlesNullIterators)
{
InteractionModelEngine * engine = InteractionModelEngine::GetInstance();

NullIteratorSubscriptionResumptionStorage nullIteratorStorage;

engine->SetDataModelProvider(CodegenDataModelProviderInstance(nullptr /* delegate */));
EXPECT_EQ(CHIP_NO_ERROR,
engine->Init(&GetExchangeManager(), &GetFabricTable(), app::reporting::GetDefaultReportScheduler(), nullptr,
&nullIteratorStorage));

EXPECT_EQ(engine->ResumeSubscriptions(), CHIP_ERROR_NO_MEMORY);
}

#if CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION

// Test verifies that HasSubscriptionsToResume handles a nullptr iterator gracefully by returning true and not crashing.
TEST_F_FROM_FIXTURE(TestInteractionModelEngine, TestHasSubscriptionsToResumeHandlesNullIterator)
{
InteractionModelEngine * engine = InteractionModelEngine::GetInstance();

NullIteratorSubscriptionResumptionStorage nullIteratorStorage;

engine->SetDataModelProvider(CodegenDataModelProviderInstance(nullptr /* delegate */));
EXPECT_EQ(CHIP_NO_ERROR,
engine->Init(&GetExchangeManager(), &GetFabricTable(), app::reporting::GetDefaultReportScheduler(), nullptr,
&nullIteratorStorage));

EXPECT_TRUE(engine->HasSubscriptionsToResume());
}
#endif // CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
#endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS

} // namespace app
} // namespace chip
Loading