Use AsyncDnsResolver in UDPPort class

Bug: webrtc:12598
Change-Id: I408d7daa0f3b5df6f45bcc97fa445bc8158b54ef
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/233561
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Markus Handell <handellm@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35147}

api/async_dns_resolver.h

@@ -51,6 +51,11 @@ class AsyncDnsResolverResult {
   virtual int GetError() const = 0;
 };
 
+// The API for a single name query.
+// The constructor, destructor and all functions must be called from
+// the same sequence, and the callback will also be called on that sequence.
+// The class guarantees that the callback will not be called if the
+// resolver's destructor has been called.
 class RTC_EXPORT AsyncDnsResolverInterface {
  public:
   virtual ~AsyncDnsResolverInterface() = default;
@@ -70,7 +75,7 @@ class AsyncDnsResolverFactoryInterface {
 
   // Creates an AsyncDnsResolver and starts resolving the name. The callback
   // will be called when resolution is finished.
-  // The callback will be called on the thread that the caller runs on.
+  // The callback will be called on the sequence that the caller runs on.
   virtual std::unique_ptr<webrtc::AsyncDnsResolverInterface> CreateAndResolve(
       const rtc::SocketAddress& addr,
       std::function<void()> callback) = 0;

p2p/base/stun_port.cc

@@ -120,29 +120,23 @@ UDPPort::AddressResolver::AddressResolver(
     std::function<void(const rtc::SocketAddress&, int)> done_callback)
     : socket_factory_(factory), done_(std::move(done_callback)) {}
 
-UDPPort::AddressResolver::~AddressResolver() {
-  for (ResolverMap::iterator it = resolvers_.begin(); it != resolvers_.end();
-       ++it) {
-    // TODO(guoweis): Change to asynchronous DNS resolution to prevent the hang
-    // when passing true to the Destroy() which is a safer way to avoid the code
-    // unloaded before the thread exits. Please see webrtc bug 5139.
-    it->second->Destroy(false);
-  }
-}
-
 void UDPPort::AddressResolver::Resolve(const rtc::SocketAddress& address) {
   if (resolvers_.find(address) != resolvers_.end())
     return;
 
-  rtc::AsyncResolverInterface* resolver =
-      socket_factory_->CreateAsyncResolver();
-  resolvers_.insert(std::pair<rtc::SocketAddress, rtc::AsyncResolverInterface*>(
-      address, resolver));
+  auto resolver = socket_factory_->CreateAsyncDnsResolver();
+  auto resolver_ptr = resolver.get();
+  std::pair<rtc::SocketAddress,
+            std::unique_ptr<webrtc::AsyncDnsResolverInterface>>
+      pair = std::make_pair(address, std::move(resolver));
 
-  resolver->SignalDone.connect(this,
-                               &UDPPort::AddressResolver::OnResolveResult);
-
-  resolver->Start(address);
+  resolvers_.insert(std::move(pair));
+  resolver_ptr->Start(address, [this, address] {
+    ResolverMap::const_iterator it = resolvers_.find(address);
+    if (it != resolvers_.end()) {
+      done_(it->first, it->second->result().GetError());
+    }
+  });
 }
 
 bool UDPPort::AddressResolver::GetResolvedAddress(
@@ -153,18 +147,7 @@ bool UDPPort::AddressResolver::GetResolvedAddress(
   if (it == resolvers_.end())
     return false;
 
-  return it->second->GetResolvedAddress(family, output);
-}
-
-void UDPPort::AddressResolver::OnResolveResult(
-    rtc::AsyncResolverInterface* resolver) {
-  for (ResolverMap::iterator it = resolvers_.begin(); it != resolvers_.end();
-       ++it) {
-    if (it->second == resolver) {
-      done_(it->first, resolver->GetError());
-      return;
-    }
-  }
+  return it->second->result().GetResolvedAddress(family, output);
 }
 
 UDPPort::UDPPort(rtc::Thread* thread,

p2p/base/stun_port.h

@@ -20,6 +20,7 @@
 #include "p2p/base/port.h"
 #include "p2p/base/stun_request.h"
 #include "rtc_base/async_packet_socket.h"
+#include "rtc_base/task_utils/pending_task_safety_flag.h"
 
 namespace cricket {
 
@@ -178,14 +179,13 @@ class UDPPort : public Port {
 
  private:
   // A helper class which can be called repeatedly to resolve multiple
-  // addresses, as opposed to rtc::AsyncResolverInterface, which can only
+  // addresses, as opposed to rtc::AsyncDnsResolverInterface, which can only
   // resolve one address per instance.
-  class AddressResolver : public sigslot::has_slots<> {
+  class AddressResolver {
    public:
     explicit AddressResolver(
         rtc::PacketSocketFactory* factory,
         std::function<void(const rtc::SocketAddress&, int)> done_callback);
-    ~AddressResolver() override;
 
     void Resolve(const rtc::SocketAddress& address);
     bool GetResolvedAddress(const rtc::SocketAddress& input,
@@ -193,17 +193,18 @@ class UDPPort : public Port {
                             rtc::SocketAddress* output) const;
 
    private:
-    typedef std::map<rtc::SocketAddress, rtc::AsyncResolverInterface*>
+    typedef std::map<rtc::SocketAddress,
+                     std::unique_ptr<webrtc::AsyncDnsResolverInterface>>
         ResolverMap;
 
-    void OnResolveResult(rtc::AsyncResolverInterface* resolver);
-
     rtc::PacketSocketFactory* socket_factory_;
-    ResolverMap resolvers_;
     // The function is called when resolving the specified address is finished.
     // The first argument is the input address, the second argument is the error
     // or 0 if it succeeded.
     std::function<void(const rtc::SocketAddress&, int)> done_;
+    // Resolver may fire callbacks that refer to done_, so ensure
+    // that all resolvers are destroyed first.
+    ResolverMap resolvers_;
   };
 
   // DNS resolution of the STUN server.