[ota] Fix exchange context leak in OTA requestor (project-chip#20304)

* [ota] Fix exchange context leak It turns out that Exchange Context allocated for BDX transfer is not released on completion or connection abort. It is not seen in a happy path that results in applying and rebooting into a new firmware, but may lead to the exchange leak when the transfer is interrupted. Furthermore, if an exchange is never released, a Sleepy End Device never returns to the idle mode, needlessly draining the battery. Signed-off-by: Damian Krolik <[email protected]> * Restyled by clang-format Co-authored-by: Restyled.io <[email protected]>
ajwak · Jul 6, 2022 · 757682e · 757682e
1 parent f65bcde
commit 757682e
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 10 deletions.
diff --git a/src/app/clusters/ota-requestor/DefaultOTARequestor.cpp b/src/app/clusters/ota-requestor/DefaultOTARequestor.cpp
@@ -638,12 +638,14 @@ void DefaultOTARequestor::OnDownloadStateChanged(OTADownloader::State state, OTA
     {
     case OTADownloader::State::kComplete:
         mOtaRequestorDriver->UpdateDownloaded();
+        mBdxMessenger.Reset();
         break;
     case OTADownloader::State::kIdle:
         if (reason != OTAChangeReasonEnum::kSuccess)
         {
             RecordErrorUpdateState(CHIP_ERROR_CONNECTION_ABORTED, reason);
         }
+        mBdxMessenger.Reset();
         break;
     default:
         break;
@@ -823,10 +825,10 @@ CHIP_ERROR DefaultOTARequestor::StartDownload(OperationalDeviceProxy & devicePro
     Optional<SessionHandle> session = deviceProxy.GetSecureSession();
     VerifyOrReturnError(session.HasValue(), CHIP_ERROR_INCORRECT_STATE);
 
-    mExchangeCtx = exchangeMgr->NewContext(session.Value(), &mBdxMessenger);
-    VerifyOrReturnError(mExchangeCtx != nullptr, CHIP_ERROR_NO_MEMORY);
+    chip::Messaging::ExchangeContext * exchangeCtx = exchangeMgr->NewContext(session.Value(), &mBdxMessenger);
+    VerifyOrReturnError(exchangeCtx != nullptr, CHIP_ERROR_NO_MEMORY);
 
-    mBdxMessenger.Init(mBdxDownloader, mExchangeCtx);
+    mBdxMessenger.Init(mBdxDownloader, exchangeCtx);
     mBdxDownloader->SetMessageDelegate(&mBdxMessenger);
     mBdxDownloader->SetStateDelegate(this);
 

diff --git a/src/app/clusters/ota-requestor/DefaultOTARequestor.h b/src/app/clusters/ota-requestor/DefaultOTARequestor.h
@@ -175,6 +175,13 @@ class DefaultOTARequestor : public OTARequestorInterface, public BDXDownloader::
             mDownloader  = downloader;
         }
 
+        void Reset()
+        {
+            VerifyOrReturn(mExchangeCtx != nullptr);
+            mExchangeCtx->Close();
+            mExchangeCtx = nullptr;
+        }
+
     private:
         chip::Messaging::ExchangeContext * mExchangeCtx;
         chip::BDXDownloader * mDownloader;
@@ -292,13 +299,12 @@ class DefaultOTARequestor : public OTARequestorInterface, public BDXDownloader::
      */
     static void OnCommissioningCompleteRequestor(const DeviceLayer::ChipDeviceEvent * event, intptr_t arg);
 
-    OTARequestorStorage * mStorage            = nullptr;
-    OTARequestorDriver * mOtaRequestorDriver  = nullptr;
-    CASESessionManager * mCASESessionManager  = nullptr;
-    OnConnectedAction mOnConnectedAction      = kQueryImage;
-    Messaging::ExchangeContext * mExchangeCtx = nullptr;
-    BDXDownloader * mBdxDownloader            = nullptr; // TODO: this should be OTADownloader
-    BDXMessenger mBdxMessenger;                          // TODO: ideally this is held by the application
+    OTARequestorStorage * mStorage           = nullptr;
+    OTARequestorDriver * mOtaRequestorDriver = nullptr;
+    CASESessionManager * mCASESessionManager = nullptr;
+    OnConnectedAction mOnConnectedAction     = kQueryImage;
+    BDXDownloader * mBdxDownloader           = nullptr; // TODO: this should be OTADownloader
+    BDXMessenger mBdxMessenger;                         // TODO: ideally this is held by the application
     uint8_t mUpdateTokenBuffer[kMaxUpdateTokenLen];
     ByteSpan mUpdateToken;
     uint32_t mCurrentVersion = 0;