Make sure DnssdServer does not dereference a dangling fabric table. (#…

…26354)

We could end up not calling StopServer if the last release of a system state was
a direct release on the state itself, not a call to ReleaseSystemState.

To fix this, move the DnssdServer::StopServer bits into the actual system state
shutdown, where we know whether we are destroying the fabric table that
DnssdServer uses and can clean up appropriately.

src/app/server/Dnssd.cpp

@@ -345,6 +345,9 @@ void DnssdServer::StartServer()
 
 void DnssdServer::StopServer()
 {
+    // Make sure we don't hold on to a dangling fabric table pointer.
+    mFabricTable = nullptr;
+
     DeviceLayer::PlatformMgr().RemoveEventHandler(OnPlatformEventWrapper, 0);
 
 #if CHIP_DEVICE_CONFIG_ENABLE_EXTENDED_DISCOVERY

src/app/server/Dnssd.h

@@ -60,8 +60,9 @@ class DLL_EXPORT DnssdServer
     Inet::InterfaceId GetInterfaceId() { return mInterfaceId; }
 
     //
-    // Override the referenced fabric table from the default that is present
-    // in Server::GetInstance().GetFabricTable() to something else.
+    // Set the fabric table the DnssdServer should use for operational
+    // advertising.  This must be set before StartServer() is called for the
+    // first time.
     //
     void SetFabricTable(FabricTable * table)
     {
@@ -94,7 +95,8 @@ class DLL_EXPORT DnssdServer
     /// (Re-)starts the Dnssd server, using the provided commissioning mode.
     void StartServer(Dnssd::CommissioningMode mode);
 
-    //// Stop the Dnssd server.
+    //// Stop the Dnssd server.  After this call, SetFabricTable must be called
+    //// again before calling StartServer().
     void StopServer();
 
     CHIP_ERROR GenerateRotatingDeviceId(char rotatingDeviceIdHexBuffer[], size_t rotatingDeviceIdHexBufferSize);

src/controller/CHIPDeviceControllerFactory.cpp

@@ -272,7 +272,7 @@ CHIP_ERROR DeviceControllerFactory::InitSystemState(FactoryInitParams params)
 
     // store the system state
     mSystemState = chip::Platform::New<DeviceControllerSystemState>(std::move(stateParams));
-    mSystemState->SetTempFabricTable(tempFabricTable);
+    mSystemState->SetTempFabricTable(tempFabricTable, params.enableServerInteractions);
     ChipLogDetail(Controller, "System State Initialized...");
     return CHIP_NO_ERROR;
 }
@@ -348,11 +348,6 @@ void DeviceControllerFactory::RetainSystemState()
 void DeviceControllerFactory::ReleaseSystemState()
 {
     mSystemState->Release();
-
-    if (!mSystemState->IsInitialized() && mEnableServerInteractions)
-    {
-        app::DnssdServer::Instance().StopServer();
-    }
 }
 
 DeviceControllerFactory::~DeviceControllerFactory()
@@ -384,6 +379,14 @@ void DeviceControllerSystemState::Shutdown()
 
     ChipLogDetail(Controller, "Shutting down the System State, this will teardown the CHIP Stack");
 
+    if (mTempFabricTable && mEnableServerInteractions)
+    {
+        // The DnssdServer is holding a reference to our temp fabric table,
+        // which we are about to destroy.  Stop it, so that it will stop trying
+        // to use it.
+        app::DnssdServer::Instance().StopServer();
+    }
+
     if (mFabricTableDelegate != nullptr)
     {
         if (mFabrics != nullptr)

src/controller/CHIPDeviceControllerSystemState.h

@@ -185,7 +185,11 @@ class DeviceControllerSystemState
     CASESessionManager * CASESessionMgr() const { return mCASESessionManager; }
     Credentials::GroupDataProvider * GetGroupDataProvider() const { return mGroupDataProvider; }
     Crypto::SessionKeystore * GetSessionKeystore() const { return mSessionKeystore; }
-    void SetTempFabricTable(FabricTable * tempFabricTable) { mTempFabricTable = tempFabricTable; }
+    void SetTempFabricTable(FabricTable * tempFabricTable, bool enableServerInteractions)
+    {
+        mTempFabricTable          = tempFabricTable;
+        mEnableServerInteractions = enableServerInteractions;
+    }
 
 private:
     DeviceControllerSystemState() {}
@@ -220,6 +224,8 @@ class DeviceControllerSystemState
 
     bool mHaveShutDown = false;
 
+    bool mEnableServerInteractions = false;
+
     void Shutdown();
 };