Index: webrtc/base/nethelpers.cc |
diff --git a/webrtc/base/nethelpers.cc b/webrtc/base/nethelpers.cc |
index 6c11ef8c38fa04800df834bc8fa0f0f8939b3b89..2c920e73476b3e462ce6c020711effb7a0c9eb43 100644 |
--- a/webrtc/base/nethelpers.cc |
+++ b/webrtc/base/nethelpers.cc |
@@ -26,12 +26,14 @@ |
#endif // defined(WEBRTC_POSIX) && !defined(__native_client__) |
#include "webrtc/base/byteorder.h" |
+#include "webrtc/base/bind.h" |
#include "webrtc/base/checks.h" |
#include "webrtc/base/logging.h" |
-#include "webrtc/base/signalthread.h" |
+#include "webrtc/base/thread.h" |
namespace rtc { |
+namespace { |
int ResolveHostname(const std::string& hostname, int family, |
std::vector<IPAddress>* addresses) { |
#ifdef __native_client__ |
@@ -81,20 +83,27 @@ int ResolveHostname(const std::string& hostname, int family, |
return 0; |
#endif // !__native_client__ |
} |
+} // namespace |
// AsyncResolver |
AsyncResolver::AsyncResolver() |
- : SignalThread(false /* use_socket_server */), error_(-1) {} |
+ : main_(Thread::Current()), |
tommi
2017/06/08 08:02:06
one other idea for a variable name could be |const
|
+ worker_(ThreadEntry, this, "AsyncResolver"), |
tommi
2017/06/08 08:02:05
nit: worker_(&ThreadEntry, this, "AsyncResolver"),
nisse-webrtc
2017/06/08 09:52:33
Done.
|
+ error_(-1) {} |
tommi
2017/06/08 08:02:07
nit: you could initialize this value inline in the
nisse-webrtc
2017/06/08 09:52:33
Done.
|
-AsyncResolver::~AsyncResolver() = default; |
+AsyncResolver::~AsyncResolver() { |
+ RTC_DCHECK(main_->IsCurrent()); |
+ worker_.Stop(); |
+} |
void AsyncResolver::Start(const SocketAddress& addr) { |
+ RTC_DCHECK(main_->IsCurrent()); |
tommi
2017/06/08 08:02:06
also add:
RTC_DCHECK(!worker_.IsRunning());
|
addr_ = addr; |
- // SignalThred Start will kickoff the resolve process. |
- SignalThread::Start(); |
+ worker_.Start(); |
} |
bool AsyncResolver::GetResolvedAddress(int family, SocketAddress* addr) const { |
tommi
2017/06/08 08:02:06
on what thread does this method run?
nisse-webrtc
2017/06/08 09:52:33
Don't know. My understanding is that the applicati
|
+ rtc::CritScope cs_(&lock_); |
if (error_ != 0 || addresses_.empty()) |
return false; |
@@ -109,22 +118,40 @@ bool AsyncResolver::GetResolvedAddress(int family, SocketAddress* addr) const { |
} |
int AsyncResolver::GetError() const { |
+ rtc::CritScope cs_(&lock_); |
tommi
2017/06/08 08:02:08
this sort of pattern always feels strange to me.
nisse-webrtc
2017/06/08 09:52:33
Agreed, this use implies a pretty heavy memory bar
|
return error_; |
} |
void AsyncResolver::Destroy(bool wait) { |
- SignalThread::Destroy(wait); |
+ RTC_DCHECK(main_->IsCurrent()); |
+ // If we don't wait here, we will nevertheless wait in the destructor. |
+ if (wait) { |
+ worker_.Stop(); |
+ } |
} |
void AsyncResolver::DoWork() { |
tommi
2017/06/08 08:02:06
would like to have a thread check for this method
nisse-webrtc
2017/06/08 09:52:33
It's private and with only a single call site. I'm
|
- error_ = ResolveHostname(addr_.hostname().c_str(), addr_.family(), |
- &addresses_); |
+ { |
+ rtc::CritScope cs_(&lock_); |
+ error_ = ResolveHostname(addr_.hostname().c_str(), addr_.family(), |
tommi
2017/06/08 08:02:06
what about not touching addr_ on any other thread
nisse-webrtc
2017/06/08 09:52:33
|addr_| is written in Start (on construction_threa
|
+ &addresses_); |
+ } |
+ // Ensure SignalDone is called on the main thread. |
+ main_->Invoke<void>(RTC_FROM_HERE, |
nisse-webrtc
2017/06/02 08:41:04
The sequence of events is slightly different from
Taylor Brandstetter
2017/06/05 22:33:25
The new sequence seems better actually. If the wor
|
+ rtc::Bind(&AsyncResolver::OnWorkDone, this)); |
} |
void AsyncResolver::OnWorkDone() { |
Taylor Brandstetter
2017/06/05 22:33:25
nit: Rename to "SignalWorkDone"?
nisse-webrtc
2017/06/07 07:17:31
Done.
|
+ RTC_DCHECK(main_->IsCurrent()); |
SignalDone(this); |
} |
+// static |
+void AsyncResolver::ThreadEntry(void *p) { |
+ AsyncResolver* resolver = static_cast<AsyncResolver*>(p); |
+ resolver->DoWork(); |
+} |
+ |
const char* inet_ntop(int af, const void *src, char* dst, socklen_t size) { |
#if defined(WEBRTC_WIN) |
return win32_inet_ntop(af, src, dst, size); |