Remove mRemoteMRPConfig member from OperationalSessionSetup (#21924)

project-chip · Sep 6, 2022 · 2347824 · 2347824
1 parent 3bdfaab
commit 2347824
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 13 deletions.
diff --git a/src/app/OperationalSessionSetup.cpp b/src/app/OperationalSessionSetup.cpp
@@ -125,7 +125,12 @@ void OperationalSessionSetup::Connect(Callback::Callback<OnDeviceConnected> * on
         isConnected = AttachToExistingSecureSession();
         if (!isConnected)
         {
-            err = EstablishConnection();
+            // We should not actually every be in be in State::HasAddress. This
+            // is because in the same call that we moved to State::HasAddress
+            // we either move to State::Connecting or call
+            // DequeueConnectionCallbacks with an error thus releasing
+            // ourselves before any call would reach this section of code.
+            err = CHIP_ERROR_INCORRECT_STATE;
         }
 
         break;
@@ -179,13 +184,11 @@ void OperationalSessionSetup::UpdateDeviceData(const Transport::PeerAddress & ad
     CHIP_ERROR err = CHIP_NO_ERROR;
     mDeviceAddress = addr;
 
-    mRemoteMRPConfig = config;
-
     // Initialize CASE session state with any MRP parameters that DNS-SD has provided.
     // It can be overridden by CASE session protocol messages that include MRP parameters.
     if (mCASEClient)
     {
-        mCASEClient->SetRemoteMRPIntervals(mRemoteMRPConfig);
+        mCASEClient->SetRemoteMRPIntervals(config);
     }
 
     if (mState == State::ResolvingAddress)
@@ -194,7 +197,7 @@ void OperationalSessionSetup::UpdateDeviceData(const Transport::PeerAddress & ad
         mInitParams.sessionManager->UpdateAllSessionsPeerAddress(mPeerId, addr);
         if (!mPerformingAddressUpdate)
         {
-            err = EstablishConnection();
+            err = EstablishConnection(config);
             if (err != CHIP_NO_ERROR)
             {
                 DequeueConnectionCallbacks(err);
@@ -216,14 +219,14 @@ void OperationalSessionSetup::UpdateDeviceData(const Transport::PeerAddress & ad
     // Do not touch `this` instance anymore; it has been destroyed in DequeueConnectionCallbacks.
 }
 
-CHIP_ERROR OperationalSessionSetup::EstablishConnection()
+CHIP_ERROR OperationalSessionSetup::EstablishConnection(const ReliableMessageProtocolConfig & config)
 {
     mCASEClient = mInitParams.clientPool->Allocate(CASEClientInitParams{
         mInitParams.sessionManager, mInitParams.sessionResumptionStorage, mInitParams.certificateValidityPolicy,
         mInitParams.exchangeMgr, mFabricTable, mInitParams.groupDataProvider, mInitParams.mrpLocalConfig });
     ReturnErrorCodeIf(mCASEClient == nullptr, CHIP_ERROR_NO_MEMORY);
 
-    CHIP_ERROR err = mCASEClient->EstablishSession(mPeerId, mDeviceAddress, mRemoteMRPConfig, this);
+    CHIP_ERROR err = mCASEClient->EstablishSession(mPeerId, mDeviceAddress, config, this);
     if (err != CHIP_NO_ERROR)
     {
         CleanupCASEClient();
@@ -334,7 +337,10 @@ void OperationalSessionSetup::CleanupCASEClient()
 
 void OperationalSessionSetup::OnSessionReleased()
 {
-    MoveToState(State::HasAddress);
+    // This is unlikely to be called since within the same call that we get SessionHandle we
+    // then call DequeueConnectionCallbacks which releases `this`. If this is called, and we
+    // we have any callbacks we will just send an error.
+    DequeueConnectionCallbacks(CHIP_ERROR_INCORRECT_STATE);
 }
 
 OperationalSessionSetup::~OperationalSessionSetup()

diff --git a/src/app/OperationalSessionSetup.h b/src/app/OperationalSessionSetup.h
@@ -208,8 +208,6 @@ class DLL_EXPORT OperationalSessionSetup : public SessionDelegate,
      */
     void Connect(Callback::Callback<OnDeviceConnected> * onConnection, Callback::Callback<OnDeviceConnectionFailure> * onFailure);
 
-    bool IsConnecting() const { return mState == State::Connecting; }
-
     bool IsForAddressUpdate() const { return mPerformingAddressUpdate; }
 
     //////////// SessionEstablishmentDelegate Implementation ///////////////
@@ -288,11 +286,9 @@ class DLL_EXPORT OperationalSessionSetup : public SessionDelegate,
     /// This is used when a node address is required.
     chip::AddressResolve::NodeLookupHandle mAddressLookupHandle;
 
-    ReliableMessageProtocolConfig mRemoteMRPConfig = GetDefaultMRPConfig();
-
     bool mPerformingAddressUpdate = false;
 
-    CHIP_ERROR EstablishConnection();
+    CHIP_ERROR EstablishConnection(const ReliableMessageProtocolConfig & config);
 
     /*
      * This checks to see if an existing CASE session exists to the peer within the SessionManager