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}
diff --git a/api/async_dns_resolver.h b/api/async_dns_resolver.h
index cbe921b..138503b 100644
--- a/api/async_dns_resolver.h
+++ b/api/async_dns_resolver.h
@@ -51,6 +51,11 @@
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 @@
// 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;
diff --git a/p2p/base/stun_port.cc b/p2p/base/stun_port.cc
index 61726dc..7ae0557 100644
--- a/p2p/base/stun_port.cc
+++ b/p2p/base/stun_port.cc
@@ -120,29 +120,23 @@
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 @@
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,
diff --git a/p2p/base/stun_port.h b/p2p/base/stun_port.h
index 72f60e2..5a76767 100644
--- a/p2p/base/stun_port.h
+++ b/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 @@
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 @@
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.
