Fix a memory error in ProxyResolverV8Tracing.

net/proxy/proxy_resolver_v8_tracing.cc

Index: net/proxy/proxy_resolver_v8_tracing.cc
diff --git a/net/proxy/proxy_resolver_v8_tracing.cc b/net/proxy/proxy_resolver_v8_tracing.cc
index ea3257751338966bec79aeaccfa1354567ca72e3..98276536bc0595cf1a5ee58d744a3eacdd753366 100644
--- a/net/proxy/proxy_resolver_v8_tracing.cc
+++ b/net/proxy/proxy_resolver_v8_tracing.cc
@@ -659,22 +659,35 @@ void ProxyResolverV8Tracing::Job::DoDnsOperation(
   if (cancelled_.IsSet())
     return;
 
+  HostResolver::RequestHandle dns_request = NULL;
   int result = host_resolver()->Resolve(
       MakeDnsRequestInfo(host, op),
       &pending_dns_addresses_,
       base::Bind(&Job::OnDnsOperationComplete, this),
-      &pending_dns_,
+      &dns_request,
       bound_net_log_);
 
   bool completed_synchronously = result != ERR_IO_PENDING;
 
+  // Check if the request was cancelled as a side-effect of calling into the
+  // HostResolver. This isn't the ordinary execution flow, however it is
+  // exercised by unit-tests.
+  if (cancelled_.IsSet()) {
+    if (!completed_synchronously)
+      host_resolver()->CancelRequest(dns_request);
+    return;
+  }
+
+  if (!completed_synchronously) {
+    DCHECK(dns_request);
+    pending_dns_ = dns_request;

[mmenke, 2013/01/31 21:11:23]

This isn't correct.  If we're doing a blocking call and complete synchronously,
we won't set pending_dns_ here, and then we'll run into the DCHECK in
OnDnsOperationComplete.

[eroman, 2013/01/31 21:29:49]

Actually it is the DCHECK in OnDnsOperationComplete() which is wrong. If the
request completed synchronously, then pending_dns_ should be NULL. Adding the
assignment shouldn't change that catch.

Good catch! This is a left-over from one of my prior refactorings.

For now I have removed the DCHECK. I will follow-up with a test to exercise this
situation, and see if I can't clarify the code in that as well.

[mmenke, 2013/01/31 21:34:32]

You're right.  Was thinking that we could just always set it, but then we'd be
relying on it being non-NULL on synchronous completion.

+  }
+
   if (!blocking_dns_) {
     // Check if the DNS result can be serviced directly from the cache.
     // (The worker thread is blocked waiting for this information).
-    if (completed_synchronously) {
+    if (completed_synchronously)
       SaveDnsToLocalCache(host, op, result, pending_dns_addresses_);
-      pending_dns_ = NULL;
-    }
 
     // Important: Do not read/write |out_cache_hit| after signalling, since
     // the memory may no longer be valid.