see if tests pass

nivi-apple · Oct 1, 2023 · 6d35757 · 6d35757
1 parent 0dddcdb
commit 6d35757
Show file tree

Hide file tree

Showing 6 changed files with 96 additions and 35 deletions.
diff --git a/src/darwin/Framework/CHIP/MTROTAImageTransferHandler.mm b/src/darwin/Framework/CHIP/MTROTAImageTransferHandler.mm
@@ -153,8 +153,7 @@
     CHIP_ERROR error = CHIP_NO_ERROR;
     if (event.EventType == TransferSession::OutputEventType::kTransferTimeout) {
         error = CHIP_ERROR_TIMEOUT;
-    } else if (event.EventType != TransferSession::OutputEventType::kAckEOFReceived
-        || event.EventType != TransferSession::OutputEventType::kAckEOFReceived) {
+    } else if (event.EventType != TransferSession::OutputEventType::kAckEOFReceived) {
         error = CHIP_ERROR_INTERNAL;
     }
 

diff --git a/src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.h b/src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.h
@@ -30,6 +30,14 @@ class MTROTAProviderDelegateBridge : public chip::app::Clusters::OTAProviderDele
 
     CHIP_ERROR Init(chip::Messaging::ExchangeManager * exchangeMgr);
 
+    // Shutdown must be called after the event loop has been stopped, since it
+    // touches Matter objects.
+    void Shutdown();
+
+    // ControllerShuttingDown must be called on the Matter work queue, since it
+    // touches Matter objects.
+    void ControllerShuttingDown(MTRDeviceController * controller);
+
     void HandleQueryImage(
         chip::app::CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath,
         const chip::app::Clusters::OtaSoftwareUpdateProvider::Commands::QueryImage::DecodableType & commandData) override;

diff --git a/src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.mm b/src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.mm
@@ -50,18 +50,17 @@
 MTROTAProviderDelegateBridge::MTROTAProviderDelegateBridge()
 {
     Clusters::OTAProvider::SetDelegate(kOtaProviderEndpoint, this);
-
-    // The singleton unsolicited BDX message handler which will create
-    // the MTROTAImageTransferHandler object for handling each BDX session.
-    sOtaUnsolicitedBDXMsgHandler = new MTROTAUnsolicitedBDXMessageHandler();
 }
 
 MTROTAProviderDelegateBridge::~MTROTAProviderDelegateBridge()
 {
     Clusters::OTAProvider::SetDelegate(kOtaProviderEndpoint, nullptr);
 
-    delete sOtaUnsolicitedBDXMsgHandler;
-    sOtaUnsolicitedBDXMsgHandler = nullptr;
+    if (sOtaUnsolicitedBDXMsgHandler != nil)
+    {
+        delete sOtaUnsolicitedBDXMsgHandler;
+        sOtaUnsolicitedBDXMsgHandler = nil;
+    }
 }
 
 CHIP_ERROR MTROTAProviderDelegateBridge::Init(Messaging::ExchangeManager * exchangeMgr)
@@ -70,16 +69,40 @@
 
     VerifyOrReturnError(exchangeMgr != nullptr, CHIP_ERROR_INCORRECT_STATE);
 
-    // Initialize the singleton MTROTAUnsolicitedBDXMessageHandler
-    VerifyOrReturnValue(sOtaUnsolicitedBDXMsgHandler != nil, CHIP_ERROR_INCORRECT_STATE);
+    VerifyOrReturnValue(sOtaUnsolicitedBDXMsgHandler == nil, CHIP_ERROR_INCORRECT_STATE);
 
+    // The singleton unsolicited BDX message handler which will create
+    // the MTROTAImageTransferHandler object for handling each BDX session.
+    sOtaUnsolicitedBDXMsgHandler = new MTROTAUnsolicitedBDXMessageHandler();
+
+    // Initialize the singleton MTROTAUnsolicitedBDXMessageHandler
     CHIP_ERROR err = sOtaUnsolicitedBDXMsgHandler->Init(exchangeMgr);
     if (err != CHIP_NO_ERROR) {
         ChipLogError(Controller, "Failed to initialize the unsolicited BDX Message handler with err %s", err.AsString());
     }
     return err;
 }
 
+void MTROTAProviderDelegateBridge::Shutdown()
+{
+    assertChipStackLockedByCurrentThread();
+
+    if (sOtaUnsolicitedBDXMsgHandler != nullptr)
+    {
+        sOtaUnsolicitedBDXMsgHandler->Shutdown();
+    }
+}
+
+void MTROTAProviderDelegateBridge::ControllerShuttingDown(MTRDeviceController * controller)
+{
+    assertChipStackLockedByCurrentThread();
+
+    if (sOtaUnsolicitedBDXMsgHandler != nullptr)
+    {
+        sOtaUnsolicitedBDXMsgHandler->ControllerShuttingDown(controller);
+    }
+}
+
 namespace {
 // Return false if we could not get peer node info (a running controller for
 // the fabric and a node id).  In that case we will have already added an

diff --git a/src/darwin/Framework/CHIP/MTROTAUnsolicitedBDXMessageHandler.h b/src/darwin/Framework/CHIP/MTROTAUnsolicitedBDXMessageHandler.h
@@ -36,6 +36,14 @@ class MTROTAUnsolicitedBDXMessageHandler : public chip::Messaging::UnsolicitedMe
 
     CHIP_ERROR Init(chip::Messaging::ExchangeManager * exchangeManager);
 
+    // Shutdown must be called after the event loop has been stopped, since it
+    // touches Matter objects.
+    void Shutdown();
+
+    // ControllerShuttingDown must be called on the Matter work queue, since it
+    // touches Matter objects.
+    void ControllerShuttingDown(MTRDeviceController * controller);
+
     // Returns the number of delegates that are currently handling BDX transfers
     static uint8_t GetNumberOfDelegates();
 

diff --git a/src/darwin/Framework/CHIP/MTROTAUnsolicitedBDXMessageHandler.mm b/src/darwin/Framework/CHIP/MTROTAUnsolicitedBDXMessageHandler.mm
@@ -27,11 +27,8 @@
 MTROTAUnsolicitedBDXMessageHandler::~MTROTAUnsolicitedBDXMessageHandler()
 {
     assertChipStackLockedByCurrentThread();
+    mExchangeMgr = nullptr;
 
-    if (mExchangeMgr) {
-        mExchangeMgr->UnregisterUnsolicitedMessageHandlerForProtocol(Protocols::BDX::Id);
-        mExchangeMgr = nullptr;
-    }
     MTROTAUnsolicitedBDXMessageHandler::mNumberOfDelegates = 0;
 }
 
@@ -47,6 +44,26 @@
     return mExchangeMgr->RegisterUnsolicitedMessageHandlerForProtocol(Protocols::BDX::Id, this);
 }
 
+void MTROTAUnsolicitedBDXMessageHandler::Shutdown()
+{
+    assertChipStackLockedByCurrentThread();
+
+    if (mExchangeMgr) {
+        mExchangeMgr->UnregisterUnsolicitedMessageHandlerForProtocol(Protocols::BDX::Id);
+    }
+    MTROTAUnsolicitedBDXMessageHandler::mNumberOfDelegates = 0;
+}
+
+void MTROTAUnsolicitedBDXMessageHandler::ControllerShuttingDown(MTRDeviceController * controller)
+{
+    assertChipStackLockedByCurrentThread();
+
+    if (mExchangeMgr) {
+        mExchangeMgr->UnregisterUnsolicitedMessageHandlerForProtocol(Protocols::BDX::Id);
+    }
+    MTROTAUnsolicitedBDXMessageHandler::mNumberOfDelegates = 0;
+}
+
 CHIP_ERROR MTROTAUnsolicitedBDXMessageHandler::OnUnsolicitedMessageReceived(
     const PayloadHeader & payloadHeader, ExchangeDelegate * _Nonnull & newDelegate)
 {
@@ -71,6 +88,12 @@
     return MTROTAUnsolicitedBDXMessageHandler::mNumberOfDelegates;
 }
 
-void MTROTAUnsolicitedBDXMessageHandler::IncrementNumberOfDelegates() { MTROTAUnsolicitedBDXMessageHandler::mNumberOfDelegates++; }
+void MTROTAUnsolicitedBDXMessageHandler::IncrementNumberOfDelegates()
+{
+    MTROTAUnsolicitedBDXMessageHandler::mNumberOfDelegates++;
+}
 
-void MTROTAUnsolicitedBDXMessageHandler::DecrementNumberOfDelegates() { MTROTAUnsolicitedBDXMessageHandler::mNumberOfDelegates--; }
+void MTROTAUnsolicitedBDXMessageHandler::DecrementNumberOfDelegates()
+{
+    MTROTAUnsolicitedBDXMessageHandler::mNumberOfDelegates--;
+}
diff --git a/src/protocols/bdx/AsyncTransferFacilitator.cpp b/src/protocols/bdx/AsyncTransferFacilitator.cpp
@@ -28,7 +28,7 @@ AsyncTransferFacilitator::~AsyncTransferFacilitator()
 
 void AsyncTransferFacilitator::Reset()
 {
-    mExchange.Release();
+    //mExchange.Release();
     mTransfer.Reset();
 }
 
@@ -94,12 +94,7 @@ CHIP_ERROR AsyncTransferFacilitator::OnMessageReceived(Messaging::ExchangeContex
         // If its a message of type kMsgToSend, send the message over the exchange.
         if (outEvent.EventType == TransferSession::OutputEventType::kMsgToSend)
         {
-            err = SendMessage(outEvent);
-            if (err != CHIP_NO_ERROR)
-            {
-                // TODO: Should we abort transfer here or let the other side timeout
-                return err;
-            }
+            SendMessage(outEvent);
         }
         else
         {
@@ -127,7 +122,7 @@ CHIP_ERROR AsyncTransferFacilitator::OnMessageReceived(Messaging::ExchangeContex
 void AsyncTransferFacilitator::OnExchangeClosing(Messaging::ExchangeContext * ec)
 {
     ChipLogDetail(BDX, "OnExchangeClosing, ec: " ChipLogFormatExchange, ChipLogValueExchange(ec));
-    CleanUp();
+    mExchange.Release();
 }
 
 void AsyncTransferFacilitator::OnResponseTimeout(Messaging::ExchangeContext * ec)
@@ -188,20 +183,25 @@ void AsyncResponder::NotifyEventHandled(CHIP_ERROR eventHandlingResult)
     {
         if (outEvent.EventType == TransferSession::OutputEventType::kMsgToSend)
         {
-            CHIP_ERROR err = AsyncTransferFacilitator::SendMessage(outEvent);
-            if (err != CHIP_NO_ERROR)
-            {
-                // TODO: Should we abort transfer here or let the other side timeout
-                return;
-            }
+            AsyncTransferFacilitator::SendMessage(outEvent);
         }
-        else if (outEvent.EventType == TransferSession::OutputEventType::kInternalError ||
-                 outEvent.EventType == TransferSession::OutputEventType::kTransferTimeout ||
-                 outEvent.EventType == TransferSession::OutputEventType::kStatusReceived)
+        else
         {
+            // Call the HandleTransferSessionOutput virtual method that's implemeted by the subclass to handle
+            // the output events.
             HandleTransferSessionOutput(outEvent);
-            CleanUp();
-            return;
+
+            // If it's a message indicating either the end of the transfer or a timeout reported by the transfer session
+            // or an error occured, we need to call the CleanUp virtual method implemented by the subclass to clean up and
+            // delete both the sublcass and base class objects.
+            if (outEvent.EventType == TransferSession::OutputEventType::kAckEOFReceived ||
+                outEvent.EventType == TransferSession::OutputEventType::kInternalError ||
+                outEvent.EventType == TransferSession::OutputEventType::kTransferTimeout ||
+                outEvent.EventType == TransferSession::OutputEventType::kStatusReceived)
+            {
+                CleanUp();
+                return;
+            }
         }
         mTransfer.GetNextAction(outEvent);
     };