diff --git a/src/app/OperationalSessionSetup.cpp b/src/app/OperationalSessionSetup.cpp index 11ceb8bb6c0e26..3a0d124297a091 100644 --- a/src/app/OperationalSessionSetup.cpp +++ b/src/app/OperationalSessionSetup.cpp @@ -125,7 +125,12 @@ void OperationalSessionSetup::Connect(Callback::Callback * 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 index d0db84f6c69ce1..cde2fddc6dac0b 100644 --- a/src/app/OperationalSessionSetup.h +++ b/src/app/OperationalSessionSetup.h @@ -208,8 +208,6 @@ class DLL_EXPORT OperationalSessionSetup : public SessionDelegate, */ void Connect(Callback::Callback * onConnection, Callback::Callback * 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