[net] Ensure aborted HostResolverImpl::Jobs release slots in the dispatcher.

net/base/host_resolver_impl.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/base/host_resolver_impl.h"
 
 #if defined(OS_WIN)
 #include <Winsock2.h>
 #elif defined(OS_POSIX)
 #include <netdb.h>
@@ skipping 1554 matching lines @@
   Job* job = req->job();
   DCHECK(job);
 
   job->CancelRequest(req);
 
   if (job->num_active_requests() == 0) {
     // If we were called from a Requests' callback within Job::CompleteRequests,
     // that Request could not have been cancelled, so job->num_active_requests()
     // could not be 0. Therefore, we are not in Job::CompleteRequests().
     RemoveJob(job);
-    if (job->is_running())
+    if (job->is_running()) {

[cbentzel, 2012/03/02 19:46:49]

I asked over email - but is there a chance that a job is not |is_running()| but
is still treated like a non-queued job by the dispatcher? That would leak slots
as well.

+      dispatcher_.OnJobFinished();
       job->Abort();

[cbentzel, 2012/03/02 20:02:40]

FYI: I don't think the Abort is needed here. ~Job appears to be doing all the
cleanup that is needed since there are no requests and associated callbacks tied
to this job.

[szym, 2012/03/02 20:05:43]

The cleaning up is now in a few places: Job::~Job, Job::CancelRequest,
HostResolverImpl::CancelRequest. Right now, ~Job is only supposed to take care
of the case when HostResolverImpl is destroyed. I'll refactor this in the next
CL.

+    }
     delete job;
   }
 }
 
 void HostResolverImpl::SetDefaultAddressFamily(AddressFamily address_family) {
   DCHECK(CalledOnValidThread());
   ipv6_probe_monitoring_ = false;
   DiscardIPv6ProbeJob();
   default_address_family_ = address_family;
 }
@@ skipping 115 matching lines @@
   if (effective_address_family == ADDRESS_FAMILY_UNSPECIFIED &&
       default_address_family_ != ADDRESS_FAMILY_UNSPECIFIED) {
     effective_address_family = default_address_family_;
     if (ipv6_probe_monitoring_)
       effective_flags |= HOST_RESOLVER_DEFAULT_FAMILY_SET_DUE_TO_NO_IPV6;
   }
   return Key(info.hostname(), effective_address_family, effective_flags);
 }
 
 void HostResolverImpl::AbortAllInProgressJobs() {
-  base::WeakPtr<HostResolverImpl> self = AsWeakPtr();
   // In Abort, a Request callback could spawn new Jobs with matching keys, so
   // first collect and remove all running jobs from |jobs_|.
   std::vector<Job*> jobs_to_abort;
   for (JobMap::iterator it = jobs_.begin(); it != jobs_.end(); ) {
     Job* job = it->second;
     if (job->is_running()) {
       jobs_to_abort.push_back(job);
       jobs_.erase(it++);
     } else {
       DCHECK(job->is_queued());
       ++it;
     }
   }
 
+  // Check if no dispatcher slots leaked out.
+  DCHECK_EQ(dispatcher_.num_running_jobs(), jobs_to_abort.size());
+
+  // Life check to bail once |this| is deleted.
+  base::WeakPtr<HostResolverImpl> self = AsWeakPtr();
+
   // Then Abort them and dispatch new Jobs.
-  for (size_t i = 0; i < jobs_to_abort.size(); ++i) {
+  for (size_t i = 0; self && i < jobs_to_abort.size(); ++i) {
+    dispatcher_.OnJobFinished();
     jobs_to_abort[i]->Abort();
-    dispatcher_.OnJobFinished();
   }
   STLDeleteElements(&jobs_to_abort);
 }
 
 void HostResolverImpl::OnIPAddressChanged() {
   if (cache_.get())
     cache_->clear();
   if (ipv6_probe_monitoring_) {
     DiscardIPv6ProbeJob();
     ipv6_probe_job_ = new IPv6ProbeJob(this);
@@ skipping 36 matching lines @@
   } else {
     dns_transaction_factory_.reset();
   }
   // Don't Abort running Jobs unless they were running on DnsTransaction.
   // TODO(szym): This will change once http://crbug.com/114827 is fixed.
   if (had_factory)
     OnDNSChanged(NetworkChangeNotifier::CHANGE_DNS_SETTINGS);
 }
 
 }  // namespace net