Move serialization of chip::Device earlier. (#7218)

Serializing in the destructor doesn't really do the right thing, because by then Reset() has usually been called and hence mSessionManager is null. Instead, serialize in Reset() if were resetting an active device. This fixes the remaining problems I was seeing with chip-tool messages being rejected for having the wrong message counter values (due to us never serializing the incremented message counter).
project-chip · Jun 8, 2021 · 2442296 · 2442296
1 parent 78bfa34
commit 2442296
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 29 deletions.
diff --git a/src/controller/CHIPDevice.cpp b/src/controller/CHIPDevice.cpp
@@ -391,6 +391,36 @@ CHIP_ERROR Device::UpdateAddress(const Transport::PeerAddress & addr)
     return CHIP_NO_ERROR;
 }
 
+void Device::Reset()
+{
+    if (IsActive() && mStorageDelegate != nullptr && mSessionManager != nullptr)
+    {
+        // If a session can be found, persist the device so that we track the newest message counter values
+        Transport::PeerConnectionState * connectionState = mSessionManager->GetPeerConnectionState(mSecureSession);
+        if (connectionState != nullptr)
+        {
+            Persist();
+        }
+    }
+
+    SetActive(false);
+    mState          = ConnectionState::NotConnected;
+    mSessionManager = nullptr;
+    mStatusDelegate = nullptr;
+    mInetLayer      = nullptr;
+#if CONFIG_NETWORK_LAYER_BLE
+    mBleLayer = nullptr;
+#endif
+    if (mExchangeMgr)
+    {
+        // Ensure that any exchange contexts we have open get closed now,
+        // because we don't want them to call back in to us after this
+        // point.
+        mExchangeMgr->CloseAllContextsForDelegate(this);
+    }
+    mExchangeMgr = nullptr;
+}
+
 CHIP_ERROR Device::LoadSecureSessionParameters(ResetTransport resetNeeded)
 {
     CHIP_ERROR err = CHIP_NO_ERROR;
@@ -528,16 +558,6 @@ Device::~Device()
         // point.
         mExchangeMgr->CloseAllContextsForDelegate(this);
     }
-
-    if (mStorageDelegate != nullptr && mSessionManager != nullptr)
-    {
-        // If a session can be found, persist the device so that we track the newest message counter values
-        Transport::PeerConnectionState * connectionState = mSessionManager->GetPeerConnectionState(mSecureSession);
-        if (connectionState != nullptr)
-        {
-            Persist();
-        }
-    }
 }
 
 } // namespace Controller

diff --git a/src/controller/CHIPDevice.h b/src/controller/CHIPDevice.h
@@ -308,25 +308,7 @@ class DLL_EXPORT Device : public Messaging::ExchangeDelegate, public SessionEsta
 
     bool IsSecureConnected() const { return IsActive() && mState == ConnectionState::SecureConnected; }
 
-    void Reset()
-    {
-        SetActive(false);
-        mState          = ConnectionState::NotConnected;
-        mSessionManager = nullptr;
-        mStatusDelegate = nullptr;
-        mInetLayer      = nullptr;
-#if CONFIG_NETWORK_LAYER_BLE
-        mBleLayer = nullptr;
-#endif
-        if (mExchangeMgr)
-        {
-            // Ensure that any exchange contexts we have open get closed now,
-            // because we don't want them to call back in to us after this
-            // point.
-            mExchangeMgr->CloseAllContextsForDelegate(this);
-        }
-        mExchangeMgr = nullptr;
-    }
+    void Reset();
 
     NodeId GetDeviceId() const { return mDeviceId; }