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 61 matching lines @@
   if (!WatchDogThread::CurrentlyOnWatchDogThread()) {
     WatchDogThread::PostTask(
         FROM_HERE,
         base::Bind(&ThreadWatcher::StartWatching, params));
     return;
   }
 
   DCHECK(WatchDogThread::CurrentlyOnWatchDogThread());
 
   // Create a new thread watcher object for the given thread and activate it.
-  ThreadWatcher* watcher = new ThreadWatcher(params);
+  std::unique_ptr<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))
+  // so don't activate thread watching.
+  if (!ThreadWatcherList::Register(std::move(watcher)))
     return;
   watcher->ActivateThreadWatching();

[Jakob Kummerow, 2016/09/01 13:35:17]

I believe using |watcher| after calling std::move() on it is a Bad Idea™.

[Ilya Sherman, 2016/09/01 18:15:18]

Ah, good call -- my bad for missing that!

One possible way to restructure the code: Change ThreadWatcherList::Register()
to also call ActivateThreadWatching().  Another: Have
ThreadWatcherList::Register() return a pointer to the registered watcher, or
null if registration failed.

 }
 
 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 117 matching lines @@
   base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
       FROM_HERE,
       base::Bind(&ThreadWatcher::OnCheckResponsiveness,
                  weak_ptr_factory_.GetWeakPtr(), ping_sequence_number_),
       unresponsive_time_);
   responsive_ = false;
 }
 
 void ThreadWatcher::Initialize() {
   DCHECK(WatchDogThread::CurrentlyOnWatchDogThread());
-  ThreadWatcherList::Register(this);
 
   const std::string response_time_histogram_name =
       "ThreadWatcher.ResponseTime." + thread_name_;
   response_time_histogram_ = base::Histogram::FactoryTimeGet(
       response_time_histogram_name,
       base::TimeDelta::FromMilliseconds(1),
       base::TimeDelta::FromSeconds(100), 50,
       base::Histogram::kUmaTargetedHistogramFlag);
 
   const std::string unresponsive_time_histogram_name =
@@ skipping 137 matching lines @@
 }
 
 // static
 void ThreadWatcherList::StopWatchingAll() {
   // TODO(rtenneti): Enable ThreadWatcher.
   ThreadWatcherObserver::RemoveNotifications();
   DeleteAll();
 }
 
 // static
-void ThreadWatcherList::Register(ThreadWatcher* watcher) {
+bool ThreadWatcherList::Register(std::unique_ptr<ThreadWatcher> watcher) {
   DCHECK(WatchDogThread::CurrentlyOnWatchDogThread());
   if (!g_thread_watcher_list_)
-    return;
-  DCHECK(!g_thread_watcher_list_->Find(watcher->thread_id()));
-  g_thread_watcher_list_->registered_[watcher->thread_id()] = watcher;
+    return false;
+  content::BrowserThread::ID thread_id = watcher->thread_id();
+  DCHECK(g_thread_watcher_list_->registered_.count(thread_id) == 0);
+  g_thread_watcher_list_->registered_[thread_id] = watcher.release();
+  return true;
 }
 
 // static
-bool ThreadWatcherList::IsRegistered(const BrowserThread::ID thread_id) {
-  DCHECK(WatchDogThread::CurrentlyOnWatchDogThread());
-  return nullptr != ThreadWatcherList::Find(thread_id);
-}
-
-// static
 void ThreadWatcherList::GetStatusOfThreads(
     uint32_t* responding_thread_count,
     uint32_t* unresponding_thread_count) {
   DCHECK(WatchDogThread::CurrentlyOnWatchDogThread());
   *responding_thread_count = 0;
   *unresponding_thread_count = 0;
   if (!g_thread_watcher_list_)
     return;
 
   for (RegistrationList::iterator it =
@@ skipping 589 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();
 }