Fix memory leak in ThreadWatcher.

chrome/browser/metrics/thread_watcher.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 "chrome/browser/metrics/thread_watcher.h"
 
 #include <math.h>  // ceil
 
 #include "base/bind.h"
 #include "base/compiler_specific.h"
@@ skipping 66 matching lines @@
   }
 
   DCHECK(WatchDogThread::CurrentlyOnWatchDogThread());
 
   // Create a new thread watcher object for the given thread and activate it.
   ThreadWatcher* watcher = new ThreadWatcher(params);
 
   DCHECK(watcher);
   // If we couldn't register the thread watcher object, we are shutting down,
   // then don't activate thread watching.
-  if (!ThreadWatcherList::IsRegistered(params.thread_id))
+  if (!ThreadWatcherList::IsRegistered(params.thread_id)) {
+    delete watcher;

[Ilya Sherman, 2016/08/30 23:51:57]

Could you instead modify |watcher| to be a unique_ptr, and call
ThreadWatcherList::Register() immediately after constructing the watcher, rather
than as part of its constructor?  Moreover, if you update
ThreadWatcherList::Register() to return a boolean, I think you can get rid of
ThreadWatcherList::IsRegistered() and ThreadWatcherList::Find().  I think the
net result would be less, and easier-to-follow, code =)

     return;
+  }
   watcher->ActivateThreadWatching();

[alokp, 2016/08/30 22:32:34]

Is this leaked?

[ramant (doing other things), 2016/08/30 22:45:12]

watcher is stored in g_thread_watcher_list_. It is deleted in
ThreadWatcherList::DeleteAll().

IsRegistered returns false if g_thread_watcher_list_ is null.

This is the case when browser was shutting down even before we started watching
threads. 

We wanted threads to stabilize during browser startup (they would be busy and
threads may not be very responsive) before we start watching them (I think it 2
minutes after startup). 

If browser were to shutdown within 2 minutes and if g_thread_watcher_list_is
already deleted, then the above case could occur.

Writing from memory (so, I could be wrong).

[alokp, 2016/08/30 22:47:21]

Thanks for the explanation. I think a comment about this would be nice.

 }
 
 void ThreadWatcher::ActivateThreadWatching() {
   DCHECK(WatchDogThread::CurrentlyOnWatchDogThread());
   if (active_) return;
   active_ = true;
   ping_count_ = unresponsive_threshold_;
   ResetHangCounters();
   base::ThreadTaskRunnerHandle::Get()->PostTask(
       FROM_HERE, base::Bind(&ThreadWatcher::PostPingMessage,
@@ skipping 899 matching lines @@
 
 #if defined(OS_WIN)
   // On Windows XP, give twice the time for shutdown.
   if (base::win::GetVersion() <= base::win::VERSION_XP)
     actual_duration *= 2;
 #endif
 
   shutdown_watchdog_ = new ShutdownWatchDogThread(actual_duration);
   shutdown_watchdog_->Arm();
 }