fix: drop poll timer to avoid crash after TransferFacilitator destruction (#34538)
diff --git a/examples/ota-provider-app/esp32/main/BdxOtaSender.cpp b/examples/ota-provider-app/esp32/main/BdxOtaSender.cpp
index 2721878..5ff0e55 100644
--- a/examples/ota-provider-app/esp32/main/BdxOtaSender.cpp
+++ b/examples/ota-provider-app/esp32/main/BdxOtaSender.cpp
@@ -174,7 +174,6 @@
{
ChipLogError(BDX, "onTransferComplete Callback not set");
}
- mStopPolling = true; // Stop polling the TransferSession only after receiving BlockAckEOF
Reset();
break;
case TransferSession::OutputEventType::kStatusReceived:
@@ -228,7 +227,7 @@
{
mFabricIndex.ClearValue();
mNodeId.ClearValue();
- mTransfer.Reset();
+ ResetTransfer();
if (mExchangeCtx != nullptr)
{
mExchangeCtx->Close();
diff --git a/examples/ota-provider-app/ota-provider-common/BdxOtaSender.cpp b/examples/ota-provider-app/ota-provider-common/BdxOtaSender.cpp
index 5c3f221..871d8bf 100644
--- a/examples/ota-provider-app/ota-provider-common/BdxOtaSender.cpp
+++ b/examples/ota-provider-app/ota-provider-common/BdxOtaSender.cpp
@@ -180,7 +180,6 @@
break;
case TransferSession::OutputEventType::kAckEOFReceived:
ChipLogDetail(BDX, "Transfer completed, got AckEOF");
- mStopPolling = true; // Stop polling the TransferSession only after receiving BlockAckEOF
Reset();
break;
case TransferSession::OutputEventType::kStatusReceived:
@@ -212,7 +211,7 @@
{
mFabricIndex.ClearValue();
mNodeId.ClearValue();
- Responder::ResetTransfer();
+ ResetTransfer();
if (mExchangeCtx != nullptr)
{
mExchangeCtx->Close();
diff --git a/src/protocols/bdx/TransferFacilitator.cpp b/src/protocols/bdx/TransferFacilitator.cpp
index fbe5631..94962e2 100644
--- a/src/protocols/bdx/TransferFacilitator.cpp
+++ b/src/protocols/bdx/TransferFacilitator.cpp
@@ -32,6 +32,20 @@
constexpr System::Clock::Timeout TransferFacilitator::kDefaultPollFreq;
constexpr System::Clock::Timeout TransferFacilitator::kImmediatePollDelay;
+TransferFacilitator::~TransferFacilitator()
+{
+ ResetTransfer();
+}
+
+void TransferFacilitator::ResetTransfer()
+{
+ mTransfer.Reset();
+ ChipLogProgress(BDX, "Stop polling for messages");
+
+ VerifyOrReturn(mSystemLayer != nullptr);
+ mSystemLayer->CancelTimer(PollTimerHandler, this);
+}
+
CHIP_ERROR TransferFacilitator::OnMessageReceived(chip::Messaging::ExchangeContext * ec, const chip::PayloadHeader & payloadHeader,
chip::System::PacketBufferHandle && payload)
{
@@ -78,15 +92,7 @@
HandleTransferSessionOutput(outEvent);
VerifyOrReturn(mSystemLayer != nullptr, ChipLogError(BDX, "%s mSystemLayer is null", __FUNCTION__));
- if (!mStopPolling)
- {
- mSystemLayer->StartTimer(mPollFreq, PollTimerHandler, this);
- }
- else
- {
- mSystemLayer->CancelTimer(PollTimerHandler, this);
- mStopPolling = false;
- }
+ mSystemLayer->StartTimer(mPollFreq, PollTimerHandler, this);
}
void TransferFacilitator::ScheduleImmediatePoll()
@@ -106,18 +112,10 @@
ReturnErrorOnFailure(mTransfer.WaitForTransfer(role, xferControlOpts, maxBlockSize, timeout));
ChipLogProgress(BDX, "Start polling for messages");
- mStopPolling = false;
mSystemLayer->StartTimer(mPollFreq, PollTimerHandler, this);
return CHIP_NO_ERROR;
}
-void Responder::ResetTransfer()
-{
- mTransfer.Reset();
- ChipLogProgress(BDX, "Stop polling for messages");
- mStopPolling = true;
-}
-
CHIP_ERROR Initiator::InitiateTransfer(System::Layer * layer, TransferRole role, const TransferSession::TransferInitData & initData,
System::Clock::Timeout timeout, System::Clock::Timeout pollFreq)
{
@@ -132,12 +130,5 @@
return CHIP_NO_ERROR;
}
-void Initiator::ResetTransfer()
-{
- mTransfer.Reset();
- ChipLogProgress(BDX, "Stop polling for messages");
- mStopPolling = true;
-}
-
} // namespace bdx
} // namespace chip
diff --git a/src/protocols/bdx/TransferFacilitator.h b/src/protocols/bdx/TransferFacilitator.h
index 97e981a..280b6bf 100644
--- a/src/protocols/bdx/TransferFacilitator.h
+++ b/src/protocols/bdx/TransferFacilitator.h
@@ -45,7 +45,12 @@
{
public:
TransferFacilitator() : mExchangeCtx(nullptr), mSystemLayer(nullptr), mPollFreq(kDefaultPollFreq) {}
- ~TransferFacilitator() override = default;
+ ~TransferFacilitator() override;
+
+ /**
+ * Calls reset on the TransferSession object and stops the poll timer.
+ */
+ void ResetTransfer();
private:
//// UnsolicitedMessageHandler Implementation ////
@@ -96,7 +101,6 @@
System::Clock::Timeout mPollFreq;
static constexpr System::Clock::Timeout kDefaultPollFreq = System::Clock::Milliseconds32(500);
static constexpr System::Clock::Timeout kImmediatePollDelay = System::Clock::Milliseconds32(1);
- bool mStopPolling = false;
};
/**
@@ -121,11 +125,6 @@
CHIP_ERROR PrepareForTransfer(System::Layer * layer, TransferRole role, BitFlags<TransferControlFlags> xferControlOpts,
uint16_t maxBlockSize, System::Clock::Timeout timeout,
System::Clock::Timeout pollFreq = TransferFacilitator::kDefaultPollFreq);
-
- /**
- * Calls reset on the TransferSession object and stops the poll timer.
- */
- void ResetTransfer();
};
/**
@@ -150,10 +149,6 @@
CHIP_ERROR InitiateTransfer(System::Layer * layer, TransferRole role, const TransferSession::TransferInitData & initData,
System::Clock::Timeout timeout,
System::Clock::Timeout pollFreq = TransferFacilitator::kDefaultPollFreq);
- /**
- * Calls reset on the TransferSession object and stops the poll timer.
- */
- void ResetTransfer();
};
} // namespace bdx
