commit f582b50dc416de301f3e4a5f1712a93b9ce02e95
author Paul Stewart <pstew@chromium.org> Wed Apr 04 21:39:22 2012 -0700
committer Gerrit <chrome-bot@google.com> Thu Apr 05 09:42:44 2012 -0700
tree 59e4dbe905d4279725d421eba0d6b199373bd739
parent 90d216ddc6752beeb2ff63880b8e6ba379815f68

shill: Fix timer canceling

The move to new callback closures included the frequent replacement
 of "task_factory_.RevokeAll()" with
"weak_ptr_factory_.InvalidateWeakPtrs()".  The former canceled all
pending timers, whereas the latter also effectively canceled all
I/O callbacks.  This caused both portal detection and the service
proxy to stop working.

This CL creates CancelableClosures for the affected timers, and
cancels these closures instead of invalidating all weak pointers.

BUG=chromium-os:28885
TEST=Manual: Test both passing and failure cases of portal detection,
ensuring that DNS timeouts work correctly.  Use "curl -x" to test
HTTP proxy.

Change-Id: Id61dfb6a1a4ce0defaa08a99e318bc510c6c84b3
Reviewed-on: https://gerrit.chromium.org/gerrit/19644
Reviewed-by: Eric Shienbrood <ers@chromium.org>
Commit-Ready: Paul Stewart <pstew@chromium.org>
Tested-by: Paul Stewart <pstew@chromium.org>

dns_client.cc

diff --git a/dns_client.cc b/dns_client.cc
index b357e02..19cf6bc 100644
--- a/dns_client.cc
+++ b/dns_client.cc
@@ -67,10 +67,6 @@
       running_(false),
       resolver_state_(NULL),
       weak_ptr_factory_(this),
-      read_callback_(Bind(&DNSClient::HandleDNSRead,
-                          weak_ptr_factory_.GetWeakPtr())),
-      write_callback_(Bind(&DNSClient::HandleDNSWrite,
-                           weak_ptr_factory_.GetWeakPtr())),
       ares_(Ares::GetInstance()),
       time_(Time::GetInstance()) {}
 
@@ -197,7 +193,7 @@
   }
   VLOG(3) << "In " << __func__;
   running_ = false;
-  weak_ptr_factory_.InvalidateWeakPtrs();
+  timeout_closure_.Cancel();
   dispatcher_->PostTask(Bind(&DNSClient::HandleCompletion,
                              weak_ptr_factory_.GetWeakPtr()));
 
@@ -275,6 +271,10 @@
   int action_bits = ares_->GetSock(resolver_state_->channel, sockets,
                                    ARES_GETSOCK_MAXNUM);
 
+  base::Callback<void(int)> read_callback(
+      Bind(&DNSClient::HandleDNSRead, weak_ptr_factory_.GetWeakPtr()));
+  base::Callback<void(int)> write_callback(
+      Bind(&DNSClient::HandleDNSWrite, weak_ptr_factory_.GetWeakPtr()));
   for (int i = 0; i < ARES_GETSOCK_MAXNUM; i++) {
     if (ARES_GETSOCK_READABLE(action_bits, i)) {
       if (ContainsKey(old_read, sockets[i])) {
@@ -284,7 +284,7 @@
             std::tr1::shared_ptr<IOHandler> (
                 dispatcher_->CreateReadyHandler(sockets[i],
                                                 IOHandler::kModeInput,
-                                                read_callback_));
+                                                read_callback));
       }
     }
     if (ARES_GETSOCK_WRITABLE(action_bits, i)) {
@@ -295,7 +295,7 @@
             std::tr1::shared_ptr<IOHandler> (
                 dispatcher_->CreateReadyHandler(sockets[i],
                                                 IOHandler::kModeOutput,
-                                                write_callback_));
+                                                write_callback));
       }
     }
   }
@@ -313,7 +313,7 @@
   timersub(&now, &resolver_state_->start_time_, &elapsed_time);
   timeout_tv.tv_sec = timeout_ms_ / 1000;
   timeout_tv.tv_usec = (timeout_ms_ % 1000) * 1000;
-  weak_ptr_factory_.InvalidateWeakPtrs();
+  timeout_closure_.Cancel();
 
   if (timercmp(&elapsed_time, &timeout_tv, >=)) {
     // There are 3 cases of interest:
@@ -336,9 +336,10 @@
     timersub(&timeout_tv, &elapsed_time, &max);
     struct timeval *tv = ares_->Timeout(resolver_state_->channel,
                                         &max, &ret_tv);
-    dispatcher_->PostDelayedTask(
-        Bind(&DNSClient::HandleTimeout, weak_ptr_factory_.GetWeakPtr()),
-        tv->tv_sec * 1000 + tv->tv_usec / 1000);
+    timeout_closure_.Reset(
+        Bind(&DNSClient::HandleTimeout, weak_ptr_factory_.GetWeakPtr()));
+    dispatcher_->PostDelayedTask(timeout_closure_.callback(),
+                                 tv->tv_sec * 1000 + tv->tv_usec / 1000);
   }
 
   return true;