diff --git a/src/app/OperationalSessionSetup.cpp b/src/app/OperationalSessionSetup.cpp index 849490b40ab8c8..7b5d70919f2a8a 100644 --- a/src/app/OperationalSessionSetup.cpp +++ b/src/app/OperationalSessionSetup.cpp @@ -276,7 +276,7 @@ void OperationalSessionSetup::EnqueueConnectionCallbacks(Callback::Callback optionalSessionHandle = mSecureSession.Get(); + ScopedNodeId peerId = mPeerId; + + if (releaseBehavior == ReleaseBehavior::Release) + { + VerifyOrDie(mReleaseDelegate != nullptr); + mReleaseDelegate->ReleaseSession(this); + } + + // DO NOT touch any members of this object after this point. It's dead. + + NotifyConnectionCallbacks(failureReady, successReady, error, peerId, performingAddressUpdate, exchangeMgr, + optionalSessionHandle); +} + +void OperationalSessionSetup::NotifyConnectionCallbacks(Cancelable & failureReady, Cancelable & successReady, CHIP_ERROR error, + const ScopedNodeId & peerId, bool performingAddressUpdate, + Messaging::ExchangeManager * exchangeMgr, + const Optional & optionalSessionHandle) +{ // // If we encountered no error, go ahead and call all success callbacks. Otherwise, // call the failure callbacks. @@ -304,7 +327,7 @@ void OperationalSessionSetup::DequeueConnectionCallbacksWithoutReleasing(CHIP_ER while (failureReady.mNext != &failureReady) { // We expect that we only have callbacks if we are not performing just address update. - VerifyOrDie(!mPerformingAddressUpdate); + VerifyOrDie(!performingAddressUpdate); Callback::Callback * cb = Callback::Callback::FromCancelable(failureReady.mNext); @@ -312,35 +335,26 @@ void OperationalSessionSetup::DequeueConnectionCallbacksWithoutReleasing(CHIP_ER if (error != CHIP_NO_ERROR) { - cb->mCall(cb->mContext, mPeerId, error); + cb->mCall(cb->mContext, peerId, error); } } while (successReady.mNext != &successReady) { // We expect that we only have callbacks if we are not performing just address update. - VerifyOrDie(!mPerformingAddressUpdate); + VerifyOrDie(!performingAddressUpdate); Callback::Callback * cb = Callback::Callback::FromCancelable(successReady.mNext); cb->Cancel(); if (error == CHIP_NO_ERROR) { - auto * exchangeMgr = mInitParams.exchangeMgr; VerifyOrDie(exchangeMgr); // We know that we for sure have the SessionHandle in the successful case. - auto optionalSessionHandle = mSecureSession.Get(); cb->mCall(cb->mContext, *exchangeMgr, optionalSessionHandle.Value()); } } } -void OperationalSessionSetup::DequeueConnectionCallbacks(CHIP_ERROR error) -{ - DequeueConnectionCallbacksWithoutReleasing(error); - VerifyOrDie(mReleaseDelegate != nullptr); - mReleaseDelegate->ReleaseSession(this); -} - void OperationalSessionSetup::OnSessionEstablishmentError(CHIP_ERROR error) { VerifyOrReturn(mState == State::Connecting, @@ -447,7 +461,7 @@ OperationalSessionSetup::~OperationalSessionSetup() CancelSessionSetupReattempt(); #endif // CHIP_DEVICE_CONFIG_ENABLE_AUTOMATIC_CASE_RETRIES - DequeueConnectionCallbacksWithoutReleasing(CHIP_ERROR_CANCELLED); + DequeueConnectionCallbacks(CHIP_ERROR_CANCELLED, ReleaseBehavior::DoNotRelease); } CHIP_ERROR OperationalSessionSetup::LookupPeerAddress() diff --git a/src/app/OperationalSessionSetup.h b/src/app/OperationalSessionSetup.h index c6208a00f4ba35..bfa76fbc41d730 100644 --- a/src/app/OperationalSessionSetup.h +++ b/src/app/OperationalSessionSetup.h @@ -301,6 +301,12 @@ class DLL_EXPORT OperationalSessionSetup : public SessionEstablishmentDelegate, void EnqueueConnectionCallbacks(Callback::Callback * onConnection, Callback::Callback * onFailure); + enum class ReleaseBehavior + { + Release, + DoNotRelease + }; + /* * This dequeues all failure and success callbacks and appropriately * invokes either set depending on the value of error. @@ -308,17 +314,23 @@ class DLL_EXPORT OperationalSessionSetup : public SessionEstablishmentDelegate, * If error == CHIP_NO_ERROR, only success callbacks are invoked. * Otherwise, only failure callbacks are invoked. * - * This uses mReleaseDelegate to release ourselves (aka `this`). As a - * result any caller should return right away without touching `this`. + * If releaseBehavior is Release, this uses mReleaseDelegate to release + * ourselves (aka `this`). As a result any caller should return right away + * without touching `this`. * + * Setting releaseBehavior to DoNotRelease is meant for use from the destructor */ - void DequeueConnectionCallbacks(CHIP_ERROR error); + void DequeueConnectionCallbacks(CHIP_ERROR error, ReleaseBehavior releaseBehavior = ReleaseBehavior::Release); - /* - * Like DequeueConnectionCallbacks but does not release ourselves. For use - * from our destructor. + /** + * Helper for DequeueConnectionCallbacks that handles the actual callback + * notifications. This happens after the object has been released, if it's + * being released. */ - void DequeueConnectionCallbacksWithoutReleasing(CHIP_ERROR error); + static void NotifyConnectionCallbacks(Callback::Cancelable & failureReady, Callback::Cancelable & successReady, + CHIP_ERROR error, const ScopedNodeId & peerId, bool performingAddressUpdate, + Messaging::ExchangeManager * exchangeMgr, + const Optional & optionalSessionHandle); /** * Triggers a DNSSD lookup to find a usable peer address.