net: Add fuzzer for HostResolverImpl.

net/dns/host_resolver_impl.cc

Index: net/dns/host_resolver_impl.cc
diff --git a/net/dns/host_resolver_impl.cc b/net/dns/host_resolver_impl.cc
index 5559c8d460fbbed748f66146ac784f2d8e1cf9d7..d738d5536bb122661ae2b2c0eb69d97711d0653e 100644
--- a/net/dns/host_resolver_impl.cc
+++ b/net/dns/host_resolver_impl.cc
@@ -24,8 +24,10 @@
 #include "base/callback.h"
 #include "base/compiler_specific.h"
 #include "base/debug/debugger.h"
+#include "base/debug/leak_annotations.h"
 #include "base/debug/stack_trace.h"
 #include "base/macros.h"
+#include "base/memory/ptr_util.h"
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/metrics/sparse_histogram.h"
@@ -621,11 +623,13 @@ class HostResolverImpl::ProcTask
   ProcTask(const Key& key,
            const ProcTaskParams& params,
            const Callback& callback,
+           scoped_refptr<base::TaskRunner> worker_task_runner,
            const BoundNetLog& job_net_log)
       : key_(key),
         params_(params),
         callback_(callback),
-        task_runner_(base::ThreadTaskRunnerHandle::Get()),
+        worker_task_runner_(std::move(worker_task_runner)),
+        network_task_runner_(base::ThreadTaskRunnerHandle::Get()),
         attempt_number_(0),
         completed_attempt_number_(0),
         completed_attempt_error_(ERR_UNEXPECTED),
@@ -639,7 +643,7 @@ class HostResolverImpl::ProcTask
   }
 
   void Start() {
-    DCHECK(task_runner_->BelongsToCurrentThread());
+    DCHECK(network_task_runner_->BelongsToCurrentThread());
     net_log_.BeginEvent(NetLog::TYPE_HOST_RESOLVER_IMPL_PROC_TASK);
     StartLookupAttempt();
   }
@@ -648,7 +652,7 @@ class HostResolverImpl::ProcTask
   // attempts running on worker threads will continue running. Only once all the
   // attempts complete will the final reference to this ProcTask be released.
   void Cancel() {
-    DCHECK(task_runner_->BelongsToCurrentThread());
+    DCHECK(network_task_runner_->BelongsToCurrentThread());
 
     if (was_canceled() || was_completed())
       return;
@@ -658,17 +662,17 @@ class HostResolverImpl::ProcTask
   }
 
   void set_had_non_speculative_request() {
-    DCHECK(task_runner_->BelongsToCurrentThread());
+    DCHECK(network_task_runner_->BelongsToCurrentThread());
     had_non_speculative_request_ = true;
   }
 
   bool was_canceled() const {
-    DCHECK(task_runner_->BelongsToCurrentThread());
+    DCHECK(network_task_runner_->BelongsToCurrentThread());
     return callback_.is_null();
   }
 
   bool was_completed() const {
-    DCHECK(task_runner_->BelongsToCurrentThread());
+    DCHECK(network_task_runner_->BelongsToCurrentThread());
     return completed_attempt_number_ > 0;
   }
 
@@ -677,27 +681,24 @@ class HostResolverImpl::ProcTask
   ~ProcTask() {}
 
   void StartLookupAttempt() {
-    DCHECK(task_runner_->BelongsToCurrentThread());
+    DCHECK(network_task_runner_->BelongsToCurrentThread());
     base::TimeTicks start_time = base::TimeTicks::Now();
     ++attempt_number_;
     // Dispatch the lookup attempt to a worker thread.
-    if (!base::WorkerPool::PostTask(
-            FROM_HERE,
-            base::Bind(&ProcTask::DoLookup, this, start_time, attempt_number_),
-            true)) {
+    // TODO(mmenke): Chrome code generally doesn't handle failed PostTasks.

[eroman, 2016/05/17 23:01:34]

Arguably that other code is wrong :)
Also best not to reference Chrome in //net code.
Plus, Chromium not Chrome

[mmenke, 2016/05/19 19:09:45]

Just removed the TODO.  In Chrome, the preference is not to handle things that
can't happen (And it seems that in practice, this can't happen).  If you feel we
should be handling all possible PostTask failures, I'm more than happy to file a
bug and assigned it to you.  :)

+    // Should this code really be any different?
+    if (!worker_task_runner_->PostTask(
+            FROM_HERE, base::Bind(&ProcTask::DoLookup, this, start_time,
+                                  attempt_number_))) {
       NOTREACHED();
 
-      // Since we could be running within Resolve() right now, we can't just
-      // call OnLookupComplete().  Instead we must wait until Resolve() has
-      // returned (IO_PENDING).
-      task_runner_->PostTask(FROM_HERE,
-                             base::Bind(&ProcTask::OnLookupComplete,
-                                        this,
-                                        AddressList(),
-                                        start_time,
-                                        attempt_number_,
-                                        ERR_UNEXPECTED,
-                                        0));
+      // Since this method may have been called from Resolve(), can't just call
+      // OnLookupComplete().  Instead, must wait until Resolve() has returned
+      // (IO_PENDING).
+      network_task_runner_->PostTask(
+          FROM_HERE,
+          base::Bind(&ProcTask::OnLookupComplete, this, AddressList(),
+                     start_time, attempt_number_, ERR_UNEXPECTED, 0));
       return;
     }
 
@@ -708,9 +709,8 @@ class HostResolverImpl::ProcTask
     // will start a new attempt on a different worker thread if none of our
     // outstanding attempts have completed yet.
     if (attempt_number_ <= params_.max_retry_attempts) {
-      task_runner_->PostDelayedTask(
-          FROM_HERE,
-          base::Bind(&ProcTask::RetryIfNotComplete, this),
+      network_task_runner_->PostDelayedTask(
+          FROM_HERE, base::Bind(&ProcTask::RetryIfNotComplete, this),
           params_.unresponsive_delay);
     }
   }
@@ -741,20 +741,15 @@ class HostResolverImpl::ProcTask
       }
     }
 
-    task_runner_->PostTask(FROM_HERE,
-                           base::Bind(&ProcTask::OnLookupComplete,
-                                      this,
-                                      results,
-                                      start_time,
-                                      attempt_number,
-                                      error,
-                                      os_error));
+    network_task_runner_->PostTask(
+        FROM_HERE, base::Bind(&ProcTask::OnLookupComplete, this, results,
+                              start_time, attempt_number, error, os_error));
   }
 
   // Makes next attempt if DoLookup() has not finished (runs on task runner

[eroman, 2016/05/17 23:01:34]

update "task runner" in comment (which is now ambiguous).
Or just delete, the DCHECK() also documents this.

[mmenke, 2016/05/19 19:09:45]

Removed the comment, agree the DCHECK is sufficient.

   // thread).
   void RetryIfNotComplete() {
-    DCHECK(task_runner_->BelongsToCurrentThread());
+    DCHECK(network_task_runner_->BelongsToCurrentThread());
 
     if (was_completed() || was_canceled())
       return;
@@ -770,7 +765,7 @@ class HostResolverImpl::ProcTask
                         int error,
                         const int os_error) {
     TRACE_EVENT0("net", "ProcTask::OnLookupComplete");
-    DCHECK(task_runner_->BelongsToCurrentThread());
+    DCHECK(network_task_runner_->BelongsToCurrentThread());
     // If results are empty, we should return an error.
     bool empty_list_on_ok = (error == OK && results.empty());
     UMA_HISTOGRAM_BOOLEAN("DNS.EmptyAddressListAndNoError", empty_list_on_ok);
@@ -836,7 +831,7 @@ class HostResolverImpl::ProcTask
   void RecordPerformanceHistograms(const base::TimeTicks& start_time,
                                    const int error,
                                    const int os_error) const {
-    DCHECK(task_runner_->BelongsToCurrentThread());
+    DCHECK(network_task_runner_->BelongsToCurrentThread());
     enum Category {  // Used in UMA_HISTOGRAM_ENUMERATION.
       RESOLVE_SUCCESS,
       RESOLVE_FAIL,
@@ -903,7 +898,7 @@ class HostResolverImpl::ProcTask
                                const uint32_t attempt_number,
                                const int error,
                                const int os_error) const {
-    DCHECK(task_runner_->BelongsToCurrentThread());
+    DCHECK(network_task_runner_->BelongsToCurrentThread());
     bool first_attempt_to_complete =
         completed_attempt_number_ == attempt_number;
     bool is_first_attempt = (attempt_number == 1);
@@ -962,8 +957,11 @@ class HostResolverImpl::ProcTask
   // The listener to the results of this ProcTask.
   Callback callback_;
 
-  // Used to post ourselves onto the task runner thread.
-  scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
+  // Task runner for the call to the platform address resolver.

[eroman, 2016/05/17 23:01:34]

Not necessarily using the "platform" address resolver.

[mmenke, 2016/05/19 19:09:45]

Done.

+  scoped_refptr<base::TaskRunner> worker_task_runner_;
+
+  // Used to post events onto the network thread.

[eroman, 2016/05/17 23:01:34]

I wonder if "origin" vs "worker" would be more clear.

[mmenke, 2016/05/19 19:09:45]

I prefer network thread.  "network thread" sounds like a specific thread. 
"Origin thread" could imply any thread the HostResolver is called on.

(I agree that "network" isn't great or relevant here, though - open to other
ideas)

+  scoped_refptr<base::SingleThreadTaskRunner> network_task_runner_;
 
   // Keeps track of the number of attempts we have made so far to resolve the
   // host. Whenever we start an attempt to resolve the host, we increase this
@@ -999,16 +997,19 @@ class HostResolverImpl::ProcTask
 // it takes 40-100ms and should not block initialization.
 class HostResolverImpl::LoopbackProbeJob {
  public:
-  explicit LoopbackProbeJob(const base::WeakPtr<HostResolverImpl>& resolver)
-      : resolver_(resolver),
-        result_(false) {
+  LoopbackProbeJob(const base::WeakPtr<HostResolverImpl>& resolver,
+                   base::TaskRunner* worker_task_runner)
+      : resolver_(resolver), result_(false) {
     DCHECK(resolver.get());
-    const bool kIsSlow = true;
-    base::WorkerPool::PostTaskAndReply(
+    // Do not report worker pool leaks in tests. The WorkerPool doesn't have a
+    // flushing API, so can't do anything about them, other than using another
+    // task runner.
+    // http://crbug.com/248513
+    ANNOTATE_SCOPED_MEMORY_LEAK;
+    worker_task_runner->PostTaskAndReply(
         FROM_HERE,
         base::Bind(&LoopbackProbeJob::DoProbe, base::Unretained(this)),
-        base::Bind(&LoopbackProbeJob::OnProbeComplete, base::Owned(this)),
-        kIsSlow);
+        base::Bind(&LoopbackProbeJob::OnProbeComplete, base::Owned(this)));
   }
 
   virtual ~LoopbackProbeJob() {}
@@ -1270,10 +1271,12 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job,
   Job(const base::WeakPtr<HostResolverImpl>& resolver,
       const Key& key,
       RequestPriority priority,
+      scoped_refptr<base::TaskRunner> worker_task_runner,
       const BoundNetLog& source_net_log)
       : resolver_(resolver),
         key_(key),
         priority_tracker_(priority),
+        worker_task_runner_(std::move(worker_task_runner)),
         had_non_speculative_request_(false),
         had_dns_config_(false),
         num_occupied_job_slots_(0),
@@ -1549,12 +1552,11 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job,
   // tighter limits.
   void StartProcTask() {
     DCHECK(!is_dns_running());
-    proc_task_ = new ProcTask(
-        key_,
-        resolver_->proc_params_,
-        base::Bind(&Job::OnProcTaskComplete, base::Unretained(this),
-                   base::TimeTicks::Now()),
-        net_log_);
+    proc_task_ =
+        new ProcTask(key_, resolver_->proc_params_,
+                     base::Bind(&Job::OnProcTaskComplete,
+                                base::Unretained(this), base::TimeTicks::Now()),
+                     worker_task_runner_, net_log_);
 
     if (had_non_speculative_request_)
       proc_task_->set_had_non_speculative_request();
@@ -1816,6 +1818,9 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job,
   // Tracks the highest priority across |requests_|.
   PriorityTracker priority_tracker_;
 
+  // Task runner where the HostResolverProc is invoked.
+  scoped_refptr<base::TaskRunner> worker_task_runner_;
+
   bool had_non_speculative_request_;
 
   // Distinguishes measurements taken while DnsClient was fully configured.
@@ -1867,53 +1872,10 @@ HostResolverImpl::ProcTaskParams::ProcTaskParams(const ProcTaskParams& other) =
 HostResolverImpl::ProcTaskParams::~ProcTaskParams() {}
 
 HostResolverImpl::HostResolverImpl(const Options& options, NetLog* net_log)
-    : max_queued_jobs_(0),
-      proc_params_(NULL, options.max_retry_attempts),
-      net_log_(net_log),
-      received_dns_config_(false),
-      num_dns_failures_(0),
-      use_local_ipv6_(false),
-      last_ipv6_probe_result_(true),
-      resolved_known_ipv6_hostname_(false),
-      additional_resolver_flags_(0),
-      fallback_to_proctask_(true),
-      weak_ptr_factory_(this),
-      probe_weak_ptr_factory_(this) {
-  if (options.enable_caching)
-    cache_ = HostCache::CreateDefaultCache();
-
-  PrioritizedDispatcher::Limits job_limits = options.GetDispatcherLimits();
-  dispatcher_.reset(new PrioritizedDispatcher(job_limits));
-  max_queued_jobs_ = job_limits.total_jobs * 100u;
-
-  DCHECK_GE(dispatcher_->num_priorities(), static_cast<size_t>(NUM_PRIORITIES));
-
-#if defined(OS_WIN)
-  EnsureWinsockInit();
-#endif
-#if defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_ANDROID)
-  new LoopbackProbeJob(weak_ptr_factory_.GetWeakPtr());
-#endif
-  NetworkChangeNotifier::AddIPAddressObserver(this);
-  NetworkChangeNotifier::AddConnectionTypeObserver(this);
-  NetworkChangeNotifier::AddDNSObserver(this);
-#if defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_OPENBSD) && \
-    !defined(OS_ANDROID)
-  EnsureDnsReloaderInit();
-#endif
-
-  OnConnectionTypeChanged(NetworkChangeNotifier::GetConnectionType());
-
-  {
-    DnsConfig dns_config;
-    NetworkChangeNotifier::GetDnsConfig(&dns_config);
-    received_dns_config_ = dns_config.IsValid();
-    // Conservatively assume local IPv6 is needed when DnsConfig is not valid.
-    use_local_ipv6_ = !dns_config.IsValid() || dns_config.use_local_ipv6;
-  }
-
-  fallback_to_proctask_ = !ConfigureAsyncDnsNoFallbackFieldTrial();
-}
+    : HostResolverImpl(
+          options,
+          net_log,
+          base::WorkerPool::GetTaskRunner(true /* task_is_slow */)) {}
 
 HostResolverImpl::~HostResolverImpl() {
   // Prevent the dispatcher from starting new jobs.
@@ -1972,8 +1934,8 @@ int HostResolverImpl::Resolve(const RequestInfo& info,
   JobMap::iterator jobit = jobs_.find(key);
   Job* job;
   if (jobit == jobs_.end()) {
-    job =
-        new Job(weak_ptr_factory_.GetWeakPtr(), key, priority, source_net_log);
+    job = new Job(weak_ptr_factory_.GetWeakPtr(), key, priority,
+                  worker_task_runner_, source_net_log);
     job->Schedule(false);
 
     // Check for queue overflow.
@@ -2003,6 +1965,67 @@ int HostResolverImpl::Resolve(const RequestInfo& info,
   return ERR_IO_PENDING;
 }
 
+HostResolverImpl::HostResolverImpl(
+    const Options& options,
+    NetLog* net_log,
+    scoped_refptr<base::TaskRunner> worker_task_runner)
+    : max_queued_jobs_(0),
+      proc_params_(NULL, options.max_retry_attempts),
+      net_log_(net_log),
+      received_dns_config_(false),
+      num_dns_failures_(0),
+      use_local_ipv6_(false),
+      last_ipv6_probe_result_(true),
+      resolved_known_ipv6_hostname_(false),
+      additional_resolver_flags_(0),
+      fallback_to_proctask_(true),
+      worker_task_runner_(std::move(worker_task_runner)),
+      weak_ptr_factory_(this),
+      probe_weak_ptr_factory_(this) {
+  if (options.enable_caching)
+    cache_ = HostCache::CreateDefaultCache();
+
+  PrioritizedDispatcher::Limits job_limits = options.GetDispatcherLimits();
+  dispatcher_.reset(new PrioritizedDispatcher(job_limits));
+  max_queued_jobs_ = job_limits.total_jobs * 100u;
+
+  DCHECK_GE(dispatcher_->num_priorities(), static_cast<size_t>(NUM_PRIORITIES));
+
+#if defined(OS_WIN)
+  EnsureWinsockInit();
+#endif
+#if defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_ANDROID)
+  RunLoopbackProbeJob();
+#endif
+  NetworkChangeNotifier::AddIPAddressObserver(this);
+  NetworkChangeNotifier::AddConnectionTypeObserver(this);
+  NetworkChangeNotifier::AddDNSObserver(this);
+#if defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_OPENBSD) && \
+    !defined(OS_ANDROID)
+  EnsureDnsReloaderInit();
+#endif
+
+  OnConnectionTypeChanged(NetworkChangeNotifier::GetConnectionType());
+
+  {
+    DnsConfig dns_config;
+    NetworkChangeNotifier::GetDnsConfig(&dns_config);
+    received_dns_config_ = dns_config.IsValid();
+    // Conservatively assume local IPv6 is needed when DnsConfig is not valid.
+    use_local_ipv6_ = !dns_config.IsValid() || dns_config.use_local_ipv6;
+  }
+
+  fallback_to_proctask_ = !ConfigureAsyncDnsNoFallbackFieldTrial();
+}
+
+void HostResolverImpl::SetHaveOnlyLoopbackAddresses(bool result) {
+  if (result) {
+    additional_resolver_flags_ |= HOST_RESOLVER_LOOPBACK_ONLY;
+  } else {
+    additional_resolver_flags_ &= ~HOST_RESOLVER_LOOPBACK_ONLY;
+  }
+}
+
 int HostResolverImpl::ResolveHelper(const Key& key,
                                     const RequestInfo& info,
                                     const IPAddress* ip_address,
@@ -2241,14 +2264,6 @@ void HostResolverImpl::RemoveJob(Job* job) {
     jobs_.erase(it);
 }
 
-void HostResolverImpl::SetHaveOnlyLoopbackAddresses(bool result) {
-  if (result) {
-    additional_resolver_flags_ |= HOST_RESOLVER_LOOPBACK_ONLY;
-  } else {
-    additional_resolver_flags_ &= ~HOST_RESOLVER_LOOPBACK_ONLY;
-  }
-}
-
 HostResolverImpl::Key HostResolverImpl::GetEffectiveKeyForRequest(
     const RequestInfo& info,
     const IPAddress* ip_address,
@@ -2291,6 +2306,11 @@ bool HostResolverImpl::IsIPv6Reachable(const BoundNetLog& net_log) {
   return last_ipv6_probe_result_;
 }
 
+void HostResolverImpl::RunLoopbackProbeJob() {
+  new LoopbackProbeJob(weak_ptr_factory_.GetWeakPtr(),
+                       worker_task_runner_.get());
+}
+
 void HostResolverImpl::AbortAllInProgressJobs() {
   // In Abort, a Request callback could spawn new Jobs with matching keys, so
   // first collect and remove all running jobs from |jobs_|.
@@ -2366,7 +2386,7 @@ void HostResolverImpl::OnIPAddressChanged() {
   if (cache_.get())
     cache_->clear();
 #if defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_ANDROID)
-  new LoopbackProbeJob(probe_weak_ptr_factory_.GetWeakPtr());
+  RunLoopbackProbeJob();
 #endif
   AbortAllInProgressJobs();
   // |this| may be deleted inside AbortAllInProgressJobs().