Skip to content

Commit a1c6f42

Browse files
Moved the IM shutdown before setting data model provider to null in the
server shutdown sequence.
1 parent cc9c41d commit a1c6f42

File tree

2 files changed

+39
-27
lines changed

2 files changed

+39
-27
lines changed

src/app/InteractionModelEngine.h

Lines changed: 33 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,39 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
340340
#endif // CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION
341341
#endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
342342

343+
/** Previously, ShutdowActiveReads() was only available in unit test builds via `#if CONFIG_BUILD_FOR_HOST_UNIT_TEST`.
344+
* However, we now require proper cleanup of active read handlers during the application's shutdown sequence,
345+
* especially before calling SetDataModelProvider(nullptr). This ensures that there are no ongoing IM reads,
346+
* which could otherwise result in crashes or undefined behavior.
347+
348+
* Therefore, this method has been moved out of the CONFIG_BUILD_FOR_HOST_UNIT_TEST block so that it can be
349+
* called as part of the normal shutdown flow.
350+
351+
* Note: This was originally introduced for unit tests to clean up subscriptions created via the high-level
352+
* APIs in src/controller/ReadInteraction.h, which did not expose a way to shut down active subscriptions.
353+
**/
354+
355+
void ShutdownActiveReads()
356+
{
357+
#if CHIP_CONFIG_ENABLE_READ_CLIENT
358+
for (auto * readClient = mpActiveReadClientList; readClient != nullptr;)
359+
{
360+
readClient->mpImEngine = nullptr;
361+
auto * tmpClient = readClient->GetNextClient();
362+
readClient->SetNextClient(nullptr);
363+
readClient->Close(CHIP_NO_ERROR);
364+
readClient = tmpClient;
365+
}
366+
367+
//
368+
// After that, we just null out our tracker.
369+
//
370+
mpActiveReadClientList = nullptr;
371+
#endif // CHIP_CONFIG_ENABLE_READ_CLIENT
372+
373+
mReadHandlers.ReleaseAll();
374+
}
375+
343376
#if CONFIG_BUILD_FOR_HOST_UNIT_TEST
344377
//
345378
// Get direct access to the underlying read handler pool
@@ -391,33 +424,6 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
391424
mSubscriptionResumptionRetrySecondsOverride = seconds;
392425
}
393426
#endif
394-
395-
//
396-
// When testing subscriptions using the high-level APIs in src/controller/ReadInteraction.h,
397-
// they don't provide for the ability to shut down those subscriptions after they've been established.
398-
//
399-
// So for the purposes of unit tests, add a helper here to shut down and clean-up all active handlers.
400-
//
401-
void ShutdownActiveReads()
402-
{
403-
#if CHIP_CONFIG_ENABLE_READ_CLIENT
404-
for (auto * readClient = mpActiveReadClientList; readClient != nullptr;)
405-
{
406-
readClient->mpImEngine = nullptr;
407-
auto * tmpClient = readClient->GetNextClient();
408-
readClient->SetNextClient(nullptr);
409-
readClient->Close(CHIP_NO_ERROR);
410-
readClient = tmpClient;
411-
}
412-
413-
//
414-
// After that, we just null out our tracker.
415-
//
416-
mpActiveReadClientList = nullptr;
417-
#endif // CHIP_CONFIG_ENABLE_READ_CLIENT
418-
419-
mReadHandlers.ReleaseAll();
420-
}
421427
#endif
422428

423429
DataModel::Provider * GetDataModelProvider() const;

src/app/server/Server.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -652,6 +652,12 @@ void Server::Shutdown()
652652
app::DnssdServer::Instance().SetICDManager(nullptr);
653653
#endif // CHIP_CONFIG_ENABLE_ICD_SERVER
654654

655+
/** Shutdown all active read handlers to ensure a clean shutdown of the Interaction Model.
656+
* This is required before resetting the DataModelProvider, as ongoing reads may access it
657+
* and lead to crashes or undefined behavior.
658+
**/
659+
app::InteractionModelEngine::GetInstance()->ShutdownActiveReads();
660+
655661
app::InteractionModelEngine::GetInstance()->SetDataModelProvider(nullptr);
656662
app::DnssdServer::Instance().SetCommissioningModeProvider(nullptr);
657663
Dnssd::ServiceAdvertiser::Instance().Shutdown();

0 commit comments

Comments
 (0)