Invalid CommandHandler::Handle when chip stack shutdown

src/app/CommandHandler.cpp

@@ -118,8 +118,12 @@ void CommandHandler::Close()
 {
     mSuppressResponse = false;
     MoveToState(CommandState::AwaitingDestruction);
-    // We must finish all async work before we can shut down a CommandHandler. The actual CommandHandler MUST finish their work in
-    // reasonable time or there is a bug.
+
+    // We must finish all async work before we can shut down a CommandHandler. The actual CommandHandler MUST finish their work
+    // in reasonable time or there is a bug.
+    //
+    // The only case for releasing CommandHandler without CommandHandler::Handle releasing its reference is the stack shutting down,
+    // in which case Close() is not called.
     VerifyOrDieWithMsg(mRefCount == 0, DataManagement, "CommandHandler::Close() called with %zu unfinished async work items",
                        mRefCount);
 
@@ -368,6 +372,21 @@ TLV::TLVWriter * CommandHandler::GetCommandDataIBTLVWriter()
     }
 }
 
+CommandHandler * CommandHandler::Handle::Get()
+{
+    return (mMagic == InteractionModelEngine::GetInstance()->GetMagicNumber()) ? mpHandler : nullptr;
+}
+
+CommandHandler::Handle::Handle(CommandHandler * handle)
+{
+    if (handle != nullptr)
+    {
+        handle->IncRef();
+        mpHandler = handle;
+        mMagic    = InteractionModelEngine::GetInstance()->GetMagicNumber();
+    }
+}
+
 } // namespace app
 } // namespace chip
 

src/app/CommandHandler.h

@@ -67,21 +67,21 @@ class CommandHandler : public Command
         Handle(Handle && handle)
         {
             mpHandler        = handle.mpHandler;
+            mMagic           = handle.mMagic;
             handle.mpHandler = nullptr;
+            handle.mMagic    = 0;
         }
         Handle(decltype(nullptr)) {}
-        Handle(CommandHandler * handle)
-        {
-            handle->IncRef();
-            mpHandler = handle;
-        }
+        Handle(CommandHandler * handle);
         ~Handle() { Release(); }
 
         Handle & operator=(Handle && handle)
         {
             Release();
             mpHandler        = handle.mpHandler;
+            mMagic           = handle.mMagic;
             handle.mpHandler = nullptr;
+            handle.mMagic    = 0;
             return *this;
         }
 
@@ -91,19 +91,21 @@ class CommandHandler : public Command
             return *this;
         }
 
-        CommandHandler * operator->() { return mpHandler; }
+        CommandHandler * Get();
 
         void Release()
         {
             if (mpHandler != nullptr)
             {
                 mpHandler->DecRef();
                 mpHandler = nullptr;
+                mMagic    = 0;
             }
         }
 
     private:
         CommandHandler * mpHandler = nullptr;
+        uint32_t mMagic            = 0;
     };
 
     /*
@@ -183,11 +185,11 @@ class CommandHandler : public Command
      */
     CHIP_ERROR AllocateBuffer();
 
-    //
-    // Called internally to signal the completion of all work on this object, gracefully close the
-    // exchange (by calling into the base class) and finally, signal to a registerd callback that it's
-    // safe to release this object.
-    //
+    /**
+     * Called internally to signal the completion of all work on this object, gracefully close the
+     * exchange (by calling into the base class) and finally, signal to a registerd callback that it's
+     * safe to release this object.
+     */
     void Close();
 
     CHIP_ERROR ProcessCommandDataIB(CommandDataIB::Parser & aCommandElement);

src/app/InteractionModelEngine.cpp

@@ -56,11 +56,15 @@ CHIP_ERROR InteractionModelEngine::Init(Messaging::ExchangeManager * apExchangeM
     mClusterInfoPool[CHIP_IM_SERVER_MAX_NUM_PATH_GROUPS - 1].mpNext = nullptr;
     mpNextAvailableClusterInfo                                      = mClusterInfoPool;
 
+    mMagic++;
+
     return CHIP_NO_ERROR;
 }
 
 void InteractionModelEngine::Shutdown()
 {
+    // Increase magic number to invalid all Handle-s.
+    mMagic++;
     //
     // Since modifying the pool during iteration is generally frowned upon,
     // I've chosen to just destroy the object but not necessarily de-allocate it.

src/app/InteractionModelEngine.h

@@ -178,6 +178,11 @@ class InteractionModelEngine : public Messaging::ExchangeDelegate, public Comman
 
     uint16_t GetWriteClientArrayIndex(const WriteClient * const apWriteClient) const;
 
+    /**
+     * The Magic number of this InteractionModelEngine, the magic number is set during Init()
+     */
+    uint32_t GetMagicNumber() { return mMagic; }
+
     reporting::Engine & GetReportingEngine() { return mReportingEngine; }
 
     void ReleaseClusterInfoList(ClusterInfo *& aClusterInfo);
@@ -235,6 +240,10 @@ class InteractionModelEngine : public Messaging::ExchangeDelegate, public Comman
     reporting::Engine mReportingEngine;
     ClusterInfo mClusterInfoPool[CHIP_IM_SERVER_MAX_NUM_PATH_GROUPS];
     ClusterInfo * mpNextAvailableClusterInfo = nullptr;
+
+    // A magic number for tracking values between stack Shutdown()-s and Init()-s.
+    // An ObjectHandle is valid iff. its magic equals to this one.
+    uint32_t mMagic = 0;
 };
 
 void DispatchSingleClusterCommand(const ConcreteCommandPath & aCommandPath, chip::TLV::TLVReader & aReader,