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 <[email protected]> * Address review comment
project-chip · Aug 8, 2022 · b52e56d · b52e56d
1 parent 9665a20
commit b52e56d
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 4 deletions.
diff --git 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
@@ -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),