Cancel the SRP timer if we have resolved on the SRP domain but the kD… (

#32948) * 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>
project-chip · Apr 12, 2024 · 1457450 · 1457450
1 parent 42833c6
commit 1457450
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 13 deletions.
diff --git 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
@@ -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
@@ -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