diff --git a/src/protocols/bdx/AsyncTransferFacilitator.cpp b/src/protocols/bdx/AsyncTransferFacilitator.cpp index 2e3a0f03794319..eb3d996b1e53b8 100644 --- a/src/protocols/bdx/AsyncTransferFacilitator.cpp +++ b/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,10 +181,14 @@ 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 || @@ -181,11 +196,10 @@ void AsyncResponder::NotifyEventHandled(const TransferSession::OutputEventType e { 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)); } diff --git a/src/protocols/bdx/AsyncTransferFacilitator.h b/src/protocols/bdx/AsyncTransferFacilitator.h index 1ad95d39e98d2d..b9d4b79bf55c5f 100644 --- a/src/protocols/bdx/AsyncTransferFacilitator.h +++ b/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.