Introduce ExchangeAcceptor for unsolicited message handler (#17069)

* Introduce ExchangeAcceptor for unsolicited message handler * Resolve comments * Apply suggestions from code review Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Fix build * Fix build Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
project-chip · Jul 24, 2023 · 1602332 · 1602332
1 parent edc0efb
commit 1602332
Show file tree

Hide file tree

Showing 17 changed files with 186 additions and 33 deletions.
diff --git a/src/app/InteractionModelEngine.cpp b/src/app/InteractionModelEngine.cpp
@@ -415,6 +415,15 @@ CHIP_ERROR InteractionModelEngine::OnUnsolicitedReportData(Messaging::ExchangeCo
     return CHIP_NO_ERROR;
 }
 
+CHIP_ERROR InteractionModelEngine::OnUnsolicitedMessageReceived(const PayloadHeader & payloadHeader,
+                                                                ExchangeDelegate *& newDelegate)
+{
+    // TODO: Implement OnUnsolicitedMessageReceived, let messaging layer dispatch message to ReadHandler/ReadClient/TimedHandler
+    // directly.
+    newDelegate = this;
+    return CHIP_NO_ERROR;
+}
+
 CHIP_ERROR InteractionModelEngine::OnMessageReceived(Messaging::ExchangeContext * apExchangeContext,
                                                      const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload)
 {

diff --git a/src/app/InteractionModelEngine.h b/src/app/InteractionModelEngine.h
@@ -69,7 +69,8 @@ namespace app {
  * handlers
  *
  */
-class InteractionModelEngine : public Messaging::ExchangeDelegate,
+class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
+                               public Messaging::ExchangeDelegate,
                                public CommandHandler::Callback,
                                public ReadHandler::ManagementCallback
 {
@@ -267,6 +268,8 @@ class InteractionModelEngine : public Messaging::ExchangeDelegate,
 
     ReadHandler::ApplicationCallback * GetAppCallback() override { return mpReadHandlerApplicationCallback; }
 
+    CHIP_ERROR OnUnsolicitedMessageReceived(const PayloadHeader & payloadHeader, ExchangeDelegate *& newDelegate) override;
+
     /**
      * Called when Interaction Model receives a Command Request message.  Errors processing
      * the Command Request are handled entirely within this function. The caller pre-sets status to failure and the callee is

diff --git a/src/messaging/ExchangeDelegate.h b/src/messaging/ExchangeDelegate.h
@@ -100,5 +100,43 @@ class DLL_EXPORT ExchangeDelegate
     virtual ExchangeMessageDispatch & GetMessageDispatch() { return ApplicationExchangeDispatch::Instance(); }
 };
 
+/**
+ * @brief
+ *   This class handles unsolicited messages. The implementation can select an exchange delegate to use based on the payload header
+ * of the incoming message.
+ */
+class DLL_EXPORT UnsolicitedMessageHandler
+{
+public:
+    virtual ~UnsolicitedMessageHandler() {}
+
+    /**
+     * @brief
+     *   This function handles an unsolicited CHIP message.
+     *
+     *   If the implementation returns CHIP_NO_ERROR, it is expected to set newDelegate to the delegate to use for the exchange
+     *   handling the message.  The message layer will handle creating the exchange with this delegate.
+     *
+     *   If the implementation returns an error, message processing will be aborted for this message.
+     *
+     *  @param[in]  payloadHeader A reference to the PayloadHeader object for the unsolicited message.  The protocol and message
+     *                            type of this header match the UnsolicitedMessageHandler.
+     *  @param[out] newDelegate   A new exchange delegate to be used by the new exchange created to handle the message.
+     */
+    virtual CHIP_ERROR OnUnsolicitedMessageReceived(const PayloadHeader & payloadHeader, ExchangeDelegate *& newDelegate) = 0;
+
+    /**
+     * @brief
+     *   This function is called when OnUnsolicitedMessageReceived successfully returns a new delegate, but the session manager
+     *   fails to assign the delegate to a new exchange.  It can be used to free the delegate as needed.
+     *
+     *   Once an exchange is created with the delegate, the OnExchangeClosing notification can be used to free the delegate as
+     *   needed.
+     *
+     *  @param[in] delegate   The exchange delegate to be released.
+     */
+    virtual void OnExchangeCreationFailed(ExchangeDelegate * delegate) {}
+};
+
 } // namespace Messaging
 } // namespace chip
diff --git a/src/messaging/ExchangeMgr.cpp b/src/messaging/ExchangeMgr.cpp
@@ -114,15 +114,16 @@ ExchangeContext * ExchangeManager::NewContext(const SessionHandle & session, Exc
     return mContextPool.CreateObject(this, mNextExchangeId++, session, true, delegate);
 }
 
-CHIP_ERROR ExchangeManager::RegisterUnsolicitedMessageHandlerForProtocol(Protocols::Id protocolId, ExchangeDelegate * delegate)
+CHIP_ERROR ExchangeManager::RegisterUnsolicitedMessageHandlerForProtocol(Protocols::Id protocolId,
+                                                                         UnsolicitedMessageHandler * handler)
 {
-    return RegisterUMH(protocolId, kAnyMessageType, delegate);
+    return RegisterUMH(protocolId, kAnyMessageType, handler);
 }
 
 CHIP_ERROR ExchangeManager::RegisterUnsolicitedMessageHandlerForType(Protocols::Id protocolId, uint8_t msgType,
-                                                                     ExchangeDelegate * delegate)
+                                                                     UnsolicitedMessageHandler * handler)
 {
-    return RegisterUMH(protocolId, static_cast<int16_t>(msgType), delegate);
+    return RegisterUMH(protocolId, static_cast<int16_t>(msgType), handler);
 }
 
 CHIP_ERROR ExchangeManager::UnregisterUnsolicitedMessageHandlerForProtocol(Protocols::Id protocolId)
@@ -135,9 +136,9 @@ CHIP_ERROR ExchangeManager::UnregisterUnsolicitedMessageHandlerForType(Protocols
     return UnregisterUMH(protocolId, static_cast<int16_t>(msgType));
 }
 
-CHIP_ERROR ExchangeManager::RegisterUMH(Protocols::Id protocolId, int16_t msgType, ExchangeDelegate * delegate)
+CHIP_ERROR ExchangeManager::RegisterUMH(Protocols::Id protocolId, int16_t msgType, UnsolicitedMessageHandler * handler)
 {
-    UnsolicitedMessageHandler * selected = nullptr;
+    UnsolicitedMessageHandlerSlot * selected = nullptr;
 
     for (auto & umh : UMHandlerPool)
     {
@@ -148,15 +149,15 @@ CHIP_ERROR ExchangeManager::RegisterUMH(Protocols::Id protocolId, int16_t msgTyp
         }
         else if (umh.Matches(protocolId, msgType))
         {
-            umh.Delegate = delegate;
+            umh.Handler = handler;
             return CHIP_NO_ERROR;
         }
     }
 
     if (selected == nullptr)
         return CHIP_ERROR_TOO_MANY_UNSOLICITED_MESSAGE_HANDLERS;
 
-    selected->Delegate    = delegate;
+    selected->Handler     = handler;
     selected->ProtocolId  = protocolId;
     selected->MessageType = msgType;
 
@@ -184,7 +185,7 @@ void ExchangeManager::OnMessageReceived(const PacketHeader & packetHeader, const
                                         const SessionHandle & session, const Transport::PeerAddress & source,
                                         DuplicateMessage isDuplicate, System::PacketBufferHandle && msgBuf)
 {
-    UnsolicitedMessageHandler * matchingUMH = nullptr;
+    UnsolicitedMessageHandlerSlot * matchingUMH = nullptr;
 
     ChipLogProgress(ExchangeManager,
                     "Received message of type " ChipLogFormatMessageType " with protocolId " ChipLogFormatProtocolId
@@ -273,7 +274,20 @@ void ExchangeManager::OnMessageReceived(const PacketHeader & packetHeader, const
     // handle the message.
     if (matchingUMH != nullptr || payloadHeader.NeedsAck())
     {
-        ExchangeDelegate * delegate = matchingUMH ? matchingUMH->Delegate : nullptr;
+        ExchangeDelegate * delegate = nullptr;
+
+        // Fetch delegate from the handler
+        if (matchingUMH != nullptr)
+        {
+            CHIP_ERROR err = matchingUMH->Handler->OnUnsolicitedMessageReceived(payloadHeader, delegate);
+            if (err != CHIP_NO_ERROR)
+            {
+                // Using same error message for all errors to reduce code size.
+                ChipLogError(ExchangeManager, "OnMessageReceived failed, err = %s", ErrorStr(err));
+                return;
+            }
+        }
+
         // If rcvd msg is from initiator then this exchange is created as not Initiator.
         // If rcvd msg is not from initiator then this exchange is created as Initiator.
         // Note that if matchingUMH is not null then rcvd msg if from initiator.
@@ -283,6 +297,11 @@ void ExchangeManager::OnMessageReceived(const PacketHeader & packetHeader, const
 
         if (ec == nullptr)
         {
+            if (matchingUMH != nullptr && delegate != nullptr)
+            {
+                matchingUMH->Handler->OnExchangeCreationFailed(delegate);
+            }
+
             // Using same error message for all errors to reduce code size.
             ChipLogError(ExchangeManager, "OnMessageReceived failed, err = %s", ErrorStr(CHIP_ERROR_NO_MEMORY));
             return;

diff --git a/src/messaging/ExchangeMgr.h b/src/messaging/ExchangeMgr.h
@@ -109,13 +109,13 @@ class DLL_EXPORT ExchangeManager : public SessionMessageDelegate
      *
      *  @param[in]    protocolId      The protocol identifier of the received message.
      *
-     *  @param[in]    delegate        A pointer to ExchangeDelegate.
+     *  @param[in]    handler         A pointer to UnsolicitedMessageHandler.
      *
      *  @retval #CHIP_ERROR_TOO_MANY_UNSOLICITED_MESSAGE_HANDLERS If the unsolicited message handler pool
      *                                                             is full and a new one cannot be allocated.
      *  @retval #CHIP_NO_ERROR On success.
      */
-    CHIP_ERROR RegisterUnsolicitedMessageHandlerForProtocol(Protocols::Id protocolId, ExchangeDelegate * delegate);
+    CHIP_ERROR RegisterUnsolicitedMessageHandlerForProtocol(Protocols::Id protocolId, UnsolicitedMessageHandler * handler);
 
     /**
      *  Register an unsolicited message handler for a given protocol identifier and message type.
@@ -124,22 +124,23 @@ class DLL_EXPORT ExchangeManager : public SessionMessageDelegate
      *
      *  @param[in]    msgType         The message type of the corresponding protocol.
      *
-     *  @param[in]    delegate        A pointer to ExchangeDelegate.
+     *  @param[in]    handler         A pointer to UnsolicitedMessageHandler.
      *
      *  @retval #CHIP_ERROR_TOO_MANY_UNSOLICITED_MESSAGE_HANDLERS If the unsolicited message handler pool
      *                                                             is full and a new one cannot be allocated.
      *  @retval #CHIP_NO_ERROR On success.
      */
-    CHIP_ERROR RegisterUnsolicitedMessageHandlerForType(Protocols::Id protocolId, uint8_t msgType, ExchangeDelegate * delegate);
+    CHIP_ERROR RegisterUnsolicitedMessageHandlerForType(Protocols::Id protocolId, uint8_t msgType,
+                                                        UnsolicitedMessageHandler * handler);
 
     /**
      * A strongly-message-typed version of RegisterUnsolicitedMessageHandlerForType.
      */
     template <typename MessageType, typename = std::enable_if_t<std::is_enum<MessageType>::value>>
-    CHIP_ERROR RegisterUnsolicitedMessageHandlerForType(MessageType msgType, ExchangeDelegate * delegate)
+    CHIP_ERROR RegisterUnsolicitedMessageHandlerForType(MessageType msgType, UnsolicitedMessageHandler * handler)
     {
         return RegisterUnsolicitedMessageHandlerForType(Protocols::MessageTypeTraits<MessageType>::ProtocolId(),
-                                                        to_underlying(msgType), delegate);
+                                                        to_underlying(msgType), handler);
     }
 
     /**
@@ -200,25 +201,26 @@ class DLL_EXPORT ExchangeManager : public SessionMessageDelegate
         kState_Initialized    = 1  // Used to indicate that the ExchangeManager is initialized.
     };
 
-    struct UnsolicitedMessageHandler
+    struct UnsolicitedMessageHandlerSlot
     {
-        UnsolicitedMessageHandler() : ProtocolId(Protocols::NotSpecified) {}
+        UnsolicitedMessageHandlerSlot() : ProtocolId(Protocols::NotSpecified) {}
 
-        constexpr void Reset() { Delegate = nullptr; }
-        constexpr bool IsInUse() const { return Delegate != nullptr; }
+        constexpr void Reset() { Handler = nullptr; }
+        constexpr bool IsInUse() const { return Handler != nullptr; }
         // Matches() only returns a sensible value if IsInUse() is true.
         constexpr bool Matches(Protocols::Id aProtocolId, int16_t aMessageType) const
         {
             return ProtocolId == aProtocolId && MessageType == aMessageType;
         }
 
-        ExchangeDelegate * Delegate;
         Protocols::Id ProtocolId;
         // Message types are normally 8-bit unsigned ints, but we use
         // kAnyMessageType, which is negative, to represent a wildcard handler,
         // so need a type that can store both that and all valid message type
         // values.
         int16_t MessageType;
+
+        UnsolicitedMessageHandler * Handler;
     };
 
     uint16_t mNextExchangeId;
@@ -232,9 +234,9 @@ class DLL_EXPORT ExchangeManager : public SessionMessageDelegate
 
     BitMapObjectPool<ExchangeContext, CHIP_CONFIG_MAX_EXCHANGE_CONTEXTS> mContextPool;
 
-    UnsolicitedMessageHandler UMHandlerPool[CHIP_CONFIG_MAX_UNSOLICITED_MESSAGE_HANDLERS];
+    UnsolicitedMessageHandlerSlot UMHandlerPool[CHIP_CONFIG_MAX_UNSOLICITED_MESSAGE_HANDLERS];
 
-    CHIP_ERROR RegisterUMH(Protocols::Id protocolId, int16_t msgType, ExchangeDelegate * delegate);
+    CHIP_ERROR RegisterUMH(Protocols::Id protocolId, int16_t msgType, UnsolicitedMessageHandler * handler);
     CHIP_ERROR UnregisterUMH(Protocols::Id protocolId, int16_t msgType);
 
     void OnMessageReceived(const PacketHeader & packetHeader, const PayloadHeader & payloadHeader, const SessionHandle & session,

diff --git a/src/messaging/tests/TestExchangeMgr.cpp b/src/messaging/tests/TestExchangeMgr.cpp
@@ -59,9 +59,15 @@ enum : uint8_t
 
 TestContext sContext;
 
-class MockAppDelegate : public ExchangeDelegate
+class MockAppDelegate : public UnsolicitedMessageHandler, public ExchangeDelegate
 {
 public:
+    CHIP_ERROR OnUnsolicitedMessageReceived(const PayloadHeader & payloadHeader, ExchangeDelegate *& newDelegate) override
+    {
+        newDelegate = this;
+        return CHIP_NO_ERROR;
+    }
+
     CHIP_ERROR OnMessageReceived(ExchangeContext * ec, const PayloadHeader & payloadHeader,
                                  System::PacketBufferHandle && buffer) override
     {

diff --git a/src/messaging/tests/TestReliableMessageProtocol.cpp b/src/messaging/tests/TestReliableMessageProtocol.cpp
@@ -63,9 +63,16 @@ const char PAYLOAD[] = "Hello!";
 
 auto & gLoopback = sContext.GetLoopback();
 
-class MockAppDelegate : public ExchangeDelegate
+class MockAppDelegate : public UnsolicitedMessageHandler, public ExchangeDelegate
 {
 public:
+    CHIP_ERROR OnUnsolicitedMessageReceived(const PayloadHeader & payloadHeader, ExchangeDelegate *& newDelegate) override
+    {
+        // Handle messages by myself
+        newDelegate = this;
+        return CHIP_NO_ERROR;
+    }
+
     CHIP_ERROR OnMessageReceived(ExchangeContext * ec, const PayloadHeader & payloadHeader,
                                  System::PacketBufferHandle && buffer) override
     {
@@ -139,9 +146,16 @@ class MockSessionEstablishmentExchangeDispatch : public Messaging::ApplicationEx
     bool mRequireEncryption = false;
 };
 
-class MockSessionEstablishmentDelegate : public ExchangeDelegate
+class MockSessionEstablishmentDelegate : public UnsolicitedMessageHandler, public ExchangeDelegate
 {
 public:
+    CHIP_ERROR OnUnsolicitedMessageReceived(const PayloadHeader & payloadHeader, ExchangeDelegate *& newDelegate) override
+    {
+        // Handle messages by myself
+        newDelegate = this;
+        return CHIP_NO_ERROR;
+    }
+
     CHIP_ERROR OnMessageReceived(ExchangeContext * ec, const PayloadHeader & payloadHeader,
                                  System::PacketBufferHandle && buffer) override
     {

diff --git a/src/protocols/bdx/TransferFacilitator.h b/src/protocols/bdx/TransferFacilitator.h
@@ -41,13 +41,22 @@ namespace bdx {
  * This class contains a repeating timer which regurlaly polls the TransferSession state machine.
  * A CHIP node may have many TransferFacilitator instances but only one TransferFacilitator should be used for each BDX transfer.
  */
-class TransferFacilitator : public Messaging::ExchangeDelegate
+class TransferFacilitator : public Messaging::ExchangeDelegate, public Messaging::UnsolicitedMessageHandler
 {
 public:
     TransferFacilitator() : mExchangeCtx(nullptr), mSystemLayer(nullptr), mPollFreq(kDefaultPollFreq) {}
     ~TransferFacilitator() override = default;
 
 private:
+    //// UnsolicitedMessageHandler Implementation ////
+    CHIP_ERROR OnUnsolicitedMessageReceived(const PayloadHeader & payloadHeader, ExchangeDelegate *& newDelegate) override
+    {
+        // TODO: Implement a bdx manager, which dispatch bdx messages to bdx transections.
+        // directly.
+        newDelegate = this;
+        return CHIP_NO_ERROR;
+    }
+
     // Inherited from ExchangeContext
     CHIP_ERROR OnMessageReceived(chip::Messaging::ExchangeContext * ec, const chip::PayloadHeader & payloadHeader,
                                  chip::System::PacketBufferHandle && payload) override;

diff --git a/src/protocols/echo/Echo.h b/src/protocols/echo/Echo.h
@@ -110,7 +110,7 @@ class DLL_EXPORT EchoClient : public Messaging::ExchangeDelegate
     void OnResponseTimeout(Messaging::ExchangeContext * ec) override;
 };
 
-class DLL_EXPORT EchoServer : public Messaging::ExchangeDelegate
+class DLL_EXPORT EchoServer : public Messaging::UnsolicitedMessageHandler, public Messaging::ExchangeDelegate
 {
 public:
     /**
@@ -147,6 +147,7 @@ class DLL_EXPORT EchoServer : public Messaging::ExchangeDelegate
     Messaging::ExchangeManager * mExchangeMgr = nullptr;
     EchoFunct OnEchoRequestReceived           = nullptr;
 
+    CHIP_ERROR OnUnsolicitedMessageReceived(const PayloadHeader & payloadHeader, ExchangeDelegate *& newDelegate) override;
     CHIP_ERROR OnMessageReceived(Messaging::ExchangeContext * ec, const PayloadHeader & payloadHeader,
                                  System::PacketBufferHandle && payload) override;
     void OnResponseTimeout(Messaging::ExchangeContext * ec) override {}

diff --git a/src/protocols/echo/EchoServer.cpp b/src/protocols/echo/EchoServer.cpp
@@ -53,6 +53,13 @@ void EchoServer::Shutdown()
     }
 }
 
+CHIP_ERROR EchoServer::OnUnsolicitedMessageReceived(const PayloadHeader & payloadHeader, ExchangeDelegate *& newDelegate)
+{
+    // Handle messages by myself
+    newDelegate = this;
+    return CHIP_NO_ERROR;
+}
+
 CHIP_ERROR EchoServer::OnMessageReceived(Messaging::ExchangeContext * ec, const PayloadHeader & payloadHeader,
                                          System::PacketBufferHandle && payload)
 {

diff --git a/src/protocols/secure_channel/CASEServer.cpp b/src/protocols/secure_channel/CASEServer.cpp
@@ -81,6 +81,13 @@ CHIP_ERROR CASEServer::InitCASEHandshake(Messaging::ExchangeContext * ec)
     return CHIP_NO_ERROR;
 }
 
+CHIP_ERROR CASEServer::OnUnsolicitedMessageReceived(const PayloadHeader & payloadHeader, ExchangeDelegate *& newDelegate)
+{
+    // TODO: assign newDelegate to CASESession, let CASESession handle future messages.
+    newDelegate = this;
+    return CHIP_NO_ERROR;
+}
+
 CHIP_ERROR CASEServer::OnMessageReceived(Messaging::ExchangeContext * ec, const PayloadHeader & payloadHeader,
                                          System::PacketBufferHandle && payload)
 {