From 105096099e2f1c183552da5bcbfcfbe7b01b68e5 Mon Sep 17 00:00:00 2001
From: Boris Zbarsky <bzbarsky@apple.com>
Date: Mon, 6 Mar 2023 19:35:44 -0500
Subject: [PATCH] Make it easier to catch bugs in async command handling.
 (#25402)

If GetSubjectDescriptor or GetAccessingFabricIndex were called on a
CommandHandler after the command handling had gone async, it would sometimes
crash, and sometimes not, depending on whether sessions had been evicted or not.

Make that situation always crash, so that it will be caught more easily in
testing.
---
 src/app/CommandHandler.cpp |  3 +++
 src/app/CommandHandler.h   | 23 ++++++++++++++++++++++-
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/src/app/CommandHandler.cpp b/src/app/CommandHandler.cpp
index 6b694ba8a4b14d..6444a2faba9951 100644
--- a/src/app/CommandHandler.cpp
+++ b/src/app/CommandHandler.cpp
@@ -93,6 +93,8 @@ void CommandHandler::OnInvokeCommandRequest(Messaging::ExchangeContext * ec, con
         StatusResponse::Send(status, mExchangeCtx.Get(), false /*aExpectResponse*/);
         mSentStatusResponse = true;
     }
+
+    mGoneAsync = true;
 }
 
 Status CommandHandler::ProcessInvokeRequest(System::PacketBufferHandle && payload, bool isTimedInvoke)
@@ -577,6 +579,7 @@ TLV::TLVWriter * CommandHandler::GetCommandDataIBTLVWriter()
 
 FabricIndex CommandHandler::GetAccessingFabricIndex() const
 {
+    VerifyOrDie(!mGoneAsync);
     return mExchangeCtx->GetSessionHandle()->GetFabricIndex();
 }
 
diff --git a/src/app/CommandHandler.h b/src/app/CommandHandler.h
index 054deebe85e4dd..cc175a9a125cd8 100644
--- a/src/app/CommandHandler.h
+++ b/src/app/CommandHandler.h
@@ -182,6 +182,13 @@ class CommandHandler : public Messaging::ExchangeDelegate
     CHIP_ERROR PrepareStatus(const ConcreteCommandPath & aCommandPath);
     CHIP_ERROR FinishStatus();
     TLV::TLVWriter * GetCommandDataIBTLVWriter();
+
+    /**
+     * GetAccessingFabricIndex() may only be called during synchronous command
+     * processing.  Anything that runs async (while holding a
+     * CommandHandler::Handle or equivalent) must not call this method, because
+     * it will not work right if the session we're using was evicted.
+     */
     FabricIndex GetAccessingFabricIndex() const;
 
     /**
@@ -272,7 +279,17 @@ class CommandHandler : public Messaging::ExchangeDelegate
         msgContext->FlushAcks();
     }
 
-    Access::SubjectDescriptor GetSubjectDescriptor() const { return mExchangeCtx->GetSessionHandle()->GetSubjectDescriptor(); }
+    /**
+     * GetSubjectDescriptor() may only be called during synchronous command
+     * processing.  Anything that runs async (while holding a
+     * CommandHandler::Handle or equivalent) must not call this method, because
+     * it might not work right if the session we're using was evicted.
+     */
+    Access::SubjectDescriptor GetSubjectDescriptor() const
+    {
+        VerifyOrDie(!mGoneAsync);
+        return mExchangeCtx->GetSessionHandle()->GetSubjectDescriptor();
+    }
 
 private:
     friend class TestCommandInteraction;
@@ -394,6 +411,10 @@ class CommandHandler : public Messaging::ExchangeDelegate
     chip::System::PacketBufferTLVWriter mCommandMessageWriter;
     TLV::TLVWriter mBackupWriter;
     bool mBufferAllocated = false;
+    // If mGoneAsync is true, we have finished out initial processing of the
+    // incoming invoke.  After this point, our session could go away at any
+    // time.
+    bool mGoneAsync = false;
 };
 
 } // namespace app