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>
diff --git a/src/protocols/bdx/AsyncTransferFacilitator.cpp b/src/protocols/bdx/AsyncTransferFacilitator.cpp
index 2e3a0f0..eb3d996 100644
--- a/src/protocols/bdx/AsyncTransferFacilitator.cpp
+++ b/src/protocols/bdx/AsyncTransferFacilitator.cpp
@@ -63,6 +63,17 @@
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 @@
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 @@
{
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 1ad95d3..b9d4b79 100644
--- a/src/protocols/bdx/AsyncTransferFacilitator.h
+++ b/src/protocols/bdx/AsyncTransferFacilitator.h
@@ -72,7 +72,9 @@
*/
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.
