Add checks to DEBUG mode that no instance of URLRequest or URLFetcher survives the destruction of the IO thread.

This checking is done by introducing a new helper class to base called LeakTracker. Classes that you want to check for leaks just need to extend LeakTracker.

The reason I am picking on URLFetcher / URLRequest, is I believe we have a bug that is making an instance of URLFetcher to outlive the IO thread.

This causes various sorts of badness.

For example:
If URLFetcher survives the IO thread, then URLRequestContext remains referenced and therefore also survives IO thread. In turn HostResolverImpl survives the IO thread, so any outstanding resolve requests are NOT cancelled before the IO thread is decomissioned. So now, when the worker thread doing the DNS resolve finally finishes (assuming it finishes before the rogue URLRequest is destroyed), it post the result to a defunct message loop. KAB00m! (http://crbug.com/15513)

Moreover, I believe we hit this same problem sporadically in AutomationProxyTest.AutocompleteGetSetText -- the test is flaky on the buildbots, and I've seen DCHECKs which suggest it is related to this issue.

BUG=http://crbug.com/18372
Review URL: http://codereview.chromium.org/160447

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

1 files changed, 39 insertions, 11 deletions

diff --git a/chrome/browser/browser_process_impl.cc b/chrome/browser/browser_process_impl.cc
index 552e171..4007cf6 100644
--- a/chrome/browser/browser_process_impl.cc
+++ b/chrome/browser/browser_process_impl.cc
@@ -58,7 +58,7 @@ class BrowserProcessSubThread : public ChromeThread {
       : ChromeThread(identifier) {
   }
 
-  ~BrowserProcessSubThread() {
+  virtual ~BrowserProcessSubThread() {
     // We cannot rely on our base class to stop the thread since we want our
     // CleanUp function to run.
     Stop();
@@ -92,6 +92,26 @@ class BrowserProcessSubThread : public ChromeThread {
   NotificationService* notification_service_;
 };
 
+class IOThread : public BrowserProcessSubThread {
+ public:
+  IOThread() : BrowserProcessSubThread(ChromeThread::IO) {}
+
+  virtual ~IOThread() {
+    // We cannot rely on our base class to stop the thread since we want our
+    // CleanUp function to run.
+    Stop();
+  }
+
+ protected:
+  virtual void CleanUp() {
+    // URLFetcher and URLRequest instances must NOT outlive the IO thread.
+    base::LeakTracker<URLRequest>::CheckForLeaks();
+    base::LeakTracker<URLFetcher>::CheckForLeaks();
+
+    BrowserProcessSubThread::CleanUp();
+  }
+};
+
 }  // namespace
 
 BrowserProcessImpl::BrowserProcessImpl(const CommandLine& command_line)
@@ -172,13 +192,6 @@ BrowserProcessImpl::~BrowserProcessImpl() {
     resource_dispatcher_host()->Shutdown();
   }
 
-  // Shutdown DNS prefetching now to ensure that network stack objects
-  // living on the IO thread get destroyed before the IO thread goes away.
-  if (io_thread_.get()) {
-    io_thread_->message_loop()->PostTask(FROM_HERE,
-        NewRunnableFunction(chrome_browser_net::EnsureDnsPrefetchShutdown));
-  }
-
 #if defined(OS_LINUX)
   // The IO thread must outlive the BACKGROUND_X11 thread.
   background_x11_thread_.reset();
@@ -187,7 +200,7 @@ BrowserProcessImpl::~BrowserProcessImpl() {
   // Need to stop io_thread_ before resource_dispatcher_host_, since
   // io_thread_ may still deref ResourceDispatcherHost and handle resource
   // request before going away.
-  io_thread_.reset();
+  ResetIOThread();
 
   // Clean up state that lives on the file_thread_ before it goes away.
   if (resource_dispatcher_host_.get()) {
@@ -307,8 +320,7 @@ void BrowserProcessImpl::CreateIOThread() {
   background_x11_thread_.swap(background_x11_thread);
 #endif
 
-  scoped_ptr<base::Thread> thread(
-      new BrowserProcessSubThread(ChromeThread::IO));
+  scoped_ptr<base::Thread> thread(new IOThread);
   base::Thread::Options options;
   options.message_loop_type = MessageLoop::TYPE_IO;
   if (!thread->StartWithOptions(options))
@@ -316,6 +328,22 @@ void BrowserProcessImpl::CreateIOThread() {
   io_thread_.swap(thread);
 }
 
+void BrowserProcessImpl::ResetIOThread() {
+  if (io_thread_.get()) {
+    io_thread_->message_loop()->PostTask(FROM_HERE,
+        NewRunnableFunction(CleanupOnIOThread));
+  }
+  io_thread_.reset();
+}
+
+// static
+void BrowserProcessImpl::CleanupOnIOThread() {
+  // Shutdown DNS prefetching now to ensure that network stack objects
+  // living on the IO thread get destroyed before the IO thread goes away.
+  chrome_browser_net::EnsureDnsPrefetchShutdown();
+  // TODO(eroman): can this be merged into IOThread::CleanUp() ?
+}
+
 void BrowserProcessImpl::CreateFileThread() {
   DCHECK(!created_file_thread_ && file_thread_.get() == NULL);
   created_file_thread_ = true;