Delete SignalThread class.

webrtc/base/nethelpers.cc

 /*
  *  Copyright 2008 The WebRTC Project Authors. All rights reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
 #include "webrtc/base/nethelpers.h"
 
 #include <memory>
 
 #if defined(WEBRTC_WIN)
 #include <ws2spi.h>
 #include <ws2tcpip.h>
 #include "webrtc/base/win32.h"
 #endif
 #if defined(WEBRTC_POSIX) && !defined(__native_client__)
 #if defined(WEBRTC_ANDROID)
 #include "webrtc/base/ifaddrs-android.h"
 #else
 #include <ifaddrs.h>
 #endif
 #endif  // defined(WEBRTC_POSIX) && !defined(__native_client__)
 
+#include "webrtc/base/bind.h"
 #include "webrtc/base/byteorder.h"
 #include "webrtc/base/checks.h"
 #include "webrtc/base/logging.h"
-#include "webrtc/base/signalthread.h"
+#include "webrtc/base/ptr_util.h"
+#include "webrtc/base/task_queue.h"
+#include "webrtc/base/thread.h"
 
 namespace rtc {
 
+namespace {
 int ResolveHostname(const std::string& hostname, int family,
                     std::vector<IPAddress>* addresses) {
 #ifdef __native_client__
   RTC_NOTREACHED();
   LOG(LS_WARNING) << "ResolveHostname() is not implemented for NaCl";
   return -1;
 #else  // __native_client__
   if (!addresses) {
     return -1;
   }
@@ skipping 29 matching lines @@
       IPAddress ip;
       if (IPFromAddrInfo(cursor, &ip)) {
         addresses->push_back(ip);
       }
     }
   }
   freeaddrinfo(result);
   return 0;
 #endif  // !__native_client__
 }
+}  // namespace
 
 // AsyncResolver
 AsyncResolver::AsyncResolver()
-    : SignalThread(false /* use_socket_server */), error_(-1) {}
+    : state_(State::NONE), construction_thread_(Thread::Current()) {
+  RTC_DCHECK(construction_thread_);
+}
 
-AsyncResolver::~AsyncResolver() = default;
+AsyncResolver::~AsyncResolver() {
+  RTC_DCHECK(construction_thread_->IsCurrent());
+}
 
 void AsyncResolver::Start(const SocketAddress& addr) {
+  RTC_DCHECK_RUN_ON(construction_thread_);
+  RTC_DCHECK(!resolver_queue_);
+  RTC_DCHECK(state_ == State::NONE);
+  // TODO(nisse): Support injection of task queue at construction?
+  resolver_queue_ = rtc::MakeUnique<TaskQueue>("AsyncResolverQueue");
   addr_ = addr;
-  // SignalThred Start will kickoff the resolve process.
-  SignalThread::Start();
+  resolver_queue_->PostTask([this]() {
+    std::vector<IPAddress> addresses;
+    int error =
+        ResolveHostname(addr_.hostname().c_str(), addr_.family(), &addresses);
+    // Ensure SignalDone is called on the main thread.
+    // TODO(nisse): Despite std::move below, we likely get a copy, due to the
+    // way rtc::Bind is implemented.
+    construction_thread_->Post(
+        RTC_FROM_HERE, rtc::Bind(&AsyncResolver::ResolveDone, this, error,
+                                 std::move(addresses)));
+  });
+  state_ = State::PENDING;
 }
 
 bool AsyncResolver::GetResolvedAddress(int family, SocketAddress* addr) const {
   if (error_ != 0 || addresses_.empty())
     return false;
 
   *addr = addr_;
   for (size_t i = 0; i < addresses_.size(); ++i) {
     if (family == addresses_[i].family()) {
       addr->SetResolvedIP(addresses_[i]);
       return true;
     }
   }
   return false;
 }
 
 int AsyncResolver::GetError() const {
   return error_;
 }
 
 void AsyncResolver::Destroy(bool wait) {
-  SignalThread::Destroy(wait);
+  RTC_DCHECK_RUN_ON(construction_thread_);
+  RTC_DCHECK(state_ != State::DESTROYED);
+  // If we don't wait here, we will nevertheless wait in the destructor.
+  if (wait || state_ != State::PENDING) {
+    // Destroy task queue, blocks on any currently running task. If we have a
+    // pending task, it will post a call to ResolveDone before finishing, which
+    // we will never handle.

[tommi, 2017/06/27 12:01:14]

this comment doesn't really explain why it's OK to sometimes delete |this| and
sometimes not. Especially considering that we might do that depending on
internal state, I'm wondering how the external code can know what to do with the
pointer to the AsyncResolver object

[nisse-webrtc, 2017/06/28 11:47:54]

The AsyncResolverInterface is poorly documented, but the way I understand it,
after Destroy() returns, the user should consider the object deleted and not
access it in any way.

Maybe users of the class also shouldn't call the destructor explicitly, perhaps
~AsyncResolverInterface() should be demoted from public to protected. But in any
case, it's invalid to explicitly destruct the object after calling the Destroy
method.

+    delete this;
+  } else {
+    state_ = State::DESTROYED;
+  }
 }
 
-void AsyncResolver::DoWork() {
-  error_ = ResolveHostname(addr_.hostname().c_str(), addr_.family(),
-                           &addresses_);
-}
-
-void AsyncResolver::OnWorkDone() {
-  SignalDone(this);
+void AsyncResolver::ResolveDone(int error, std::vector<IPAddress> addresses) {
+  RTC_DCHECK_RUN_ON(construction_thread_);
+  error_ = error;
+  addresses_ = std::move(addresses);
+  switch (state_) {
+    case State::PENDING:
+      SignalDone(this);
+      state_ = State::NONE;
+      break;
+    case State::DESTROYED:
+      delete this;
+      break;
+    default:
+      RTC_NOTREACHED();
+  }
 }
 
 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);
 #else
   return ::inet_ntop(af, src, dst, size);
 #endif
 }
 
@@ skipping 74 matching lines @@
       break;
     }
   }
   freeifaddrs(ifa);
   return has_ipv6;
 #else
   return true;
 #endif
 }
 }  // namespace rtc