Add an API to stop a DNS-SD browse operation.

Most backends don't implement this yet. Darwin does, and no longer
stops Browse operations itself.

Fixes project-chip#19194

May provide a way toward fixing
project-chip#13275

src/controller/CHIPDeviceController.cpp

@@ -1431,6 +1431,11 @@ CHIP_ERROR DeviceCommissioner::DiscoverCommissionableNodes(Dnssd::DiscoveryFilte
     return mDNSResolver.DiscoverCommissionableNodes(filter);
 }
 
+CHIP_ERROR DeviceCommissioner::StopCommissionableDiscovery()
+{
+    return mDNSResolver.StopDiscovery();
+}
+
 const Dnssd::DiscoveredNodeData * DeviceCommissioner::GetDiscoveredDevice(int idx)
 {
     return GetDiscoveredNode(idx);

src/controller/CHIPDeviceController.h

@@ -597,6 +597,12 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
      */
     CHIP_ERROR DiscoverCommissionableNodes(Dnssd::DiscoveryFilter filter);
 
+    /**
+     * Stop commissionable discovery triggered by a previous
+     * DiscoverCommissionableNodes call.
+     */
+    CHIP_ERROR StopCommissionableDiscovery();
+
     /**
      * @brief
      *   Returns information about discovered devices.

src/controller/SetUpCodePairer.cpp

@@ -187,6 +187,8 @@ CHIP_ERROR SetUpCodePairer::StopConnectOverIP()
 
     mWaitingForDiscovery[kIPTransport] = false;
     currentFilter.type                 = Dnssd::DiscoveryFilterType::kNone;
+
+    mCommissioner->StopCommissionableDiscovery();
     return CHIP_NO_ERROR;
 }
 

src/controller/tests/TestCommissionableNodeController.cpp

@@ -41,6 +41,7 @@ class MockResolver : public Resolver
     {
         return CHIP_ERROR_NOT_IMPLEMENTED;
     }
+    CHIP_ERROR StopDiscovery() override { return CHIP_ERROR_NOT_IMPLEMENTED; }
 
     CHIP_ERROR InitStatus                  = CHIP_NO_ERROR;
     CHIP_ERROR ResolveNodeIdStatus         = CHIP_NO_ERROR;

src/lib/dnssd/Discovery_ImplPlatform.cpp

@@ -153,6 +153,12 @@ static void HandleNodeBrowse(void * context, DnssdService * services, size_t ser
 
     if (error != CHIP_NO_ERROR)
     {
+        // We don't have access to the ResolverProxy here to clear out its
+        // mDiscoveryContext.  The underlying implementation of
+        // ChipDnssdStopBrowse needs to handle a possibly-stale reference
+        // anyway, though, so it's OK.
+        //
+        // TODO: Have a way to clear that state here.
         proxy->Release();
         return;
     }
@@ -174,6 +180,12 @@ static void HandleNodeBrowse(void * context, DnssdService * services, size_t ser
 
     if (finalBrowse)
     {
+        // We don't have access to the ResolverProxy here to clear out its
+        // mDiscoveryContext.  The underlying implementation of
+        // ChipDnssdStopBrowse needs to handle a possibly-stale reference
+        // anyway, though, so it's OK.
+        //
+        // TODO: Have a way to clear that state here.
         proxy->Release();
     }
 }
@@ -616,6 +628,12 @@ CHIP_ERROR DiscoveryImplPlatform::DiscoverCommissioners(DiscoveryFilter filter)
     return mResolverProxy.DiscoverCommissioners(filter);
 }
 
+CHIP_ERROR DiscoveryImplPlatform::StopDiscovery()
+{
+    ReturnErrorOnFailure(InitImpl());
+    return mResolverProxy.StopDiscovery();
+}
+
 DiscoveryImplPlatform & DiscoveryImplPlatform::GetInstance()
 {
     return sManager;
@@ -656,6 +674,8 @@ ResolverProxy::~ResolverProxy()
 
 CHIP_ERROR ResolverProxy::DiscoverCommissionableNodes(DiscoveryFilter filter)
 {
+    StopDiscovery();
+
     VerifyOrReturnError(mDelegate != nullptr, CHIP_ERROR_INCORRECT_STATE);
     mDelegate->Retain();
 
@@ -674,12 +694,17 @@ CHIP_ERROR ResolverProxy::DiscoverCommissionableNodes(DiscoveryFilter filter)
     char serviceName[kMaxCommissionableServiceNameSize];
     ReturnErrorOnFailure(MakeServiceTypeName(serviceName, sizeof(serviceName), filter, DiscoveryType::kCommissionableNode));
 
-    return ChipDnssdBrowse(serviceName, DnssdServiceProtocol::kDnssdProtocolUdp, Inet::IPAddressType::kAny,
-                           Inet::InterfaceId::Null(), HandleNodeBrowse, mDelegate);
+    intptr_t browseIdentifier;
+    ReturnErrorOnFailure(ChipDnssdBrowse(serviceName, DnssdServiceProtocol::kDnssdProtocolUdp, Inet::IPAddressType::kAny,
+                                         Inet::InterfaceId::Null(), HandleNodeBrowse, mDelegate, &browseIdentifier));
+    mDiscoveryContext.Emplace(browseIdentifier);
+    return CHIP_NO_ERROR;
 }
 
 CHIP_ERROR ResolverProxy::DiscoverCommissioners(DiscoveryFilter filter)
 {
+    StopDiscovery();
+
     VerifyOrReturnError(mDelegate != nullptr, CHIP_ERROR_INCORRECT_STATE);
     mDelegate->Retain();
 
@@ -698,8 +723,24 @@ CHIP_ERROR ResolverProxy::DiscoverCommissioners(DiscoveryFilter filter)
     char serviceName[kMaxCommissionerServiceNameSize];
     ReturnErrorOnFailure(MakeServiceTypeName(serviceName, sizeof(serviceName), filter, DiscoveryType::kCommissionerNode));
 
-    return ChipDnssdBrowse(serviceName, DnssdServiceProtocol::kDnssdProtocolUdp, Inet::IPAddressType::kAny,
-                           Inet::InterfaceId::Null(), HandleNodeBrowse, mDelegate);
+    intptr_t browseIdentifier;
+    ReturnErrorOnFailure(ChipDnssdBrowse(serviceName, DnssdServiceProtocol::kDnssdProtocolUdp, Inet::IPAddressType::kAny,
+                                         Inet::InterfaceId::Null(), HandleNodeBrowse, mDelegate, &browseIdentifier));
+    mDiscoveryContext.Emplace(browseIdentifier);
+    return CHIP_NO_ERROR;
+}
+
+CHIP_ERROR ResolverProxy::StopDiscovery()
+{
+    if (!mDiscoveryContext.HasValue())
+    {
+        // No discovery going on.
+        return CHIP_NO_ERROR;
+    }
+
+    CHIP_ERROR err = ChipDnssdStopBrowse(mDiscoveryContext.Value());
+    mDiscoveryContext.ClearValue();
+    return err;
 }
 
 } // namespace Dnssd

src/lib/dnssd/Discovery_ImplPlatform.h

@@ -57,6 +57,7 @@ class DiscoveryImplPlatform : public ServiceAdvertiser, public Resolver
     CHIP_ERROR ResolveNodeId(const PeerId & peerId, Inet::IPAddressType type) override;
     CHIP_ERROR DiscoverCommissionableNodes(DiscoveryFilter filter = DiscoveryFilter()) override;
     CHIP_ERROR DiscoverCommissioners(DiscoveryFilter filter = DiscoveryFilter()) override;
+    CHIP_ERROR StopDiscovery() override;
 
     static DiscoveryImplPlatform & GetInstance();
 

src/lib/dnssd/Resolver.h

@@ -390,6 +390,14 @@ class Resolver
      */
     virtual CHIP_ERROR DiscoverCommissioners(DiscoveryFilter filter = DiscoveryFilter()) = 0;
 
+    /**
+     * Stop discovery (of commissionable or commissioner nodes).
+     *
+     * Some back ends may not support stopping discovery, so consumers should
+     * not assume they will stop getting callbacks after calling this.
+     */
+    virtual CHIP_ERROR StopDiscovery() = 0;
+
     /**
      * Provides the system-wide implementation of the service resolver
      */

src/lib/dnssd/ResolverProxy.h

@@ -152,11 +152,16 @@ class ResolverProxy : public Resolver
     CHIP_ERROR ResolveNodeId(const PeerId & peerId, Inet::IPAddressType type) override;
     CHIP_ERROR DiscoverCommissionableNodes(DiscoveryFilter filter = DiscoveryFilter()) override;
     CHIP_ERROR DiscoverCommissioners(DiscoveryFilter filter = DiscoveryFilter()) override;
+    CHIP_ERROR StopDiscovery() override;
 
 private:
     ResolverDelegateProxy * mDelegate                            = nullptr;
     OperationalResolveDelegate * mPreInitOperationalDelegate     = nullptr;
     CommissioningResolveDelegate * mPreInitCommissioningDelegate = nullptr;
+
+    // While discovery (commissionable or commissioner) is ongoing,
+    // mDiscoveryContext may have a value to allow StopDiscovery to work.
+    Optional<intptr_t> mDiscoveryContext;
 };
 
 } // namespace Dnssd

src/lib/dnssd/Resolver_ImplMinimalMdns.cpp

@@ -281,6 +281,7 @@ class MinMdnsResolver : public Resolver, public MdnsPacketDelegate
     CHIP_ERROR ResolveNodeId(const PeerId & peerId, Inet::IPAddressType type) override;
     CHIP_ERROR DiscoverCommissionableNodes(DiscoveryFilter filter = DiscoveryFilter()) override;
     CHIP_ERROR DiscoverCommissioners(DiscoveryFilter filter = DiscoveryFilter()) override;
+    CHIP_ERROR StopDiscovery() override { return CHIP_ERROR_NOT_IMPLEMENTED; }
 
 private:
     OperationalResolveDelegate * mOperationalDelegate     = nullptr;
@@ -712,5 +713,10 @@ CHIP_ERROR ResolverProxy::DiscoverCommissioners(DiscoveryFilter filter)
     return chip::Dnssd::Resolver::Instance().DiscoverCommissioners(filter);
 }
 
+CHIP_ERROR ResolverProxy::StopDiscovery()
+{
+    return CHIP_ERROR_NOT_IMPLEMENTED;
+}
+
 } // namespace Dnssd
 } // namespace chip

src/lib/dnssd/Resolver_ImplNone.cpp

@@ -42,6 +42,7 @@ class NoneResolver : public Resolver
         return CHIP_ERROR_NOT_IMPLEMENTED;
     }
     CHIP_ERROR DiscoverCommissioners(DiscoveryFilter filter = DiscoveryFilter()) override { return CHIP_ERROR_NOT_IMPLEMENTED; }
+    CHIP_ERROR StopDiscovery() override { return CHIP_ERROR_NOT_IMPLEMENTED; }
 };
 
 NoneResolver gResolver;
@@ -73,5 +74,10 @@ CHIP_ERROR ResolverProxy::DiscoverCommissioners(DiscoveryFilter filter)
     return CHIP_ERROR_NOT_IMPLEMENTED;
 }
 
+CHIP_ERROR ResolverProxy::StopDiscovery()
+{
+    return CHIP_ERROR_NOT_IMPLEMENTED;
+}
+
 } // namespace Dnssd
 } // namespace chip

src/lib/dnssd/platform/Dnssd.h

@@ -203,14 +203,28 @@ CHIP_ERROR ChipDnssdFinalizeServiceUpdate();
  * @param[in] interface   The interface to send queries.
  * @param[in] callback    The callback for found services.
  * @param[in] context     The user context.
+ * @param[out] browseIdentifier an identifier for this browse operation. This
+ *                              can be used to call ChipDnssdStopBrowse.  Only
+ *                              set on success.  This value remains valid until
+ *                              the browse callback is called with an error or
+ *                              is called with finalBrowse set to true.
  *
  * @retval CHIP_NO_ERROR                The browse succeeds.
  * @retval CHIP_ERROR_INVALID_ARGUMENT  The type or callback is nullptr.
  * @retval Error code                   The browse fails.
  *
  */
 CHIP_ERROR ChipDnssdBrowse(const char * type, DnssdServiceProtocol protocol, chip::Inet::IPAddressType addressType,
-                           chip::Inet::InterfaceId interface, DnssdBrowseCallback callback, void * context);
+                           chip::Inet::InterfaceId interface, DnssdBrowseCallback callback, void * context,
+                           intptr_t * browseIdentifier);
+
+/**
+ * Stop an ongoing browse, if supported by this backend.
+ *
+ * @param browseIdentifier an identifier for a browse operation that came from
+ *                         ChipDnssdBrowse.
+ */
+CHIP_ERROR ChipDnssdStopBrowse(intptr_t browseIdentifier);
 
 /**
  * This function resolves the services published by mDNS

src/platform/Ameba/DnssdImpl.cpp

@@ -97,7 +97,13 @@ CHIP_ERROR ChipDnssdStopPublish()
 }
 
 CHIP_ERROR ChipDnssdBrowse(const char * type, DnssdServiceProtocol protocol, chip::Inet::IPAddressType addressType,
-                           chip::Inet::InterfaceId interface, DnssdBrowseCallback callback, void * context);
+                           chip::Inet::InterfaceId interface, DnssdBrowseCallback callback, void * context,
+                           intptr_t * browseIdentifier);
+{
+    return CHIP_ERROR_NOT_IMPLEMENTED;
+}
+
+CHIP_ERROR ChipDnssdStopBrowse(intptr_t browseIdentifier)
 {
     return CHIP_ERROR_NOT_IMPLEMENTED;
 }

src/platform/Darwin/DnssdContexts.cpp

@@ -299,6 +299,12 @@ void BrowseContext::DispatchSuccess()
     MdnsContexts::GetInstance().Remove(this);
 }
 
+void BrowseContext::DispatchPartialSuccess()
+{
+    callback(context, services.data(), services.size(), false, CHIP_NO_ERROR);
+    services.clear();
+}
+
 ResolveContext::ResolveContext(void * cbContext, DnssdResolveCallback cb, chip::Inet::IPAddressType cbAddressType)
 {
     type     = ContextType::Resolve;

src/platform/Darwin/DnssdImpl.cpp

@@ -256,12 +256,12 @@ static void OnBrowse(DNSServiceRef sdRef, DNSServiceFlags flags, uint32_t interf
 
     if (!(flags & kDNSServiceFlagsMoreComing))
     {
-        sdCtx->Finalize();
+        sdCtx->DispatchPartialSuccess();
     }
 }
 
 CHIP_ERROR Browse(void * context, DnssdBrowseCallback callback, uint32_t interfaceId, const char * type,
-                  DnssdServiceProtocol protocol)
+                  DnssdServiceProtocol protocol, intptr_t * browseIdentifier)
 {
     auto sdCtx = chip::Platform::New<BrowseContext>(context, callback, protocol);
     VerifyOrReturnError(nullptr != sdCtx, CHIP_ERROR_NO_MEMORY);
@@ -271,7 +271,9 @@ CHIP_ERROR Browse(void * context, DnssdBrowseCallback callback, uint32_t interfa
     auto err = DNSServiceBrowse(&sdRef, kBrowseFlags, interfaceId, type, kLocalDot, OnBrowse, sdCtx);
     VerifyOrReturnError(kDNSServiceErr_NoError == err, sdCtx->Finalize(err));
 
-    return MdnsContexts::GetInstance().Add(sdCtx, sdRef);
+    ReturnErrorOnFailure(MdnsContexts::GetInstance().Add(sdCtx, sdRef));
+    *browseIdentifier = reinterpret_cast<intptr_t>(sdCtx);
+    return CHIP_NO_ERROR;
 }
 
 static void OnGetAddrInfo(DNSServiceRef sdRef, DNSServiceFlags flags, uint32_t interfaceId, DNSServiceErrorType err,
@@ -398,15 +400,39 @@ CHIP_ERROR ChipDnssdFinalizeServiceUpdate()
 }
 
 CHIP_ERROR ChipDnssdBrowse(const char * type, DnssdServiceProtocol protocol, chip::Inet::IPAddressType addressType,
-                           chip::Inet::InterfaceId interface, DnssdBrowseCallback callback, void * context)
+                           chip::Inet::InterfaceId interface, DnssdBrowseCallback callback, void * context,
+                           intptr_t * browseIdentifier)
 {
     VerifyOrReturnError(type != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
     VerifyOrReturnError(callback != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
     VerifyOrReturnError(IsSupportedProtocol(protocol), CHIP_ERROR_INVALID_ARGUMENT);
 
     auto regtype     = GetFullTypeWithSubTypes(type, protocol);
     auto interfaceId = GetInterfaceId(interface);
-    return Browse(context, callback, interfaceId, regtype.c_str(), protocol);
+    return Browse(context, callback, interfaceId, regtype.c_str(), protocol, browseIdentifier);
+}
+
+CHIP_ERROR ChipDnssdStopBrowse(intptr_t browseIdentifier)
+{
+    auto ctx = reinterpret_cast<GenericContext *>(browseIdentifier);
+    if (MdnsContexts::GetInstance().Has(ctx) != CHIP_NO_ERROR)
+    {
+        return CHIP_ERROR_NOT_FOUND;
+    }
+
+    // We know this is an actual context now, so can check the type.
+    if (ctx->type != ContextType::Browse)
+    {
+        // stale pointer that got reallocated.
+        return CHIP_ERROR_NOT_FOUND;
+    }
+
+    // Just treat this as a timeout error.  Don't bother delivering the partial
+    // results we have queued up in the BrowseContext, if any.  In practice
+    // there shouldn't be anything there long-term anyway.
+    auto browseCtx = static_cast<BrowseContext *>(ctx);
+    browseCtx->Finalize(kDNSServiceErr_Timeout);
+    return CHIP_NO_ERROR;
 }
 
 CHIP_ERROR ChipDnssdResolve(DnssdService * service, chip::Inet::InterfaceId interface, DnssdResolveCallback callback,

src/platform/Darwin/DnssdImpl.h

@@ -117,6 +117,9 @@ struct BrowseContext : public GenericContext
 
     void DispatchFailure(DNSServiceErrorType err) override;
     void DispatchSuccess() override;
+
+    // Dispatch what we have found so far, but don't stop browsing.
+    void DispatchPartialSuccess();
 };
 
 struct InterfaceInfo

src/platform/ESP32/DnssdImpl.cpp

@@ -463,7 +463,8 @@ void MdnsQueryNotifier(mdns_search_once_t * searchHandle)
 }
 
 CHIP_ERROR ChipDnssdBrowse(const char * type, DnssdServiceProtocol protocol, chip::Inet::IPAddressType addressType,
-                           chip::Inet::InterfaceId interface, DnssdBrowseCallback callback, void * context)
+                           chip::Inet::InterfaceId interface, DnssdBrowseCallback callback, void * context,
+                           intptr_t * browseIdentifier)
 {
     CHIP_ERROR error = CHIP_NO_ERROR;
     mdns_search_once_t * searchHandle =
@@ -481,9 +482,18 @@ CHIP_ERROR ChipDnssdBrowse(const char * type, DnssdServiceProtocol protocol, chi
     {
         chip::Platform::Delete(ctx);
     }
+    else
+    {
+        *browseIdentifier = reinterpret_cast<intptr_t>(nullptr);
+    }
     return error;
 }
 
+CHIP_ERROR ChipDnssdStopBrowse(intptr_t browseIdentifier)
+{
+    return CHIP_ERROR_NOT_IMPLEMENTED;
+}
+
 CHIP_ERROR ChipDnssdResolve(DnssdService * service, chip::Inet::InterfaceId interface, DnssdResolveCallback callback,
                             void * context)
 {

src/platform/Linux/DnssdImpl.cpp

@@ -850,11 +850,18 @@ CHIP_ERROR ChipDnssdFinalizeServiceUpdate()
 }
 
 CHIP_ERROR ChipDnssdBrowse(const char * type, DnssdServiceProtocol protocol, chip::Inet::IPAddressType addressType,
-                           chip::Inet::InterfaceId interface, DnssdBrowseCallback callback, void * context)
+                           chip::Inet::InterfaceId interface, DnssdBrowseCallback callback, void * context,
+                           intptr_t * browseIdentifier)
 {
+    *browseIdentifier = reinterpret_cast<intptr_t>(nullptr);
     return MdnsAvahi::GetInstance().Browse(type, protocol, addressType, interface, callback, context);
 }
 
+CHIP_ERROR ChipDnssdStopBrowse(intptr_t browseIdentifier)
+{
+    return CHIP_ERROR_NOT_IMPLEMENTED;
+}
+
 CHIP_ERROR ChipDnssdResolve(DnssdService * browseResult, chip::Inet::InterfaceId interface, DnssdResolveCallback callback,
                             void * context)