Apply suggestions from code review

Co-authored-by: Boris Zbarsky <[email protected]>

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();

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; }

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),
     };
 

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:

src/transport/SecureSession.h

@@ -144,7 +144,7 @@ class SecureSession : public Session, public ReferenceCounted<SecureSession, Sec
     /// @brief Mark as pending removal, all holders to this session will be cleared, and disallow future grab
     void MarkForRemoval();
 
-    // Used by UpdateNOC command to prevent any new exchange created on the session.
+    // Used to prevent any new exchange created on the session while the existing exchanges finish their work.
     void MarkForInactive();
 
     Session::SessionType GetSessionType() const override { return Session::SessionType::kSecure; }
@@ -253,9 +253,11 @@ class SecureSession : public Session, public ReferenceCounted<SecureSession, Sec
         // session object will be released automatically.
         kPendingRemoval = 3,
 
-        // For UpdateNOC command, the session still functional, but it can't
-        // yield any new exchanges, the last exchange running UpdateNOC command
-        // will close the session soon.
+        // The session is still functional, but it can't yield any new exchanges,
+        // This is meant to be used in conjunction with 
+        // ExchangeManager::AbortExchangesForFabricExceptOne, with the one
+        // exceptional exchange handling moving this session out of this state when
+        // it finishes whatever it needs the session for.
         kInactive = 4,
     };