A more portable HostResolver implementation.

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@108 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 101 insertions, 63 deletions

diff --git a/net/base/host_resolver.cc b/net/base/host_resolver.cc
index 101c5ec..6d79b46 100644
--- a/net/base/host_resolver.cc
+++ b/net/base/host_resolver.cc
@@ -32,7 +32,9 @@
 #include <ws2tcpip.h>
 #include <wspiapi.h>  // Needed for Win2k compat.
 
+#include "base/message_loop.h"
 #include "base/string_util.h"
+#include "base/worker_pool.h"
 #include "net/base/address_list.h"
 #include "net/base/net_errors.h"
 #include "net/base/winsock_init.h"
@@ -56,58 +58,94 @@ static int ResolveAddrInfo(const std::string& host, const std::string& port,
 
 //-----------------------------------------------------------------------------
 
-struct HostResolver::Request :
+class HostResolver::Request :
     public base::RefCountedThreadSafe<HostResolver::Request> {
-  Request() : error(OK), results(NULL) {
-    DuplicateHandle(GetCurrentProcess(), GetCurrentThread(),
-                    GetCurrentProcess(), &origin_thread,
-                    0, FALSE, DUPLICATE_SAME_ACCESS);
+ public:
+  Request(HostResolver* resolver,
+          const std::string& host,
+          const std::string& port,
+          AddressList* addresses,
+          CompletionCallback* callback)
+      : host_(host),
+        port_(port),
+        resolver_(resolver),
+        addresses_(addresses),
+        callback_(callback),
+        origin_loop_(MessageLoop::current()),
+        error_(OK),
+        results_(NULL) {
   }
+  
   ~Request() {
-    CloseHandle(origin_thread);
+    if (results_)
+      freeaddrinfo(results_);
   }
 
-  // Only used on the origin thread (where Resolve was called).
-  AddressList* addresses;
-  CompletionCallback* callback;
+  void DoLookup() {
+    // Running on the worker thread
+    error_ = ResolveAddrInfo(host_, port_, &results_);
 
-  // Set on the origin thread, read on the worker thread.
-  std::string host;
-  std::string port;
-  HANDLE origin_thread;
-
-  // Assigned on the worker thread.
-  int error;
-  struct addrinfo* results;
-
-  static void CALLBACK ReturnResults(ULONG_PTR param) {
-    Request* r = reinterpret_cast<Request*>(param);
-    // The HostResolver may have gone away.
-    if (r->addresses) {
-      if (r->error == OK)
-        r->addresses->Adopt(r->results);
-      if (r->callback)
-        r->callback->Run(r->error);
-    } else if (r->results) {
-      freeaddrinfo(r->results);
+    Task* reply = NewRunnableMethod(this, &Request::DoCallback);
+
+    // The origin loop could go away while we are trying to post to it, so we
+    // need to call its PostTask method inside a lock.  See ~HostResolver.
+    {
+      AutoLock locked(origin_loop_lock_);
+      if (origin_loop_) {
+        origin_loop_->PostTask(FROM_HERE, reply);
+        reply = NULL;
+      }
     }
-    r->Release();
+    
+    // Does nothing if it got posted.
+    delete reply;
   }
 
-  static DWORD WINAPI DoLookup(void* param) {
-    Request* r = static_cast<Request*>(param);
+  void DoCallback() {
+    // Running on the origin thread.
+    DCHECK(error_ || results_);
 
-    r->error = ResolveAddrInfo(r->host, r->port, &r->results);
+    // We may have been cancelled!
+    if (!resolver_)
+      return;
 
-    if (!QueueUserAPC(ReturnResults, r->origin_thread,
-                      reinterpret_cast<ULONG_PTR>(param))) {
-      // The origin thread must have gone away.
-      if (r->results)
-        freeaddrinfo(r->results);
-      r->Release();
+    if (!error_) {
+      addresses_->Adopt(results_);
+      results_ = NULL;
     }
-    return 0;
+
+    // Drop the resolver's reference to us.  Do this before running the
+    // callback since the callback might result in the resolver being
+    // destroyed.
+    resolver_->request_ = NULL;
+
+    callback_->Run(error_);
+  }
+
+  void Cancel() {
+    resolver_ = NULL;
+
+    AutoLock locked(origin_loop_lock_);
+    origin_loop_ = NULL;
   }
+  
+ private:
+  // Set on the origin thread, read on the worker thread.
+  std::string host_;
+  std::string port_;
+
+  // Only used on the origin thread (where Resolve was called).
+  HostResolver* resolver_;
+  AddressList* addresses_;
+  CompletionCallback* callback_;
+
+  // Used to post ourselves onto the origin thread.
+  Lock origin_loop_lock_;
+  MessageLoop* origin_loop_;
+
+  // Assigned on the worker thread, read on the origin thread.
+  int error_;
+  struct addrinfo* results_;
 };
 
 //-----------------------------------------------------------------------------
@@ -117,17 +155,14 @@ HostResolver::HostResolver() {
 }
 
 HostResolver::~HostResolver() {
-  if (request_) {
-    // Prevent the thread pool from running the callback.
-    request_->addresses = NULL;
-    request_->callback = NULL;
-  }
+  if (request_)
+    request_->Cancel();
 }
 
 int HostResolver::Resolve(const std::string& hostname, int port,
                           AddressList* addresses,
                           CompletionCallback* callback) {
-  DCHECK(!request_);
+  DCHECK(!request_) << "resolver already in use";
 
   const std::string& port_str = IntToString(port);
 
@@ -140,18 +175,12 @@ int HostResolver::Resolve(const std::string& hostname, int port,
     return rv;
   }
 
+  request_ = new Request(this, hostname, port_str, addresses, callback);
+
   // Dispatch to worker thread...
-  request_ = new Request();
-  request_->host = hostname;
-  request_->port = port_str;
-  request_->addresses = addresses;
-  request_->callback = callback;
-
-  // Balanced in Request::ReturnResults (or DoLookup if there is an error).
-  request_->AddRef();
-  if (!QueueUserWorkItem(Request::DoLookup, request_, WT_EXECUTELONGFUNCTION)) {
-    DLOG(ERROR) << "QueueUserWorkItem failed: " << GetLastError();
-    request_->Release();
+  if (!WorkerPool::PostTask(FROM_HERE,
+          NewRunnableMethod(request_.get(), &Request::DoLookup), true)) {
+    NOTREACHED();
     request_ = NULL;
     return ERR_FAILED;
   }
diff --git a/net/base/host_resolver.h b/net/base/host_resolver.h
index 2ef7b86..34feacd 100644
--- a/net/base/host_resolver.h
+++ b/net/base/host_resolver.h
@@ -40,8 +40,16 @@ namespace net {
 
 class AddressList;
 
-// This class represents the task of resolving a single hostname.  To resolve
-// multiple hostnames, a new resolver will need to be created for each.
+// This class represents the task of resolving a hostname (or IP address
+// literal) to an AddressList object.  It can only resolve a single hostname at
+// a time, so if you need to resolve multiple hostnames at the same time, you
+// will need to allocate a HostResolver object for each hostname.
+//
+// No attempt is made at this level to cache or pin resolution results.  For
+// each request, this API talks directly to the underlying name resolver of
+// the local system, which may or may not result in a DNS query.  The exact
+// behavior depends on the system configuration.
+//
 class HostResolver {
  public:
   HostResolver();
@@ -51,10 +59,10 @@ class HostResolver {
   // called.
   ~HostResolver();
 
-  // Resolves the given hostname, filling out the |addresses| object upon
-  // success.  The |port| parameter will be set as the sin(6)_port field of
-  // the sockaddr_in{6} struct.  Returns OK if successful or an error code
-  // upon failure.
+  // Resolves the given hostname (or IP address literal), filling out the
+  // |addresses| object upon success.  The |port| parameter will be set as the
+  // sin(6)_port field of the sockaddr_in{6} struct.  Returns OK if successful
+  // or an error code upon failure.
   //
   // When callback is non-null, ERR_IO_PENDING is returned if the operation
   // could not be completed synchronously, in which case the result code will
@@ -64,7 +72,8 @@ class HostResolver {
               AddressList* addresses, CompletionCallback* callback);
 
  private:
-  struct Request;
+  class Request;
+  friend class Request;
   scoped_refptr<Request> request_;
   DISALLOW_EVIL_CONSTRUCTORS(HostResolver);
 };