Revert "Fix: Null mContext crash when commissioning after background/foreground (#43127)" (#43416)

This reverts commit ead8174.

Author: andy31415

examples/tv-casting-app/darwin/MatterTvCastingBridge/MatterTvCastingBridge/MCCastingApp.mm

@@ -46,9 +46,6 @@ @interface MCCastingApp ()
 // Client defiend data source used to initialize the MCCommissionableDataProvider and, if needed, update the MCCommissionableDataProvider post initialization. This is necessary for the Commissioner-Generated passcode commissioning feature.
 @property (nonatomic, strong) id<MCDataSource> dataSource;
 
-// Track whether this is the first start (cold boot) or subsequent start (warm boot)
-@property (atomic) BOOL hasStartedBefore;
-
 @end
 
 @implementation MCCastingApp
@@ -132,61 +129,32 @@ - (NSError *)updateCommissionableDataProvider
 
 - (void)startWithCompletionBlock:(void (^)(NSError *))completion
 {
-    ChipLogProgress(AppServer, "MCCastingApp.startWithCompletionBlock called (%s start)", self.hasStartedBefore ? "warm" : "cold");
-    self.hasStartedBefore = YES;
-
+    ChipLogProgress(AppServer, "MCCastingApp.startWithCompletionBlock called");
     VerifyOrReturn(_workQueue != nil && _clientQueue != nil, dispatch_async(self->_clientQueue, ^{
-        ChipLogError(AppServer, "MCCastingApp.startWithCompletionBlock failed: work queue or client queue is nil");
         completion([MCErrorUtils NSErrorFromChipError:CHIP_ERROR_INCORRECT_STATE]);
     }));
 
-    // Start event loop task FIRST (synchronously) to ensure proper state
-    __block CHIP_ERROR err = chip::DeviceLayer::PlatformMgrImpl().StartEventLoopTask();
-    if (err != CHIP_NO_ERROR) {
-        ChipLogError(AppServer, "MCCastingApp.startWithCompletionBlock StartEventLoopTask failed: %s", err.AsString());
-        dispatch_async(self->_clientQueue, ^{
-            completion([MCErrorUtils NSErrorFromChipError:err]);
-        });
-        // Early return to prevent double completion call
-        return;
-    }
-
-    // Only then start the casting app (on work queue)
     dispatch_async(_workQueue, ^{
-        CHIP_ERROR startErr = matter::casting::core::CastingApp::GetInstance()->Start();
-        if (startErr != CHIP_NO_ERROR) {
-            ChipLogError(AppServer, "MCCastingApp.startWithCompletionBlock CastingApp::Start failed: %s", startErr.AsString());
-        }
-
+        __block CHIP_ERROR err = matter::casting::core::CastingApp::GetInstance()->Start();
         dispatch_async(self->_clientQueue, ^{
-            completion([MCErrorUtils NSErrorFromChipError:startErr]);
+            completion([MCErrorUtils NSErrorFromChipError:err]);
         });
     });
+    __block CHIP_ERROR err = chip::DeviceLayer::PlatformMgrImpl().StartEventLoopTask();
+    VerifyOrReturn(err == CHIP_NO_ERROR, dispatch_async(self->_clientQueue, ^{
+        completion([MCErrorUtils NSErrorFromChipError:err]);
+    }));
 }
 
 - (void)stopWithCompletionBlock:(void (^)(NSError *))completion
 {
     ChipLogProgress(AppServer, "MCCastingApp.stopWithCompletionBlock called");
     VerifyOrReturn(_workQueue != nil && _clientQueue != nil, dispatch_async(self->_clientQueue, ^{
-        ChipLogError(AppServer, "MCCastingApp.stopWithCompletionBlock failed: work queue or client queue is nil");
         completion([MCErrorUtils NSErrorFromChipError:CHIP_ERROR_INCORRECT_STATE]);
     }));
 
     dispatch_async(_workQueue, ^{
-        // Stop the casting app first
-        CHIP_ERROR err = matter::casting::core::CastingApp::GetInstance()->Stop();
-        if (err != CHIP_NO_ERROR) {
-            ChipLogError(AppServer, "MCCastingApp.stopWithCompletionBlock CastingApp::Stop failed: %s", err.AsString());
-        }
-
-        // Then stop the event loop task to transition WorkQueue to suspended state
-        CHIP_ERROR stopEventLoopErr = chip::DeviceLayer::PlatformMgrImpl().StopEventLoopTask();
-        if (stopEventLoopErr != CHIP_NO_ERROR) {
-            ChipLogError(AppServer, "MCCastingApp.stopWithCompletionBlock StopEventLoopTask failed: %s", stopEventLoopErr.AsString());
-            if (err == CHIP_NO_ERROR) {
-                err = stopEventLoopErr;
-            }
-        }
+        __block CHIP_ERROR err = matter::casting::core::CastingApp::GetInstance()->Stop();
 
         dispatch_async(self->_clientQueue, ^{
             completion([MCErrorUtils NSErrorFromChipError:err]);

examples/tv-casting-app/tv-casting-common/core/CastingApp.cpp

@@ -24,13 +24,11 @@
 #include "support/ChipDeviceEventHandler.h"
 
 #include <DeviceInfoProviderImpl.h>
-#include <app/EventManagement.h>
 #include <app/InteractionModelEngine.h>
 #include <app/clusters/bindings/BindingManager.h>
 #include <app/server/Server.h>
 #include <credentials/DeviceAttestationCredsProvider.h>
 #include <credentials/attestation_verifier/DeviceAttestationVerifier.h>
-#include <data-model-providers/codegen/CodegenDataModelProvider.h>
 
 namespace {
 chip::DeviceLayer::DeviceInfoProviderImpl gExampleDeviceInfoProvider;
@@ -125,13 +123,7 @@ CHIP_ERROR CastingApp::Start()
     // Start Matter server
     chip::ServerInitParams * serverInitParams = mAppParameters->GetServerInitParamsProvider()->Get();
     VerifyOrReturnError(serverInitParams != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
-
-    CHIP_ERROR initError = chip::Server::GetInstance().Init(*serverInitParams);
-    if (initError != CHIP_NO_ERROR)
-    {
-        ChipLogError(Discovery, "CastingApp::Start() Server::Init failed: %s", initError.AsString());
-        return initError;
-    }
+    ReturnErrorOnFailure(chip::Server::GetInstance().Init(*serverInitParams));
 
     // Perform post server startup registrations
     ReturnErrorOnFailure(PostStartRegistrations());
@@ -150,7 +142,6 @@ CHIP_ERROR CastingApp::Start()
         CastingPlayer::GetTargetCastingPlayer()->VerifyOrEstablishConnection(connectionCallbacks);
     }
 
-    ChipLogProgress(Discovery, "CastingApp::Start() completed");
     return CHIP_NO_ERROR;
 }
 
@@ -197,21 +188,10 @@ CHIP_ERROR CastingApp::Stop()
     chip::Server::GetInstance().GetUserDirectedCommissioningClient()->SetCommissionerDeclarationHandler(nullptr);
 #endif // CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_DISCOVERY_CLIENT
 
-    // Shutdown the Matter server to clean up active sessions
+    // Shutdown the Matter server
     chip::Server::GetInstance().Shutdown();
 
-    // Shutdown the CodegenDataModelProvider to reset mContext
-    CHIP_ERROR providerShutdownErr = chip::app::CodegenDataModelProvider::Instance().Shutdown();
-    if (providerShutdownErr != CHIP_NO_ERROR)
-    {
-        ChipLogError(Discovery, "CastingApp::Stop() CodegenDataModelProvider::Shutdown failed: %s", providerShutdownErr.AsString());
-    }
-
-    // Destroy EventManagement to reset its state
-    chip::app::EventManagement::DestroyEventManagement();
-
     mState = CASTING_APP_NOT_RUNNING; // CastingApp stopped successfully, set state to NOT_RUNNING
-    ChipLogProgress(Discovery, "CastingApp::Stop() completed");
 
     return CHIP_NO_ERROR;
 }

src/app/InteractionModelEngine.cpp

@@ -240,19 +240,6 @@ void InteractionModelEngine::Shutdown()
 
     mReadHandlers.ReleaseAll();
 
-    // Shut down the data model provider to clear cluster mContext pointers.
-    // Required for proper Stop() → Start() lifecycle - ensures clusters are reinitialized.
-    if (mDataModelProvider != nullptr)
-    {
-        CHIP_ERROR err = mDataModelProvider->Shutdown();
-        if (err != CHIP_NO_ERROR)
-        {
-            ChipLogError(InteractionModel,
-                         "InteractionModelEngine::Shutdown() Data model provider shutdown failed: %" CHIP_ERROR_FORMAT,
-                         err.Format());
-        }
-    }
-
 #if CHIP_CONFIG_ENABLE_READ_CLIENT
     // Shut down any subscription clients that are still around.  They won't be
     // able to work after this point anyway, since we're about to drop our refs
@@ -1929,20 +1916,14 @@ DataModel::Provider * InteractionModelEngine::SetDataModelProvider(DataModel::Pr
     // Altering data model should not be done while IM is actively handling requests.
     VerifyOrDie(mReadHandlers.begin() == mReadHandlers.end());
 
-    // REMOVED: Early return when (model == mDataModelProvider) - breaks Stop() → Start()
-    // After Shutdown(), server clusters are uninitialized even though pointer is unchanged.
-    // Must call Startup() again to reinitialize cluster mContext pointers.
-
     if (model == mDataModelProvider)
     {
-        ChipLogDetail(DataManagement,
-                      "InteractionModelEngine::SetDataModelProvider() re-initializing same provider (Stop/Start cycle)");
+        // no-op, just return
+        return model;
     }
 
     DataModel::Provider * oldModel = mDataModelProvider;
-
-    // Only shutdown if changing to a different provider
-    if (oldModel != nullptr && oldModel != model)
+    if (oldModel != nullptr)
     {
         CHIP_ERROR err = oldModel->Shutdown();
         if (err != CHIP_NO_ERROR)

src/app/clusters/general-commissioning-server/GeneralCommissioningCluster.cpp

@@ -526,23 +526,6 @@ GeneralCommissioningCluster::HandleCommissioningComplete(const DataModel::Invoke
     }
 #endif // CHIP_CONFIG_TERMS_AND_CONDITIONS_REQUIRED
 
-    // Check if mContext is valid before accessing it (can be null during app shutdown/restart)
-    if (mContext == nullptr)
-    {
-        ChipLogError(FailSafe, "GeneralCommissioning: mContext is NULL, cluster not initialized");
-        response.errorCode = CommissioningErrorEnum::kInvalidAuthentication;
-        handler->AddResponse(request.path, response);
-        return std::nullopt;
-    }
-
-    if (!mContext->interactionContext.actionContext.CurrentExchange())
-    {
-        ChipLogError(FailSafe, "GeneralCommissioning: Invalid exchange during commissioning complete");
-        response.errorCode = CommissioningErrorEnum::kInvalidAuthentication;
-        handler->AddResponse(request.path, response);
-        return std::nullopt;
-    }
-
     SessionHandle handle = mContext->interactionContext.actionContext.CurrentExchange()->GetSessionHandle();
 
     // Ensure it's a valid CASE session

src/app/server-cluster/DefaultServerCluster.cpp

@@ -81,19 +81,7 @@ CHIP_ERROR DefaultServerCluster::Attributes(const ConcreteClusterPath & path, Re
 
 CHIP_ERROR DefaultServerCluster::Startup(ServerClusterContext & context)
 {
-    // Reset shutdown state to allow restart after shutdown
-    mIsShutdown = false;
-
-    // Make Startup() idempotent for Stop() → Start() lifecycle.
-    // Shutdown() does not fully clean up cluster objects - cluster objects persist across Stop() → Start().
-    // Only their state (mContext) is cleared.
-    // If already initialized, update context and return success (preserves mDataVersion).
-    if (mContext != nullptr)
-    {
-        ChipLogDetail(DataManagement, "DefaultServerCluster::Startup() already initialized, updating context (idempotent)");
-        mContext = &context;
-        return CHIP_NO_ERROR;
-    }
+    VerifyOrReturnError(mContext == nullptr, CHIP_ERROR_ALREADY_INITIALIZED);
 
     mContext = &context;
 
@@ -106,14 +94,7 @@ CHIP_ERROR DefaultServerCluster::Startup(ServerClusterContext & context)
 
 void DefaultServerCluster::Shutdown(ClusterShutdownType)
 {
-    // Make shutdown idempotent - safe to call multiple times
-    if (mIsShutdown)
-    {
-        return;
-    }
-
-    mContext    = nullptr;
-    mIsShutdown = true;
+    mContext = nullptr;
 }
 
 void DefaultServerCluster::NotifyAttributeChanged(AttributeId attributeId)

src/app/server-cluster/DefaultServerCluster.h

@@ -106,8 +106,6 @@ class DefaultServerCluster : public ServerClusterInterface
 protected:
     const ConcreteClusterPath mPath;
     ServerClusterContext * mContext = nullptr;
-    // Tracks if shutdown has been called to make it idempotent
-    bool mIsShutdown = false;
 
     bool IsStarted() const { return mContext != nullptr; }
 

src/app/server-cluster/ServerClusterInterfaceRegistry.cpp

@@ -43,58 +43,21 @@ ServerClusterInterfaceRegistry::~ServerClusterInterfaceRegistry()
 
 CHIP_ERROR ServerClusterInterfaceRegistry::Register(ServerClusterRegistration & entry)
 {
+    // we have no strong way to check if entry is already registered somewhere else, so we use "next" as some
+    // form of double-check
+    VerifyOrReturnError(entry.next == nullptr, CHIP_ERROR_INVALID_ARGUMENT);
     VerifyOrReturnError(entry.serverClusterInterface != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
 
     Span<const ConcreteClusterPath> paths = entry.serverClusterInterface->GetPaths();
     VerifyOrReturnError(!paths.empty(), CHIP_ERROR_INVALID_ARGUMENT);
 
-    // Check early if this cluster is already registered (idempotent case)
-    // This prevents unnecessary validation work and ensures we don't modify the linked list structure
-    bool isAlreadyRegistered = false;
-    for (const ConcreteClusterPath & path : paths)
-    {
-        ServerClusterInterface * existing = Get(path);
-        if (existing == entry.serverClusterInterface)
-        {
-            isAlreadyRegistered = true;
-            break;
-        }
-    }
-
-    // Validate entry.next is nullptr (unless already registered)
-    // A non-null entry.next when not registered indicates the entry is
-    // already part of another list or improperly initialized
-    if (entry.next != nullptr && !isAlreadyRegistered)
-    {
-        return CHIP_ERROR_INVALID_ARGUMENT;
-    }
-
-    // If already registered (idempotent case), return early
-    if (isAlreadyRegistered)
-    {
-#if CHIP_DETAIL_LOGGING
-        const ConcreteClusterPath path = entry.serverClusterInterface->GetPaths().front();
-        ChipLogDetail(DataManagement, "Cluster already registered for %u/" ChipLogFormatMEI ", skipping re-registration",
-                      path.mEndpointId, ChipLogValueMEI(path.mClusterId));
-#endif
-        return CHIP_NO_ERROR;
-    }
-
-    // Validate paths and check for duplicate registrations
-    // Note: Same cluster re-registering is OK (handled above), but a DIFFERENT
-    // cluster with the same endpoint/cluster ID is an error
-    // Duplicate checking makes this O(n^2) on total registered items. We preserve this to ensure
-    // cluster integrity during Stop/Start cycles, but this may be optimized in the future if needed.
     for (const ConcreteClusterPath & path : paths)
     {
         VerifyOrReturnError(path.HasValidIds(), CHIP_ERROR_INVALID_ARGUMENT);
 
-        // A different cluster is already registered for this path
-        ServerClusterInterface * existing = Get(path);
-        if (existing != nullptr)
-        {
-            return CHIP_ERROR_DUPLICATE_KEY_ID;
-        }
+        // Double-checking for duplicates makes the checks O(n^2) on the total number of registered
+        // items. We preserve this however we may want to make this optional at some point in time.
+        VerifyOrReturnError(Get(path) == nullptr, CHIP_ERROR_DUPLICATE_KEY_ID);
     }
 
     if (mContext.has_value())

src/app/server-cluster/ServerClusterInterfaceRegistry.h

@@ -20,7 +20,6 @@
 #include <app/server-cluster/ServerClusterInterface.h>
 #include <lib/core/CHIPError.h>
 #include <lib/core/DataModelTypes.h>
-#include <lib/support/logging/CHIPLogging.h>
 
 #include <cstdint>
 #include <new>
@@ -98,14 +97,7 @@ struct LazyRegisteredServerCluster
     template <typename... Args>
     void Create(Args &&... args)
     {
-        // Handle idempotent Create() - if already constructed, this is a no-op.
-        // The cluster remains registered and functional. This supports the Stop() → Start() lifecycle
-        // where clusters are shutdown but remain constructed.
-        if (IsConstructed())
-        {
-            ChipLogDetail(DataManagement, "LazyRegisteredServerCluster::Create: Cluster already constructed, skipping re-creation");
-            return;
-        }
+        VerifyOrDie(!IsConstructed());
 
         new (mCluster) SERVER_CLUSTER(std::forward<Args>(args)...);
         new (mRegistration) ServerClusterRegistration(Cluster());