AsyncResolver: remove dependency on SignalThread.
This change removes dependency on SignalThread which is a
heavy user of re-entered mutexes, and is a step to being
able to delete SignalThread.
The new AsyncResolver is based on task queue instead.
Bug: webrtc:11567, webrtc:7723
Change-Id: Iab125ccbc0fb9d72af44341e58f89b3868c952cb
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/175910
Commit-Queue: Markus Handell <handellm@webrtc.org>
Reviewed-by: Tommi <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31361}
diff --git a/rtc_base/BUILD.gn b/rtc_base/BUILD.gn
index 2d90898..fdb622b 100644
--- a/rtc_base/BUILD.gn
+++ b/rtc_base/BUILD.gn
@@ -761,6 +761,7 @@
deps = [
":checks",
":deprecation",
+ ":rtc_task_queue",
":stringutils",
"../api:array_view",
"../api:function_view",
@@ -768,9 +769,11 @@
"../api/task_queue",
"../system_wrappers:field_trial",
"network:sent_packet",
+ "synchronization:sequence_checker",
"system:file_wrapper",
"system:inline",
"system:rtc_export",
+ "task_utils:pending_task_safety_flag",
"task_utils:to_queued_task",
"third_party/base64",
"third_party/sigslot",
diff --git a/rtc_base/net_helpers.cc b/rtc_base/net_helpers.cc
index 6ff3791..c6685e2 100644
--- a/rtc_base/net_helpers.cc
+++ b/rtc_base/net_helpers.cc
@@ -10,8 +10,6 @@
#include "rtc_base/net_helpers.h"
-#include <memory>
-
#if defined(WEBRTC_WIN)
#include <ws2spi.h>
#include <ws2tcpip.h>
@@ -26,8 +24,11 @@
#endif
#endif // defined(WEBRTC_POSIX) && !defined(__native_client__)
+#include "api/task_queue/task_queue_base.h"
#include "rtc_base/logging.h"
#include "rtc_base/signal_thread.h"
+#include "rtc_base/task_queue.h"
+#include "rtc_base/task_utils/to_queued_task.h"
#include "rtc_base/third_party/sigslot/sigslot.h" // for signal_with_thread...
namespace rtc {
@@ -83,18 +84,35 @@
#endif // !__native_client__
}
-// AsyncResolver
-AsyncResolver::AsyncResolver() : SignalThread(), error_(-1) {}
+AsyncResolver::AsyncResolver() : error_(-1) {}
-AsyncResolver::~AsyncResolver() = default;
+AsyncResolver::~AsyncResolver() {
+ RTC_DCHECK_RUN_ON(&sequence_checker_);
+}
void AsyncResolver::Start(const SocketAddress& addr) {
+ RTC_DCHECK_RUN_ON(&sequence_checker_);
+ RTC_DCHECK(!destroy_called_);
addr_ = addr;
- // SignalThred Start will kickoff the resolve process.
- SignalThread::Start();
+ webrtc::TaskQueueBase* current_task_queue = webrtc::TaskQueueBase::Current();
+ popup_thread_ = Thread::Create();
+ popup_thread_->Start();
+ popup_thread_->PostTask(webrtc::ToQueuedTask(
+ [this, flag = safety_.flag(), addr, current_task_queue] {
+ std::vector<IPAddress> addresses;
+ int error =
+ ResolveHostname(addr.hostname().c_str(), addr.family(), &addresses);
+ current_task_queue->PostTask(webrtc::ToQueuedTask(
+ std::move(flag), [this, error, addresses = std::move(addresses)] {
+ RTC_DCHECK_RUN_ON(&sequence_checker_);
+ ResolveDone(std::move(addresses), error);
+ }));
+ }));
}
bool AsyncResolver::GetResolvedAddress(int family, SocketAddress* addr) const {
+ RTC_DCHECK_RUN_ON(&sequence_checker_);
+ RTC_DCHECK(!destroy_called_);
if (error_ != 0 || addresses_.empty())
return false;
@@ -109,20 +127,40 @@
}
int AsyncResolver::GetError() const {
+ RTC_DCHECK_RUN_ON(&sequence_checker_);
+ RTC_DCHECK(!destroy_called_);
return error_;
}
void AsyncResolver::Destroy(bool wait) {
- SignalThread::Destroy(wait);
+ // Some callers have trouble guaranteeing that Destroy is called on the
+ // sequence guarded by |sequence_checker_|.
+ // RTC_DCHECK_RUN_ON(&sequence_checker_);
+ RTC_DCHECK(!destroy_called_);
+ destroy_called_ = true;
+ MaybeSelfDestruct();
}
-void AsyncResolver::DoWork() {
- error_ =
- ResolveHostname(addr_.hostname().c_str(), addr_.family(), &addresses_);
+const std::vector<IPAddress>& AsyncResolver::addresses() const {
+ RTC_DCHECK_RUN_ON(&sequence_checker_);
+ RTC_DCHECK(!destroy_called_);
+ return addresses_;
}
-void AsyncResolver::OnWorkDone() {
+void AsyncResolver::ResolveDone(std::vector<IPAddress> addresses, int error) {
+ addresses_ = addresses;
+ error_ = error;
+ recursion_check_ = true;
SignalDone(this);
+ MaybeSelfDestruct();
+}
+
+void AsyncResolver::MaybeSelfDestruct() {
+ if (!recursion_check_) {
+ delete this;
+ } else {
+ recursion_check_ = false;
+ }
}
const char* inet_ntop(int af, const void* src, char* dst, socklen_t size) {
diff --git a/rtc_base/net_helpers.h b/rtc_base/net_helpers.h
index 1e06940..c6aa4be 100644
--- a/rtc_base/net_helpers.h
+++ b/rtc_base/net_helpers.h
@@ -21,16 +21,23 @@
#include "rtc_base/async_resolver_interface.h"
#include "rtc_base/ip_address.h"
-#include "rtc_base/signal_thread.h"
#include "rtc_base/socket_address.h"
+#include "rtc_base/synchronization/sequence_checker.h"
#include "rtc_base/system/rtc_export.h"
+#include "rtc_base/task_utils/pending_task_safety_flag.h"
+#include "rtc_base/thread.h"
+#include "rtc_base/thread_annotations.h"
namespace rtc {
// AsyncResolver will perform async DNS resolution, signaling the result on
// the SignalDone from AsyncResolverInterface when the operation completes.
-class RTC_EXPORT AsyncResolver : public SignalThread,
- public AsyncResolverInterface {
+//
+// This class is thread-compatible, and all methods and destruction needs to
+// happen from the same rtc::Thread, except for Destroy which is allowed to
+// happen on another context provided it's not happening concurrently to another
+// public API call, and is the last access to the object.
+class RTC_EXPORT AsyncResolver : public AsyncResolverInterface {
public:
AsyncResolver();
~AsyncResolver() override;
@@ -40,17 +47,22 @@
int GetError() const override;
void Destroy(bool wait) override;
- const std::vector<IPAddress>& addresses() const { return addresses_; }
- void set_error(int error) { error_ = error; }
-
- protected:
- void DoWork() override;
- void OnWorkDone() override;
+ const std::vector<IPAddress>& addresses() const;
private:
- SocketAddress addr_;
- std::vector<IPAddress> addresses_;
- int error_;
+ void ResolveDone(std::vector<IPAddress> addresses, int error)
+ RTC_EXCLUSIVE_LOCKS_REQUIRED(sequence_checker_);
+ void MaybeSelfDestruct();
+
+ SocketAddress addr_ RTC_GUARDED_BY(sequence_checker_);
+ std::vector<IPAddress> addresses_ RTC_GUARDED_BY(sequence_checker_);
+ int error_ RTC_GUARDED_BY(sequence_checker_);
+ webrtc::ScopedTaskSafety safety_ RTC_GUARDED_BY(sequence_checker_);
+ std::unique_ptr<Thread> popup_thread_ RTC_GUARDED_BY(sequence_checker_);
+ bool recursion_check_ =
+ false; // Protects against SignalDone calling into Destroy.
+ bool destroy_called_ = false;
+ webrtc::SequenceChecker sequence_checker_;
};
// rtc namespaced wrappers for inet_ntop and inet_pton so we can avoid
