From a4e004a3fc4ffecd0d4894713edff95cb5d79778 Mon Sep 17 00:00:00 2001 From: Zang MingJie Date: Thu, 9 Jun 2022 01:38:23 +0800 Subject: [PATCH 1/8] Add API to invalid sessions/exchanges for UpdateNOC command --- src/messaging/ExchangeMgr.cpp | 11 +++++++++++ src/messaging/ExchangeMgr.h | 3 +++ src/transport/SessionManager.cpp | 11 +++++++++++ src/transport/SessionManager.h | 3 +++ 4 files changed, 28 insertions(+) diff --git a/src/messaging/ExchangeMgr.cpp b/src/messaging/ExchangeMgr.cpp index 2c1242aff55c1c..d3da9ccdf0b38c 100644 --- a/src/messaging/ExchangeMgr.cpp +++ b/src/messaging/ExchangeMgr.cpp @@ -376,5 +376,16 @@ void ExchangeManager::CloseAllContextsForDelegate(const ExchangeDelegate * deleg }); } +void ExchangeManager::AbortExchangeForNodeExceptOne(const ScopedNodeId & node, ExchangeContext * stay) +{ + mContextPool.ForEachActiveObject([&](auto * ec) { + if (ec != stay && ec->HasSessionHandle() && ec->GetSessionHandle()->GetPeer() == node) + { + ec->Abort(); + } + return Loop::Continue; + }); +} + } // namespace Messaging } // namespace chip diff --git a/src/messaging/ExchangeMgr.h b/src/messaging/ExchangeMgr.h index d6b775436799f4..3411c8dfdf444e 100644 --- a/src/messaging/ExchangeMgr.h +++ b/src/messaging/ExchangeMgr.h @@ -183,6 +183,9 @@ class DLL_EXPORT ExchangeManager : public SessionMessageDelegate */ void CloseAllContextsForDelegate(const ExchangeDelegate * delegate); + // This API is used by UpdateNOC command, to invalid all exchanges except the given one. + void AbortExchangeForNodeExceptOne(const ScopedNodeId & node, ExchangeContext * stay); + SessionManager * GetSessionManager() const { return mSessionManager; } ReliableMessageMgr * GetReliableMessageMgr() { return &mReliableMessageMgr; }; diff --git a/src/transport/SessionManager.cpp b/src/transport/SessionManager.cpp index a9e95411e778aa..00ed1df347bafe 100644 --- a/src/transport/SessionManager.cpp +++ b/src/transport/SessionManager.cpp @@ -387,6 +387,17 @@ void SessionManager::ExpireAllPASEPairings() }); } +void SessionManager::ReleaseSessionForNodeExceptOne(const ScopedNodeId & node, const SessionHandle & stay) +{ + mSecureSessions.ForEachSession([&](auto session) { + if (session->GetPeer() == node && session != stay->AsSecureSession()) + { + session->MarkForRemoval(); + } + return Loop::Continue; + }); +} + Optional SessionManager::AllocateSession(SecureSession::Type secureSessionType, const ScopedNodeId & sessionEvictionHint) { diff --git a/src/transport/SessionManager.h b/src/transport/SessionManager.h index 1b01f1d5bcb5e7..6e6435928dc0f9 100644 --- a/src/transport/SessionManager.h +++ b/src/transport/SessionManager.h @@ -176,6 +176,9 @@ class DLL_EXPORT SessionManager : public TransportMgrDelegate void ExpireAllPairingsForFabric(FabricIndex fabric); void ExpireAllPASEPairings(); + // This API is used by UpdateNOC command, to invalid all sessions except the given one. + void ReleaseSessionForNodeExceptOne(const ScopedNodeId & node, const SessionHandle & stay); + /** * @brief * Return the System Layer pointer used by current SessionManager. From e52e2d860b11c4fa543e498796a102db82b10f17 Mon Sep 17 00:00:00 2001 From: Zang MingJie Date: Fri, 10 Jun 2022 04:10:23 +0800 Subject: [PATCH 2/8] Complete the idea --- src/messaging/ExchangeContext.cpp | 3 +++ src/messaging/ExchangeMgr.cpp | 9 ++++++--- src/messaging/ReliableMessageContext.h | 19 +++++++++++++++++++ src/transport/SecureSession.cpp | 22 ++++++++++++++++++++++ src/transport/SecureSession.h | 8 ++++++++ src/transport/SessionManager.cpp | 9 ++++++--- 6 files changed, 64 insertions(+), 6 deletions(-) diff --git a/src/messaging/ExchangeContext.cpp b/src/messaging/ExchangeContext.cpp index a2aa221a7cc78e..08d916a15246e2 100644 --- a/src/messaging/ExchangeContext.cpp +++ b/src/messaging/ExchangeContext.cpp @@ -325,6 +325,9 @@ ExchangeContext::~ExchangeContext() VerifyOrDie(mExchangeMgr != nullptr && GetReferenceCount() == 0); VerifyOrDie(!IsAckPending()); + if (IsAutoReleaseSession() && mSession) + mSession->AsSecureSession()->MarkForRemoval(); + #if CONFIG_DEVICE_LAYER && CHIP_DEVICE_CONFIG_ENABLE_SED // Make sure that the exchange withdraws the request for Sleepy End Device active mode. UpdateSEDIntervalMode(false); diff --git a/src/messaging/ExchangeMgr.cpp b/src/messaging/ExchangeMgr.cpp index d3da9ccdf0b38c..b968fc2be11e9f 100644 --- a/src/messaging/ExchangeMgr.cpp +++ b/src/messaging/ExchangeMgr.cpp @@ -376,12 +376,15 @@ void ExchangeManager::CloseAllContextsForDelegate(const ExchangeDelegate * deleg }); } -void ExchangeManager::AbortExchangeForNodeExceptOne(const ScopedNodeId & node, ExchangeContext * stay) +void ExchangeManager::AbortExchangeForNodeExceptOne(const ScopedNodeId & node, ExchangeContext * exception) { mContextPool.ForEachActiveObject([&](auto * ec) { - if (ec != stay && ec->HasSessionHandle() && ec->GetSessionHandle()->GetPeer() == node) + if (ec->HasSessionHandle() && ec->GetSessionHandle()->GetPeer() == node) { - ec->Abort(); + if (ec == exception) + ec->SetAutoReleaseSession(); + else + ec->Abort(); } return Loop::Continue; }); diff --git a/src/messaging/ReliableMessageContext.h b/src/messaging/ReliableMessageContext.h index e67b2d596e963e..778ec6ce631e12 100644 --- a/src/messaging/ReliableMessageContext.h +++ b/src/messaging/ReliableMessageContext.h @@ -121,9 +121,14 @@ class ReliableMessageContext /// Determine whether this exchange is requesting Sleepy End Device active mode bool IsRequestingActiveMode() const; + /// Determine whether this exchange is a EphemeralExchange for replying a StandaloneAck bool IsEphemeralExchange() const; + // For UpdateNOC command, I'm the last exchange, I must release the session when finishing my work. + void SetAutoReleaseSession(); + bool IsAutoReleaseSession(); + /** * Get the reliable message manager that corresponds to this reliable * message context. @@ -162,8 +167,12 @@ class ReliableMessageContext /// When set, signifies that the exchange is requesting Sleepy End Device active mode. kFlagActiveMode = (1u << 8), + /// When set, signifies that the exchange created sorely for replying a StandaloneAck kFlagEphemeralExchange = (1u << 9), + + /// When set, automatically release the session when finishing its work. Used by UpdateNOC command + kFlagAutoReleaseSession = (1u << 10), }; BitFlags mFlags; // Internal state flags @@ -245,5 +254,15 @@ inline bool ReliableMessageContext::IsEphemeralExchange() const return mFlags.Has(Flags::kFlagEphemeralExchange); } +inline void ReliableMessageContext::SetAutoReleaseSession() +{ + mFlags.Set(Flags::kFlagAutoReleaseSession, true); +} + +inline bool ReliableMessageContext::IsAutoReleaseSession() +{ + return mFlags.Has(Flags::kFlagAutoReleaseSession); +} + } // namespace Messaging } // namespace chip diff --git a/src/transport/SecureSession.cpp b/src/transport/SecureSession.cpp index f2ec416c0ba9d6..6aa5cc044a868f 100644 --- a/src/transport/SecureSession.cpp +++ b/src/transport/SecureSession.cpp @@ -39,6 +39,7 @@ void SecureSession::MarkForRemoval() NotifySessionReleased(); return; case State::kActive: + case State::kInactive: Release(); // Decrease the ref which is retained at Activate mState = State::kPendingRemoval; NotifySessionReleased(); @@ -49,6 +50,27 @@ void SecureSession::MarkForRemoval() } } +void SecureSession::MarkForInactive() +{ + ChipLogDetail(Inet, "SecureSession[%p]: MarkForInactive Type:%d LSID:%d", this, to_underlying(mSecureSessionType), + mLocalSessionId); + ReferenceCountedHandle ref(*this); + switch (mState) + { + case State::kPairing: + VerifyOrDie(false); + return; + case State::kActive: + // By setting this state, IsActiveSession() will return false, which prevent creating new exchanges. + mState = State::kInactive; + return; + case State::kInactive: + case State::kPendingRemoval: + // Do nothing + return; + } +} + Access::SubjectDescriptor SecureSession::GetSubjectDescriptor() const { Access::SubjectDescriptor subjectDescriptor; diff --git a/src/transport/SecureSession.h b/src/transport/SecureSession.h index a505a3237f00eb..9978918929a1d2 100644 --- a/src/transport/SecureSession.h +++ b/src/transport/SecureSession.h @@ -144,6 +144,9 @@ class SecureSession : public Session, public ReferenceCountedGetPeer() == node && session != stay->AsSecureSession()) + if (session->GetPeer() == node) { - session->MarkForRemoval(); + if (session == exception->AsSecureSession()) + session->MarkForInactive(); + else + session->MarkForRemoval(); } return Loop::Continue; }); From ca00d2171487bc3f2ed71d321a25448c62ab4955 Mon Sep 17 00:00:00 2001 From: Zang MingJie Date: Fri, 10 Jun 2022 04:53:13 +0800 Subject: [PATCH 3/8] Add purge all fabric API --- src/messaging/ExchangeMgr.cpp | 14 ++++++++++++++ src/messaging/ExchangeMgr.h | 5 +++-- src/transport/SessionManager.cpp | 14 ++++++++++++++ src/transport/SessionManager.h | 3 ++- 4 files changed, 33 insertions(+), 3 deletions(-) diff --git a/src/messaging/ExchangeMgr.cpp b/src/messaging/ExchangeMgr.cpp index b968fc2be11e9f..00b0cf2a5fe60e 100644 --- a/src/messaging/ExchangeMgr.cpp +++ b/src/messaging/ExchangeMgr.cpp @@ -376,6 +376,20 @@ void ExchangeManager::CloseAllContextsForDelegate(const ExchangeDelegate * deleg }); } +void ExchangeManager::AbortExchangeForFabricExceptOne(FabricIndex fabricIndex, ExchangeContext * exception) +{ + mContextPool.ForEachActiveObject([&](auto * ec) { + if (ec->HasSessionHandle() && ec->GetSessionHandle()->GetPeer().GetFabricIndex() == fabricIndex) + { + if (ec == exception) + ec->SetAutoReleaseSession(); + else + ec->Abort(); + } + return Loop::Continue; + }); +} + void ExchangeManager::AbortExchangeForNodeExceptOne(const ScopedNodeId & node, ExchangeContext * exception) { mContextPool.ForEachActiveObject([&](auto * ec) { diff --git a/src/messaging/ExchangeMgr.h b/src/messaging/ExchangeMgr.h index 3411c8dfdf444e..53e23b23cd4367 100644 --- a/src/messaging/ExchangeMgr.h +++ b/src/messaging/ExchangeMgr.h @@ -183,8 +183,9 @@ class DLL_EXPORT ExchangeManager : public SessionMessageDelegate */ void CloseAllContextsForDelegate(const ExchangeDelegate * delegate); - // This API is used by UpdateNOC command, to invalid all exchanges except the given one. - void AbortExchangeForNodeExceptOne(const ScopedNodeId & node, ExchangeContext * stay); + // This 2 APIs are used by UpdateNOC command, to invalid all exchanges except the given one. + void AbortExchangeForFabricExceptOne(FabricIndex fabricIndex, ExchangeContext * exception); + void AbortExchangeForNodeExceptOne(const ScopedNodeId & node, ExchangeContext * exception); SessionManager * GetSessionManager() const { return mSessionManager; } diff --git a/src/transport/SessionManager.cpp b/src/transport/SessionManager.cpp index d8333ce1ab9425..4560566656e547 100644 --- a/src/transport/SessionManager.cpp +++ b/src/transport/SessionManager.cpp @@ -387,6 +387,20 @@ void SessionManager::ExpireAllPASEPairings() }); } +void SessionManager::ReleaseSessionForFabricExceptOne(FabricIndex fabricIndex, const SessionHandle & exception) +{ + mSecureSessions.ForEachSession([&](auto session) { + if (session->GetPeer().GetFabricIndex() == fabricIndex) + { + if (session == exception->AsSecureSession()) + session->MarkForInactive(); + else + session->MarkForRemoval(); + } + return Loop::Continue; + }); +} + void SessionManager::ReleaseSessionForNodeExceptOne(const ScopedNodeId & node, const SessionHandle & exception) { mSecureSessions.ForEachSession([&](auto session) { diff --git a/src/transport/SessionManager.h b/src/transport/SessionManager.h index 6e6435928dc0f9..e8c8f85d47aa82 100644 --- a/src/transport/SessionManager.h +++ b/src/transport/SessionManager.h @@ -177,7 +177,8 @@ class DLL_EXPORT SessionManager : public TransportMgrDelegate void ExpireAllPASEPairings(); // This API is used by UpdateNOC command, to invalid all sessions except the given one. - void ReleaseSessionForNodeExceptOne(const ScopedNodeId & node, const SessionHandle & stay); + void ReleaseSessionForFabricExceptOne(FabricIndex fabricIndex, const SessionHandle & exception); + void ReleaseSessionForNodeExceptOne(const ScopedNodeId & node, const SessionHandle & exception); /** * @brief From 3c7158f977e9f637eeeb972b141c3b1f5ad584a1 Mon Sep 17 00:00:00 2001 From: Zang MingJie Date: Tue, 14 Jun 2022 22:49:32 +0800 Subject: [PATCH 4/8] Resolve comments: adjust function name --- src/messaging/ExchangeMgr.cpp | 18 ++---------------- src/messaging/ExchangeMgr.h | 6 +++--- src/transport/SessionManager.cpp | 2 +- src/transport/SessionManager.h | 6 +++--- 4 files changed, 9 insertions(+), 23 deletions(-) diff --git a/src/messaging/ExchangeMgr.cpp b/src/messaging/ExchangeMgr.cpp index 00b0cf2a5fe60e..b9e133e75cdd53 100644 --- a/src/messaging/ExchangeMgr.cpp +++ b/src/messaging/ExchangeMgr.cpp @@ -376,26 +376,12 @@ void ExchangeManager::CloseAllContextsForDelegate(const ExchangeDelegate * deleg }); } -void ExchangeManager::AbortExchangeForFabricExceptOne(FabricIndex fabricIndex, ExchangeContext * exception) +void ExchangeManager::AbortExchangesForFabricExceptOne(FabricIndex fabricIndex, ExchangeContext * deferred) { mContextPool.ForEachActiveObject([&](auto * ec) { if (ec->HasSessionHandle() && ec->GetSessionHandle()->GetPeer().GetFabricIndex() == fabricIndex) { - if (ec == exception) - ec->SetAutoReleaseSession(); - else - ec->Abort(); - } - return Loop::Continue; - }); -} - -void ExchangeManager::AbortExchangeForNodeExceptOne(const ScopedNodeId & node, ExchangeContext * exception) -{ - mContextPool.ForEachActiveObject([&](auto * ec) { - if (ec->HasSessionHandle() && ec->GetSessionHandle()->GetPeer() == node) - { - if (ec == exception) + if (ec == deferred) ec->SetAutoReleaseSession(); else ec->Abort(); diff --git a/src/messaging/ExchangeMgr.h b/src/messaging/ExchangeMgr.h index 53e23b23cd4367..69dbd80a79ff40 100644 --- a/src/messaging/ExchangeMgr.h +++ b/src/messaging/ExchangeMgr.h @@ -183,9 +183,9 @@ class DLL_EXPORT ExchangeManager : public SessionMessageDelegate */ void CloseAllContextsForDelegate(const ExchangeDelegate * delegate); - // This 2 APIs are used by UpdateNOC command, to invalid all exchanges except the given one. - void AbortExchangeForFabricExceptOne(FabricIndex fabricIndex, ExchangeContext * exception); - void AbortExchangeForNodeExceptOne(const ScopedNodeId & node, ExchangeContext * exception); + // This API is used by UpdateNOC command, to abort all exchanges except the given one, whose abort is deferred until UpdateNOC + // command finishing its work. + void AbortExchangesForFabricExceptOne(FabricIndex fabricIndex, ExchangeContext * deferred); SessionManager * GetSessionManager() const { return mSessionManager; } diff --git a/src/transport/SessionManager.cpp b/src/transport/SessionManager.cpp index 4560566656e547..8c90fccf145ee4 100644 --- a/src/transport/SessionManager.cpp +++ b/src/transport/SessionManager.cpp @@ -387,7 +387,7 @@ void SessionManager::ExpireAllPASEPairings() }); } -void SessionManager::ReleaseSessionForFabricExceptOne(FabricIndex fabricIndex, const SessionHandle & exception) +void SessionManager::ReleaseSessionsForFabricExceptOne(FabricIndex fabricIndex, const SessionHandle & deferred) { mSecureSessions.ForEachSession([&](auto session) { if (session->GetPeer().GetFabricIndex() == fabricIndex) diff --git a/src/transport/SessionManager.h b/src/transport/SessionManager.h index e8c8f85d47aa82..c57f7089f04725 100644 --- a/src/transport/SessionManager.h +++ b/src/transport/SessionManager.h @@ -176,9 +176,9 @@ class DLL_EXPORT SessionManager : public TransportMgrDelegate void ExpireAllPairingsForFabric(FabricIndex fabric); void ExpireAllPASEPairings(); - // This API is used by UpdateNOC command, to invalid all sessions except the given one. - void ReleaseSessionForFabricExceptOne(FabricIndex fabricIndex, const SessionHandle & exception); - void ReleaseSessionForNodeExceptOne(const ScopedNodeId & node, const SessionHandle & exception); + // 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. + void ReleaseSessionsForFabricExceptOne(FabricIndex fabricIndex, const SessionHandle & deferred); /** * @brief From 4355c0364b143f38ee69a9bdd51cb60c222a0147 Mon Sep 17 00:00:00 2001 From: Zang MingJie Date: Tue, 14 Jun 2022 23:08:24 +0800 Subject: [PATCH 5/8] Restyle --- src/messaging/ReliableMessageContext.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/messaging/ReliableMessageContext.h b/src/messaging/ReliableMessageContext.h index 778ec6ce631e12..d4a89c73237e86 100644 --- a/src/messaging/ReliableMessageContext.h +++ b/src/messaging/ReliableMessageContext.h @@ -121,7 +121,6 @@ class ReliableMessageContext /// Determine whether this exchange is requesting Sleepy End Device active mode bool IsRequestingActiveMode() const; - /// Determine whether this exchange is a EphemeralExchange for replying a StandaloneAck bool IsEphemeralExchange() const; @@ -167,7 +166,6 @@ class ReliableMessageContext /// When set, signifies that the exchange is requesting Sleepy End Device active mode. kFlagActiveMode = (1u << 8), - /// When set, signifies that the exchange created sorely for replying a StandaloneAck kFlagEphemeralExchange = (1u << 9), From 35a3ca98e9d9fca522e64de6aad7ad53dc279757 Mon Sep 17 00:00:00 2001 From: Zang MingJie Date: Wed, 15 Jun 2022 00:32:18 +0800 Subject: [PATCH 6/8] Fix rebase conflict --- src/transport/SessionManager.cpp | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/src/transport/SessionManager.cpp b/src/transport/SessionManager.cpp index 8c90fccf145ee4..1a6e7939c5b702 100644 --- a/src/transport/SessionManager.cpp +++ b/src/transport/SessionManager.cpp @@ -392,21 +392,7 @@ void SessionManager::ReleaseSessionsForFabricExceptOne(FabricIndex fabricIndex, mSecureSessions.ForEachSession([&](auto session) { if (session->GetPeer().GetFabricIndex() == fabricIndex) { - if (session == exception->AsSecureSession()) - session->MarkForInactive(); - else - session->MarkForRemoval(); - } - return Loop::Continue; - }); -} - -void SessionManager::ReleaseSessionForNodeExceptOne(const ScopedNodeId & node, const SessionHandle & exception) -{ - mSecureSessions.ForEachSession([&](auto session) { - if (session->GetPeer() == node) - { - if (session == exception->AsSecureSession()) + if (session == deferred->AsSecureSession()) session->MarkForInactive(); else session->MarkForRemoval(); From 42049c5e44bb6aa077649ad2f7828acc646de1e8 Mon Sep 17 00:00:00 2001 From: Zang MingJie Date: Sat, 18 Jun 2022 01:20:25 +0800 Subject: [PATCH 7/8] Apply suggestions from code review Co-authored-by: Boris Zbarsky --- src/messaging/ExchangeMgr.cpp | 2 +- src/messaging/ExchangeMgr.h | 6 ++++-- src/messaging/ReliableMessageContext.h | 7 ++++--- src/transport/SecureSession.cpp | 2 +- src/transport/SecureSession.h | 10 ++++++---- 5 files changed, 16 insertions(+), 11 deletions(-) diff --git a/src/messaging/ExchangeMgr.cpp b/src/messaging/ExchangeMgr.cpp index b9e133e75cdd53..05b70adec8adf9 100644 --- a/src/messaging/ExchangeMgr.cpp +++ b/src/messaging/ExchangeMgr.cpp @@ -379,7 +379,7 @@ void ExchangeManager::CloseAllContextsForDelegate(const ExchangeDelegate * deleg void ExchangeManager::AbortExchangesForFabricExceptOne(FabricIndex fabricIndex, ExchangeContext * deferred) { mContextPool.ForEachActiveObject([&](auto * ec) { - if (ec->HasSessionHandle() && ec->GetSessionHandle()->GetPeer().GetFabricIndex() == fabricIndex) + if (ec->HasSessionHandle() && ec->GetSessionHandle()->GetFabricIndex() == fabricIndex) { if (ec == deferred) ec->SetAutoReleaseSession(); diff --git a/src/messaging/ExchangeMgr.h b/src/messaging/ExchangeMgr.h index 69dbd80a79ff40..910dcb8ee81db3 100644 --- a/src/messaging/ExchangeMgr.h +++ b/src/messaging/ExchangeMgr.h @@ -183,8 +183,10 @@ class DLL_EXPORT ExchangeManager : public SessionMessageDelegate */ void CloseAllContextsForDelegate(const ExchangeDelegate * delegate); - // This API is used by UpdateNOC command, to abort all exchanges except the given one, whose abort is deferred until UpdateNOC - // command finishing its work. + // 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); SessionManager * GetSessionManager() const { return mSessionManager; } diff --git a/src/messaging/ReliableMessageContext.h b/src/messaging/ReliableMessageContext.h index d4a89c73237e86..6e94da51f4a739 100644 --- a/src/messaging/ReliableMessageContext.h +++ b/src/messaging/ReliableMessageContext.h @@ -124,9 +124,10 @@ class ReliableMessageContext /// Determine whether this exchange is a EphemeralExchange for replying a StandaloneAck bool IsEphemeralExchange() const; - // For UpdateNOC command, I'm the last exchange, I must release the session when finishing my work. + // If SetAutoReleaseSession() 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 IsAutoReleaseSession(); + bool ReleaseSessionOnDestruction(); /** * Get the reliable message manager that corresponds to this reliable @@ -169,7 +170,7 @@ class ReliableMessageContext /// When set, signifies that the exchange created sorely for replying a StandaloneAck kFlagEphemeralExchange = (1u << 9), - /// When set, automatically release the session when finishing its work. Used by UpdateNOC command + /// When set, automatically release the session when this exchange is destroyed. kFlagAutoReleaseSession = (1u << 10), }; diff --git a/src/transport/SecureSession.cpp b/src/transport/SecureSession.cpp index 6aa5cc044a868f..e63d2d861e4ce1 100644 --- a/src/transport/SecureSession.cpp +++ b/src/transport/SecureSession.cpp @@ -61,7 +61,7 @@ void SecureSession::MarkForInactive() VerifyOrDie(false); return; case State::kActive: - // By setting this state, IsActiveSession() will return false, which prevent creating new exchanges. + // By setting this state, IsActiveSession() will return false, which prevents creating new exchanges. mState = State::kInactive; return; case State::kInactive: diff --git a/src/transport/SecureSession.h b/src/transport/SecureSession.h index 9978918929a1d2..6ce8202c65b2d3 100644 --- a/src/transport/SecureSession.h +++ b/src/transport/SecureSession.h @@ -144,7 +144,7 @@ class SecureSession : public Session, public ReferenceCounted Date: Sat, 18 Jun 2022 02:16:13 +0800 Subject: [PATCH 8/8] Resolve comments --- src/messaging/ExchangeContext.cpp | 2 +- src/messaging/ExchangeMgr.cpp | 4 ++++ src/messaging/ExchangeMgr.h | 5 ++--- src/messaging/ReliableMessageContext.h | 2 +- src/transport/SecureSession.cpp | 4 ++-- src/transport/SecureSession.h | 4 ++-- src/transport/SessionManager.cpp | 7 +++++-- src/transport/SessionManager.h | 5 +++-- 8 files changed, 20 insertions(+), 13 deletions(-) diff --git a/src/messaging/ExchangeContext.cpp b/src/messaging/ExchangeContext.cpp index 08d916a15246e2..422a2f88e34ce4 100644 --- a/src/messaging/ExchangeContext.cpp +++ b/src/messaging/ExchangeContext.cpp @@ -325,7 +325,7 @@ ExchangeContext::~ExchangeContext() VerifyOrDie(mExchangeMgr != nullptr && GetReferenceCount() == 0); VerifyOrDie(!IsAckPending()); - if (IsAutoReleaseSession() && mSession) + if (ReleaseSessionOnDestruction() && mSession) mSession->AsSecureSession()->MarkForRemoval(); #if CONFIG_DEVICE_LAYER && CHIP_DEVICE_CONFIG_ENABLE_SED diff --git a/src/messaging/ExchangeMgr.cpp b/src/messaging/ExchangeMgr.cpp index 05b70adec8adf9..b01d5b4fe4cb5c 100644 --- a/src/messaging/ExchangeMgr.cpp +++ b/src/messaging/ExchangeMgr.cpp @@ -378,6 +378,8 @@ 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) { @@ -388,6 +390,8 @@ void ExchangeManager::AbortExchangesForFabricExceptOne(FabricIndex fabricIndex, } 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..9b2f8d67ab0539 100644 --- a/src/messaging/ReliableMessageContext.h +++ b/src/messaging/ReliableMessageContext.h @@ -258,7 +258,7 @@ inline void ReliableMessageContext::SetAutoReleaseSession() mFlags.Set(Flags::kFlagAutoReleaseSession, true); } -inline bool ReliableMessageContext::IsAutoReleaseSession() +inline bool ReliableMessageContext::ReleaseSessionOnDestruction() { return mFlags.Has(Flags::kFlagAutoReleaseSession); } 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); /**