Handle the output event of type TransferSession::OutputEventType::kIn… (#37324)

* Handle the output event of type TransferSession::OutputEventType::kInternalError in the ProcessOuputEvents processing loop

- The transfer session state machine generates an output event of type TransferSession::OutputEventType::kInternalError in
certain error scenarios which put the transfer session in a bad state and when that happens, we need to unwind the processing
loop for events and clean up.

* Update src/protocols/bdx/AsyncTransferFacilitator.cpp

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

* Address review comments

* Update src/protocols/bdx/AsyncTransferFacilitator.cpp

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

---------

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

Author: nivi-apple

src/protocols/bdx/AsyncTransferFacilitator.cpp

@@ -63,6 +63,17 @@ void AsyncTransferFacilitator::ProcessOutputEvents()
     mTransfer.GetNextAction(outEvent);
     while (outEvent.EventType != TransferSession::OutputEventType::kNone)
     {
+
+        // If the transfer session state machine generates an event of type TransferSession::OutputEventType::kInternalError,
+        // indicating that the session is in a bad state, it will keep doing that thereafter.
+        //
+        // So stop trying to process events, and go ahead and destroy ourselves to clean up the transfer.
+        if (outEvent.EventType == TransferSession::OutputEventType::kInternalError)
+        {
+            mDestroySelfAfterProcessingEvents = true;
+            break;
+        }
+
         if (outEvent.EventType == TransferSession::OutputEventType::kMsgToSend)
         {
             CHIP_ERROR err = SendMessage(outEvent.msgTypeData, outEvent.MsgData);
@@ -170,22 +181,25 @@ void AsyncResponder::NotifyEventHandled(const TransferSession::OutputEventType e
     ChipLogDetail(BDX, "NotifyEventHandled : Event %s Error %" CHIP_ERROR_FORMAT,
                   TransferSession::OutputEvent::TypeToString(eventType), status.Format());
 
-    // If this is the end of the transfer (whether a clean end, or some sort of error condition), ensure that
-    // we destroy ourselves after processing any output events that might have been generated by
-    // the transfer session to handle end-of-transfer (e.g. in some error conditions it might want to send
-    // out a status report).
+    // If this is the end of the transfer (whether a clean end, or some sort of error condition), ensure
+    // that we destroy ourselves after unwinding the processing loop in the ProcessOutputEvents API.
+    // We can ignore the status for these output events because none of them are supposed to result in
+    // us sending a StatusReport, and that's all we use the status for.
+    //
+    // In particular, for kTransferTimeout, kAckEOFReceived, and kStatusReceived per spec we
+    // are not supposed to reply with a StatusReport.  And for kInternalError the state machine
+    // is in an unrecoverable state of some sort, and we should stop trying to make use of it.
     if (eventType == TransferSession::OutputEventType::kAckEOFReceived ||
         eventType == TransferSession::OutputEventType::kInternalError ||
         eventType == TransferSession::OutputEventType::kTransferTimeout ||
         eventType == TransferSession::OutputEventType::kStatusReceived)
     {
         mDestroySelfAfterProcessingEvents = true;
     }
-
-    // If there was an error handling the output event, this should notify the transfer object to abort transfer so it can send a
-    // status report across the exchange when we call ProcessOutputEvents below.
-    if (status != CHIP_NO_ERROR)
+    else if (status != CHIP_NO_ERROR)
     {
+        // If there was an error handling the output event, this should notify the transfer object to abort transfer
+        // so it can send a status report across the exchange when we call ProcessOutputEvents below.
         mTransfer.AbortTransfer(GetBdxStatusCodeFromChipError(status));
     }
 

src/protocols/bdx/AsyncTransferFacilitator.h

@@ -72,7 +72,9 @@ class AsyncTransferFacilitator : public Messaging::ExchangeDelegate
      */
     virtual void DestroySelf() = 0;
 
-    // Calling ProcessOutputEvents can destroy this object before the call returns.
+    /**
+     * Calling ProcessOutputEvents can destroy this object before the call returns.
+     */
     void ProcessOutputEvents();
 
     // The transfer session corresponding to this AsyncTransferFacilitator object.