From 76e86f25a525a5e68fdf0da9032ab37a449bf8c6 Mon Sep 17 00:00:00 2001 From: Zang MingJie Date: Sat, 18 Jun 2022 02:16:13 +0800 Subject: [PATCH] Resolve comments --- src/messaging/ExchangeMgr.cpp | 6 +++++- src/messaging/ExchangeMgr.h | 5 ++--- src/messaging/ReliableMessageContext.h | 8 ++++---- src/transport/SecureSession.cpp | 4 ++-- src/transport/SecureSession.h | 4 ++-- src/transport/SessionManager.cpp | 7 +++++-- src/transport/SessionManager.h | 5 +++-- 7 files changed, 23 insertions(+), 16 deletions(-) diff --git a/src/messaging/ExchangeMgr.cpp b/src/messaging/ExchangeMgr.cpp index 05b70adec8adf9..06ca1559c04eb2 100644 --- a/src/messaging/ExchangeMgr.cpp +++ b/src/messaging/ExchangeMgr.cpp @@ -378,16 +378,20 @@ void ExchangeManager::CloseAllContextsForDelegate(const ExchangeDelegate * deleg void ExchangeManager::AbortExchangesForFabricExceptOne(FabricIndex fabricIndex, ExchangeContext * deferred) { + VerifyOrDie(deferred->HasSessionHandle() && deferred->GetSessionHandle()->IsSecureSession()); + mContextPool.ForEachActiveObject([&](auto * ec) { if (ec->HasSessionHandle() && ec->GetSessionHandle()->GetFabricIndex() == fabricIndex) { if (ec == deferred) - ec->SetAutoReleaseSession(); + ec->ReleaseSessionOnDestruction(); else ec->Abort(); } return Loop::Continue; }); + + mSessionManager->ReleaseSessionsForFabricExceptOne(fabricIndex, deferred->GetSessionHandle()); } } // namespace Messaging diff --git a/src/messaging/ExchangeMgr.h b/src/messaging/ExchangeMgr.h index 910dcb8ee81db3..112cddd82c7ecb 100644 --- a/src/messaging/ExchangeMgr.h +++ b/src/messaging/ExchangeMgr.h @@ -183,9 +183,8 @@ class DLL_EXPORT ExchangeManager : public SessionMessageDelegate */ void CloseAllContextsForDelegate(const ExchangeDelegate * delegate); - // This API is used by commands that need to shut down all existing exchanges on - // a fabric but need to make sure the response to the command still goes out on the exchange - // the command came in on. This API flags that one exchange to shut down its session + // This API is used by commands that need to shut down all existing exchanges on a fabric but need to make sure the response to + // the command still goes out on the exchange the command came in on. This API flags that one exchange to shut down its session // when it's done. void AbortExchangesForFabricExceptOne(FabricIndex fabricIndex, ExchangeContext * deferred); diff --git a/src/messaging/ReliableMessageContext.h b/src/messaging/ReliableMessageContext.h index 6e94da51f4a739..01b830cbd0d42e 100644 --- a/src/messaging/ReliableMessageContext.h +++ b/src/messaging/ReliableMessageContext.h @@ -124,10 +124,10 @@ class ReliableMessageContext /// Determine whether this exchange is a EphemeralExchange for replying a StandaloneAck bool IsEphemeralExchange() const; - // If SetAutoReleaseSession() is called, this exchange must be using a SecureSession, and should + // If ReleaseSessionOnDestruction() is called, this exchange must be using a SecureSession, and should // evict it when the exchange is done with all its work (including any MRP traffic). - void SetAutoReleaseSession(); - bool ReleaseSessionOnDestruction(); + void ReleaseSessionOnDestruction(); + bool IsAutoReleaseSession(); /** * Get the reliable message manager that corresponds to this reliable @@ -253,7 +253,7 @@ inline bool ReliableMessageContext::IsEphemeralExchange() const return mFlags.Has(Flags::kFlagEphemeralExchange); } -inline void ReliableMessageContext::SetAutoReleaseSession() +inline void ReliableMessageContext::ReleaseSessionOnDestruction() { mFlags.Set(Flags::kFlagAutoReleaseSession, true); } diff --git a/src/transport/SecureSession.cpp b/src/transport/SecureSession.cpp index e63d2d861e4ce1..9363d92b323f21 100644 --- a/src/transport/SecureSession.cpp +++ b/src/transport/SecureSession.cpp @@ -50,9 +50,9 @@ void SecureSession::MarkForRemoval() } } -void SecureSession::MarkForInactive() +void SecureSession::MarkInactive() { - ChipLogDetail(Inet, "SecureSession[%p]: MarkForInactive Type:%d LSID:%d", this, to_underlying(mSecureSessionType), + ChipLogDetail(Inet, "SecureSession[%p]: MarkInactive Type:%d LSID:%d", this, to_underlying(mSecureSessionType), mLocalSessionId); ReferenceCountedHandle ref(*this); switch (mState) diff --git a/src/transport/SecureSession.h b/src/transport/SecureSession.h index 6ce8202c65b2d3..829b179965a8b2 100644 --- a/src/transport/SecureSession.h +++ b/src/transport/SecureSession.h @@ -145,7 +145,7 @@ class SecureSession : public Session, public ReferenceCountedIsSecureSession()); + SecureSession * deferredSecureSession = deferred->AsSecureSession(); + mSecureSessions.ForEachSession([&](auto session) { if (session->GetPeer().GetFabricIndex() == fabricIndex) { - if (session == deferred->AsSecureSession()) - session->MarkForInactive(); + if (session == deferredSecureSession) + session->MarkInactive(); else session->MarkForRemoval(); } diff --git a/src/transport/SessionManager.h b/src/transport/SessionManager.h index c57f7089f04725..d88031f670f4ac 100644 --- a/src/transport/SessionManager.h +++ b/src/transport/SessionManager.h @@ -176,8 +176,9 @@ class DLL_EXPORT SessionManager : public TransportMgrDelegate void ExpireAllPairingsForFabric(FabricIndex fabric); void ExpireAllPASEPairings(); - // This API is used by UpdateNOC command, to invalidate all sessions except the given one, whose release is deferred until - // UpdateNOC command finishing its work. + // This API is used by commands that need to release all existing sessions on a fabric but need to make sure the response to the + // command still goes out on the exchange the command came in on. This API flags that the release of the session used by the + // exchange is deferred until the exchange is done. void ReleaseSessionsForFabricExceptOne(FabricIndex fabricIndex, const SessionHandle & deferred); /**