From 90bae8d118c67c63e0ea6617349e562d8d850930 Mon Sep 17 00:00:00 2001
From: Boris Zbarsky <bzbarsky@apple.com>
Date: Mon, 11 Sep 2023 15:07:24 -0400
Subject: [PATCH] Don't mark session defunct on response timeout if the message
 was acked.

If we got an ack, the session is fine and there is no reason to mark it defunct.

Fixes https://github.com/project-chip/connectedhomeip/issues/29165
---
 src/messaging/ExchangeContext.cpp    | 13 ++++++++++---
 src/messaging/ReliableMessageMgr.cpp | 16 +++++++++++++---
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/src/messaging/ExchangeContext.cpp b/src/messaging/ExchangeContext.cpp
index 9d1e7ca06fce23..8c085025456ba8 100644
--- a/src/messaging/ExchangeContext.cpp
+++ b/src/messaging/ExchangeContext.cpp
@@ -500,11 +500,18 @@ void ExchangeContext::NotifyResponseTimeout(bool aCloseIfNeeded)
     // evicted.
     if (mSession)
     {
-        if (mSession->IsSecureSession() && mSession->AsSecureSession()->IsCASESession())
+        // If we timed out _after_ getting an ack for the message, that means
+        // the session is probably fine (since our message and the ack got
+        // through), so don't mark the session defunct unless we have an
+        // un-acked message here.
+        if (IsMessageNotAcked())
         {
-            mSession->AsSecureSession()->MarkAsDefunct();
+            if (mSession->IsSecureSession() && mSession->AsSecureSession()->IsCASESession())
+            {
+                mSession->AsSecureSession()->MarkAsDefunct();
+            }
+            mSession->DispatchSessionEvent(&SessionDelegate::OnSessionHang);
         }
-        mSession->DispatchSessionEvent(&SessionDelegate::OnSessionHang);
     }
 
     ExchangeDelegate * delegate = GetDelegate();
diff --git a/src/messaging/ReliableMessageMgr.cpp b/src/messaging/ReliableMessageMgr.cpp
index 5dc397a39b9ad8..7bfca6342c889d 100644
--- a/src/messaging/ReliableMessageMgr.cpp
+++ b/src/messaging/ReliableMessageMgr.cpp
@@ -131,19 +131,22 @@ void ReliableMessageMgr::ExecuteActions()
 
         if (sendCount == CHIP_CONFIG_RMP_DEFAULT_MAX_RETRANS)
         {
+            // Make sure our exchange stays alive until we are done working with it.
+            ExchangeHandle ec(entry->ec);
+
             ChipLogError(ExchangeManager,
                          "Failed to Send CHIP MessageCounter:" ChipLogFormatMessageCounter " on exchange " ChipLogFormatExchange
                          " sendCount: %u max retries: %d",
-                         messageCounter, ChipLogValueExchange(&entry->ec.Get()), sendCount, CHIP_CONFIG_RMP_DEFAULT_MAX_RETRANS);
+                         messageCounter, ChipLogValueExchange(&ec.Get()), sendCount, CHIP_CONFIG_RMP_DEFAULT_MAX_RETRANS);
 
             // Don't check whether the session in the exchange is valid, because when the session is released, the retrans entry is
             // cleared inside ExchangeContext::OnSessionReleased, so the session must be valid if the entry exists.
-            SessionHandle session = entry->ec->GetSessionHandle();
+            SessionHandle session = ec->GetSessionHandle();
 
             // If the exchange is expecting a response, it will handle sending
             // this notification once it detects that it has not gotten a
             // response.  Otherwise, we need to do it.
-            if (!entry->ec->IsResponseExpected())
+            if (!ec->IsResponseExpected())
             {
                 if (session->IsSecureSession() && session->AsSecureSession()->IsCASESession())
                 {
@@ -154,6 +157,13 @@ void ReliableMessageMgr::ExecuteActions()
 
             // Do not StartTimer, we will schedule the timer at the end of the timer handler.
             mRetransTable.ReleaseObject(entry);
+
+            // That marked the exchange as not having an un-acked message... but
+            // of course it _does_ have an un-acked message and we have just
+            // given up on waiting for the ack.
+
+            ec->GetReliableMessageContext()->SetMessageNotAcked(true);
+
             return Loop::Continue;
         }