Clarify the logic in ExchangeContext::OnSessionReleased.

src/messaging/ExchangeContext.cpp

@@ -369,18 +369,34 @@ void ExchangeContext::OnSessionReleased()
         return;
     }
 
+    // Hold a ref to ourselves so we can make calls into our delegate that might
+    // decrease our refcount without worrying about use-after-free.
     ExchangeHandle ref(*this);
 
     if (IsResponseExpected())
     {
         // If we're waiting on a response, we now know it's never going to show up
         // and we should notify our delegate accordingly.
         CancelResponseTimer();
-        SetResponseExpected(false);
-        NotifyResponseTimeout();
+        // We want to Abort, not just Close, so that RMP bits are cleared, so
+        // don't let NotifyResponseTimeout close us.
+        NotifyResponseTimeout(/* aCloseIfNeeded = */ false);
+        Abort();
+    }
+    else
+    {
+        // Either we're expecting a send or we are in our "just allocated, first
+        // send has not happened yet" state.
+        //
+        // Just mark ourselves as closed.  The consumer is responsible for
+        // releasing us.  See documentation for
+        // ExchangeDelegate::OnExchangeClosing.
+        if (IsSendExpected())
+        {
+            mFlags.Clear(Flags::kFlagWillSendMessage);
+        }
+        DoClose(true /* clearRetransTable */);
     }
-
-    DoClose(true /* clearRetransTable */);
 }
 
 CHIP_ERROR ExchangeContext::StartResponseTimer()
@@ -414,10 +430,10 @@ void ExchangeContext::HandleResponseTimeout(System::Layer * aSystemLayer, void *
     if (ec == nullptr)
         return;
 
-    ec->NotifyResponseTimeout();
+    ec->NotifyResponseTimeout(/* aCloseIfNeeded = */ true);
 }
 
-void ExchangeContext::NotifyResponseTimeout()
+void ExchangeContext::NotifyResponseTimeout(bool aCloseIfNeeded)
 {
     SetResponseExpected(false);
 
@@ -429,7 +445,10 @@ void ExchangeContext::NotifyResponseTimeout()
         delegate->OnResponseTimeout(this);
     }
 
-    MessageHandled();
+    if (aCloseIfNeeded)
+    {
+        MessageHandled();
+    }
 }
 
 CHIP_ERROR ExchangeContext::HandleMessage(uint32_t messageCounter, const PayloadHeader & payloadHeader, MessageFlags msgFlags,

src/messaging/ExchangeContext.h

@@ -238,9 +238,10 @@ class DLL_EXPORT ExchangeContext : public ReliableMessageContext,
 
     /**
      * Notify our delegate, if any, that we have timed out waiting for a
-     * response.
+     * response.  If aCloseIfNeeded is true, check whether the exchange needs to
+     * be closed.
      */
-    void NotifyResponseTimeout();
+    void NotifyResponseTimeout(bool aCloseIfNeeded);
 
     CHIP_ERROR StartResponseTimer();
 

src/messaging/ExchangeDelegate.h

@@ -91,7 +91,16 @@ class DLL_EXPORT ExchangeDelegate
     /**
      * @brief
      *   This function is the protocol callback to invoke when the associated
-     *   exchange context is being closed
+     *   exchange context is being closed.
+     *
+     *   If the exchange was in a state where it was expecting a message to be
+     *   sent due to an earlier WillSendMessage call or because the exchange has
+     *   just been created as an initiator, the consumer is holding a reference
+     *   to the exchange and it's the consumer's responsibility to call
+     *   Release() on the exchange at some point.  The usual way this happens is
+     *   that the consumer tries to send its message, that fails, and the
+     *   consumer calls Close() on the exchange.  Calling Close() after an
+     *   OnExchangeClosing() notification is allowed in this situation.
      *
      *  @param[in]    ec            A pointer to the ExchangeContext object.
      */