From b52e56daf28e0f107774dcd9cfdef6b9974a7fcd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Damian=20Kr=C3=B3lik?=
 <66667989+Damian-Nordic@users.noreply.github.com>
Date: Mon, 8 Aug 2022 09:06:26 +0200
Subject: [PATCH] Fix crash on fail-safe timeout due to closed exchange
 (#21590)

* Fix crash on fail-safe timeout due to closed exchange

When CommandHandler is released it dereferences an exchange
holder to figure out if the status response can be sent.
The exchange holder, however, can be nulled out at this
point.

Signed-off-by: Damian Krolik <damian.krolik@nordicsemi.no>

* Address review comment
---
 src/app/CommandHandler.cpp               | 15 ++++++++++----
 src/app/tests/TestCommandInteraction.cpp | 26 ++++++++++++++++++++++++
 2 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/src/app/CommandHandler.cpp b/src/app/CommandHandler.cpp
index d17c4f36cd33e1..9ad0b4eca7cbab 100644
--- a/src/app/CommandHandler.cpp
+++ b/src/app/CommandHandler.cpp
@@ -186,12 +186,19 @@ void CommandHandler::DecrementHoldOff()
         return;
     }
 
-    if (!mExchangeCtx->IsGroupExchangeContext() && !mSentStatusResponse)
+    if (!mSentStatusResponse)
     {
-        CHIP_ERROR err = SendCommandResponse();
-        if (err != CHIP_NO_ERROR)
+        if (!mExchangeCtx)
+        {
+            ChipLogProgress(DataManagement, "Skipping command response: exchange context is null");
+        }
+        else if (!mExchangeCtx->IsGroupExchangeContext())
         {
-            ChipLogError(DataManagement, "Failed to send command response: %" CHIP_ERROR_FORMAT, err.Format());
+            CHIP_ERROR err = SendCommandResponse();
+            if (err != CHIP_NO_ERROR)
+            {
+                ChipLogError(DataManagement, "Failed to send command response: %" CHIP_ERROR_FORMAT, err.Format());
+            }
         }
     }
 
diff --git a/src/app/tests/TestCommandInteraction.cpp b/src/app/tests/TestCommandInteraction.cpp
index 41245dca9be0a5..3821b7fad536eb 100644
--- a/src/app/tests/TestCommandInteraction.cpp
+++ b/src/app/tests/TestCommandInteraction.cpp
@@ -221,6 +221,7 @@ class TestCommandInteraction
     static void TestCommandHandlerWithProcessReceivedMsg(nlTestSuite * apSuite, void * apContext);
     static void TestCommandHandlerWithProcessReceivedEmptyDataMsg(nlTestSuite * apSuite, void * apContext);
     static void TestCommandHandlerRejectMultipleCommands(nlTestSuite * apSuite, void * apContext);
+    static void TestCommandHandlerReleaseWithExchangeClosed(nlTestSuite * apSuite, void * apContext);
 
     static void TestCommandSenderCommandSuccessResponseFlow(nlTestSuite * apSuite, void * apContext);
     static void TestCommandSenderCommandAsyncSuccessResponseFlow(nlTestSuite * apSuite, void * apContext);
@@ -927,6 +928,30 @@ void TestCommandInteraction::TestCommandHandlerRejectMultipleCommands(nlTestSuit
     NL_TEST_ASSERT(apSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0);
 }
 
+void TestCommandInteraction::TestCommandHandlerReleaseWithExchangeClosed(nlTestSuite * apSuite, void * apContext)
+{
+    TestContext & ctx = *static_cast<TestContext *>(apContext);
+    CHIP_ERROR err    = CHIP_NO_ERROR;
+
+    app::CommandSender commandSender(&mockCommandSenderDelegate, &ctx.GetExchangeManager());
+
+    AddInvokeRequestData(apSuite, apContext, &commandSender);
+    asyncCommandHandle = nullptr;
+    asyncCommand       = true;
+    err                = commandSender.SendCommandRequest(ctx.GetSessionBobToAlice());
+
+    NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
+
+    ctx.DrainAndServiceIO();
+
+    // Verify that async command handle has been allocated
+    NL_TEST_ASSERT(apSuite, asyncCommandHandle.Get() != nullptr);
+
+    // Close the exchange associated with the handle and verify the handle can be gracefully released
+    asyncCommandHandle.Get()->mExchangeCtx->Close();
+    asyncCommandHandle = nullptr;
+}
+
 } // namespace app
 } // namespace chip
 
@@ -948,6 +973,7 @@ const nlTest sTests[] =
     NL_TEST_DEF("TestCommandHandlerWithProcessReceivedNotExistCommand", chip::app::TestCommandInteraction::TestCommandHandlerWithProcessReceivedNotExistCommand),
     NL_TEST_DEF("TestCommandHandlerWithProcessReceivedEmptyDataMsg", chip::app::TestCommandInteraction::TestCommandHandlerWithProcessReceivedEmptyDataMsg),
     NL_TEST_DEF("TestCommandHandlerRejectMultipleCommands", chip::app::TestCommandInteraction::TestCommandHandlerRejectMultipleCommands),
+    NL_TEST_DEF("TestCommandHandlerReleaseWithExchangeClosed", chip::app::TestCommandInteraction::TestCommandHandlerReleaseWithExchangeClosed),
 
     NL_TEST_DEF("TestCommandSenderCommandSuccessResponseFlow", chip::app::TestCommandInteraction::TestCommandSenderCommandSuccessResponseFlow),
     NL_TEST_DEF("TestCommandSenderCommandAsyncSuccessResponseFlow", chip::app::TestCommandInteraction::TestCommandSenderCommandAsyncSuccessResponseFlow),