From 14574500f523a7ff27bf2f71f44cdb468cbeca14 Mon Sep 17 00:00:00 2001
From: Nivi Sarkar <55898241+nivi-apple@users.noreply.github.com>
Date: Thu, 11 Apr 2024 13:51:48 -0700
Subject: [PATCH] =?UTF-8?q?Cancel=20the=20SRP=20timer=20if=20we=20have=20r?=
 =?UTF-8?q?esolved=20on=20the=20SRP=20domain=20but=20the=20kD=E2=80=A6=20(?=
 =?UTF-8?q?#32948)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* Cancel the SRP timer if we have resolved on the SRP domain but the kDNSServiceFlagsMoreComing is set indicating there are more results coming

* Update src/platform/Darwin/DnssdImpl.cpp

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

* Addressed review comments

---------

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
---
 src/platform/Darwin/DnssdContexts.cpp | 15 ++++++++-------
 src/platform/Darwin/DnssdImpl.cpp     |  7 +++++++
 src/platform/Darwin/DnssdImpl.h       | 12 ++++++------
 3 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/src/platform/Darwin/DnssdContexts.cpp b/src/platform/Darwin/DnssdContexts.cpp
index 46c7bb34fc4365..7dc5956845414f 100644
--- a/src/platform/Darwin/DnssdContexts.cpp
+++ b/src/platform/Darwin/DnssdContexts.cpp
@@ -492,10 +492,7 @@ ResolveContext::ResolveContext(DiscoverNodeDelegate * delegate, chip::Inet::IPAd
 
 ResolveContext::~ResolveContext()
 {
-    if (isSRPTimerRunning)
-    {
-        CancelSRPTimer();
-    }
+    CancelSRPTimerIfRunning();
 }
 
 void ResolveContext::DispatchFailure(const char * errorStr, CHIP_ERROR err)
@@ -647,10 +644,14 @@ void ResolveContext::SRPTimerExpiredCallback(chip::System::Layer * systemLayer,
     sdCtx->Finalize();
 }
 
-void ResolveContext::CancelSRPTimer()
+void ResolveContext::CancelSRPTimerIfRunning()
 {
-    DeviceLayer::SystemLayer().CancelTimer(SRPTimerExpiredCallback, static_cast<void *>(this));
-    ChipLogProgress(Discovery, "SRP resolve timer for %s cancelled; resolve timed out", instanceName.c_str());
+    if (isSRPTimerRunning)
+    {
+        DeviceLayer::SystemLayer().CancelTimer(SRPTimerExpiredCallback, static_cast<void *>(this));
+        ChipLogProgress(Discovery, "SRP resolve timer for %s cancelled; resolve timed out", instanceName.c_str());
+        isSRPTimerRunning = false;
+    }
 }
 
 CHIP_ERROR ResolveContext::OnNewAddress(const InterfaceKey & interfaceKey, const struct sockaddr * address)
diff --git a/src/platform/Darwin/DnssdImpl.cpp b/src/platform/Darwin/DnssdImpl.cpp
index a9dccf21ef47f5..4156c7a4bf0924 100644
--- a/src/platform/Darwin/DnssdImpl.cpp
+++ b/src/platform/Darwin/DnssdImpl.cpp
@@ -272,6 +272,13 @@ static void OnGetAddrInfo(DNSServiceRef sdRef, DNSServiceFlags flags, uint32_t i
 
     if (flags & kDNSServiceFlagsMoreComing)
     {
+        // If we now don't need to have a timer while we wait for SRP results, ensure that there is no such
+        // timer running.  Otherwise the timer could fire before we get the rest of the results that flags
+        // say are coming, and trigger a finalize before we have all the data that is already available.
+        if (!sdCtx->shouldStartSRPTimerForResolve)
+        {
+            sdCtx->CancelSRPTimerIfRunning();
+        }
         return;
     }
 
diff --git a/src/platform/Darwin/DnssdImpl.h b/src/platform/Darwin/DnssdImpl.h
index 3cf48db33545a5..42ae55fdd9d4c0 100644
--- a/src/platform/Darwin/DnssdImpl.h
+++ b/src/platform/Darwin/DnssdImpl.h
@@ -297,6 +297,12 @@ struct ResolveContext : public GenericContext
      */
     static void SRPTimerExpiredCallback(chip::System::Layer * systemLayer, void * callbackContext);
 
+    /**
+     * @brief Cancels the timer that was started to wait for the resolution on the kSRPDot domain to happen.
+     *
+     */
+    void CancelSRPTimerIfRunning();
+
 private:
     /**
      * Try reporting the results we got on the provided interface index.
@@ -306,12 +312,6 @@ struct ResolveContext : public GenericContext
     bool TryReportingResultsForInterfaceIndex(uint32_t interfaceIndex, const std::string & hostname, bool isSRPResult);
 
     bool TryReportingResultsForInterfaceIndex(uint32_t interfaceIndex);
-
-    /**
-     * @brief Cancels the timer that was started to wait for the resolution on the kSRPDot domain to happen.
-     *
-     */
-    void CancelSRPTimer();
 };
 
 } // namespace Dnssd