diff options
| -rw-r--r-- | chrome/browser/browser_main.cc | 15 | ||||
| -rw-r--r-- | chrome/browser/browser_process.h | 4 | ||||
| -rw-r--r-- | chrome/browser/browser_process_impl.cc | 23 | ||||
| -rw-r--r-- | chrome/browser/browser_process_impl.h | 5 | ||||
| -rw-r--r-- | chrome/browser/metrics/thread_watcher.cc | 206 | ||||
| -rw-r--r-- | chrome/browser/metrics/thread_watcher.h | 75 | ||||
| -rw-r--r-- | chrome/browser/metrics/thread_watcher_unittest.cc | 82 | ||||
| -rw-r--r-- | chrome/chrome_browser.gypi | 2 | ||||
| -rw-r--r-- | chrome/chrome_tests.gypi | 1 | ||||
| -rw-r--r-- | chrome/test/testing_browser_process.cc | 4 | ||||
| -rw-r--r-- | chrome/test/testing_browser_process.h | 3 | ||||
| -rw-r--r-- | tools/valgrind/tsan/suppressions.txt | 21 |
12 files changed, 140 insertions, 301 deletions
diff --git a/chrome/browser/browser_main.cc b/chrome/browser/browser_main.cc index 3e481a2..1622bfa 100644 --- a/chrome/browser/browser_main.cc +++ b/chrome/browser/browser_main.cc @@ -44,7 +44,6 @@ #include "chrome/browser/metrics/histogram_synchronizer.h" #include "chrome/browser/metrics/metrics_log.h" #include "chrome/browser/metrics/metrics_service.h" -#include "chrome/browser/metrics/thread_watcher.h" #include "chrome/browser/net/blob_url_request_job_factory.h" #include "chrome/browser/net/chrome_dns_cert_provenance_checker.h" #include "chrome/browser/net/chrome_dns_cert_provenance_checker_factory.h" @@ -626,9 +625,6 @@ void CreateChildThreads(BrowserProcessImpl* process) { process->process_launcher_thread(); process->cache_thread(); process->io_thread(); - // Create watchdog thread after creating all other threads because it will - // watch the other threads and they must be running. - process->watchdog_thread(); } // Returns the new local state object, guaranteed non-NULL. @@ -1280,11 +1276,6 @@ int BrowserMain(const MainFunctionParams& parameters) { scoped_refptr<HistogramSynchronizer> histogram_synchronizer( new HistogramSynchronizer()); - // Initialize thread watcher system. This is a singleton and is used by - // WatchDogThread to keep track of information about threads that are being - // watched. - scoped_ptr<ThreadWatcherList> thread_watcher_list(new ThreadWatcherList()); - // Initialize the prefs of the local state. browser::RegisterLocalState(local_state); @@ -1318,9 +1309,6 @@ int BrowserMain(const MainFunctionParams& parameters) { CreateChildThreads(browser_process.get()); - // Start watching all browser threads for responsiveness. - ThreadWatcherList::StartWatchingAll(); - #if defined(OS_CHROMEOS) // Now that the file thread exists we can record our stats. chromeos::BootTimesLoader::Get()->RecordChromeMainStats(); @@ -1838,9 +1826,6 @@ int BrowserMain(const MainFunctionParams& parameters) { process_singleton.Cleanup(); - // Stop all tasks that might run on WatchDogThread. - ThreadWatcherList::StopWatchingAll(); - metrics->Stop(); // browser_shutdown takes care of deleting browser_process, so we need to diff --git a/chrome/browser/browser_process.h b/chrome/browser/browser_process.h index d406eb0..1eb9f03 100644 --- a/chrome/browser/browser_process.h +++ b/chrome/browser/browser_process.h @@ -40,7 +40,6 @@ class ResourceDispatcherHost; class SidebarManager; class TabCloseableStateWatcher; class ThumbnailGenerator; -class WatchDogThread; namespace base { class Thread; @@ -119,9 +118,6 @@ class BrowserProcess { virtual base::Thread* background_x11_thread() = 0; #endif - // Returns the thread that is used for health check of all browser threads. - virtual WatchDogThread* watchdog_thread() = 0; - virtual policy::BrowserPolicyConnector* browser_policy_connector() = 0; virtual IconManager* icon_manager() = 0; diff --git a/chrome/browser/browser_process_impl.cc b/chrome/browser/browser_process_impl.cc index b173c72..3a76341 100644 --- a/chrome/browser/browser_process_impl.cc +++ b/chrome/browser/browser_process_impl.cc @@ -31,7 +31,6 @@ #include "chrome/browser/intranet_redirect_detector.h" #include "chrome/browser/io_thread.h" #include "chrome/browser/metrics/metrics_service.h" -#include "chrome/browser/metrics/thread_watcher.h" #include "chrome/browser/net/chrome_net_log.h" #include "chrome/browser/net/predictor_api.h" #include "chrome/browser/net/sdch_dictionary_fetcher.h" @@ -99,7 +98,6 @@ BrowserProcessImpl::BrowserProcessImpl(const CommandLine& command_line) created_db_thread_(false), created_process_launcher_thread_(false), created_cache_thread_(false), - created_watchdog_thread_(false), created_profile_manager_(false), created_local_state_(false), created_icon_manager_(false), @@ -250,9 +248,6 @@ BrowserProcessImpl::~BrowserProcessImpl() { // on the db thread too. db_thread_.reset(); - // Stop the watchdog thread after stopping other threads. - watchdog_thread_.reset(); - // At this point, no render process exist and the file, io, db, and // webkit threads in this process have all terminated, so it's safe // to access local state data such as cookies, database, or local storage. @@ -398,14 +393,6 @@ base::Thread* BrowserProcessImpl::background_x11_thread() { } #endif -WatchDogThread* BrowserProcessImpl::watchdog_thread() { - DCHECK(CalledOnValidThread()); - if (!created_watchdog_thread_) - CreateWatchdogThread(); - DCHECK(watchdog_thread_.get() != NULL); - return watchdog_thread_.get(); -} - ProfileManager* BrowserProcessImpl::profile_manager() { DCHECK(CalledOnValidThread()); if (!created_profile_manager_) @@ -747,16 +734,6 @@ void BrowserProcessImpl::CreateCacheThread() { cache_thread_.swap(thread); } -void BrowserProcessImpl::CreateWatchdogThread() { - DCHECK(!created_watchdog_thread_ && watchdog_thread_.get() == NULL); - created_watchdog_thread_ = true; - - scoped_ptr<WatchDogThread> thread(new WatchDogThread()); - if (!thread->Start()) - return; - watchdog_thread_.swap(thread); -} - void BrowserProcessImpl::CreateProfileManager() { DCHECK(!created_profile_manager_ && profile_manager_.get() == NULL); created_profile_manager_ = true; diff --git a/chrome/browser/browser_process_impl.h b/chrome/browser/browser_process_impl.h index cccd985..581650a 100644 --- a/chrome/browser/browser_process_impl.h +++ b/chrome/browser/browser_process_impl.h @@ -57,7 +57,6 @@ class BrowserProcessImpl : public BrowserProcess, #if defined(USE_X11) virtual base::Thread* background_x11_thread(); #endif - virtual WatchDogThread* watchdog_thread(); virtual ProfileManager* profile_manager(); virtual PrefService* local_state(); virtual DevToolsManager* devtools_manager(); @@ -125,7 +124,6 @@ class BrowserProcessImpl : public BrowserProcess, void CreateDBThread(); void CreateProcessLauncherThread(); void CreateCacheThread(); - void CreateWatchdogThread(); void CreateTemplateURLModel(); void CreateProfileManager(); void CreateWebDataService(); @@ -173,9 +171,6 @@ class BrowserProcessImpl : public BrowserProcess, bool created_cache_thread_; scoped_ptr<base::Thread> cache_thread_; - bool created_watchdog_thread_; - scoped_ptr<WatchDogThread> watchdog_thread_; - bool created_profile_manager_; scoped_ptr<ProfileManager> profile_manager_; diff --git a/chrome/browser/metrics/thread_watcher.cc b/chrome/browser/metrics/thread_watcher.cc index b1597f7..ececcac 100644 --- a/chrome/browser/metrics/thread_watcher.cc +++ b/chrome/browser/metrics/thread_watcher.cc @@ -2,16 +2,14 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#if 0 + #include "base/threading/thread_restrictions.h" #include "build/build_config.h" #include "chrome/browser/metrics/metrics_service.h" #include "chrome/browser/metrics/thread_watcher.h" #include "chrome/common/notification_service.h" -#if defined(OS_WIN) -#include <Objbase.h> -#endif - // static const int ThreadWatcher::kPingCount = 3; @@ -44,10 +42,11 @@ void ThreadWatcher::StartWatching(const BrowserThread::ID thread_id, DCHECK_GE(sleep_time.InMilliseconds(), 0); DCHECK_GE(unresponsive_time.InMilliseconds(), sleep_time.InMilliseconds()); - // If we are not on WatchDogThread, then post a task to call StartWatching on - // WatchDogThread. - if (!WatchDogThread::CurrentlyOnWatchDogThread()) { - WatchDogThread::PostTask( + // If we are not on WATCHDOG thread, then post a task to call StartWatching on + // WATCHDOG thread. + if (!BrowserThread::CurrentlyOn(BrowserThread::WATCHDOG)) { + BrowserThread::PostTask( + BrowserThread::WATCHDOG, FROM_HERE, NewRunnableFunction( &ThreadWatcher::StartWatching, @@ -55,7 +54,7 @@ void ThreadWatcher::StartWatching(const BrowserThread::ID thread_id, return; } - DCHECK(WatchDogThread::CurrentlyOnWatchDogThread()); + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::WATCHDOG)); // Create a new thread watcher object for the given thread and activate it. ThreadWatcher* watcher = @@ -65,7 +64,7 @@ void ThreadWatcher::StartWatching(const BrowserThread::ID thread_id, } void ThreadWatcher::ActivateThreadWatching() { - DCHECK(WatchDogThread::CurrentlyOnWatchDogThread()); + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::WATCHDOG)); if (active_) return; active_ = true; ping_count_ = kPingCount; @@ -75,14 +74,14 @@ void ThreadWatcher::ActivateThreadWatching() { } void ThreadWatcher::DeActivateThreadWatching() { - DCHECK(WatchDogThread::CurrentlyOnWatchDogThread()); + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::WATCHDOG)); active_ = false; ping_count_ = 0; method_factory_.RevokeAll(); } void ThreadWatcher::WakeUp() { - DCHECK(WatchDogThread::CurrentlyOnWatchDogThread()); + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::WATCHDOG)); // There is some user activity, PostPingMessage task of thread watcher if // needed. if (!active_) return; @@ -96,7 +95,7 @@ void ThreadWatcher::WakeUp() { } void ThreadWatcher::PostPingMessage() { - DCHECK(WatchDogThread::CurrentlyOnWatchDogThread()); + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::WATCHDOG)); // If we have stopped watching or if the user is idle, then stop sending // ping messages. if (!active_ || ping_count_ <= 0) @@ -127,7 +126,7 @@ void ThreadWatcher::PostPingMessage() { } void ThreadWatcher::OnPongMessage(uint64 ping_sequence_number) { - DCHECK(WatchDogThread::CurrentlyOnWatchDogThread()); + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::WATCHDOG)); // Record watched thread's response time. base::TimeDelta response_time = base::TimeTicks::Now() - ping_time_; histogram_->AddTime(response_time); @@ -153,7 +152,7 @@ void ThreadWatcher::OnPongMessage(uint64 ping_sequence_number) { } bool ThreadWatcher::OnCheckResponsiveness(uint64 ping_sequence_number) { - DCHECK(WatchDogThread::CurrentlyOnWatchDogThread()); + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::WATCHDOG)); // If we have stopped watching then consider thread as responding. if (!active_) return true; @@ -179,7 +178,7 @@ void ThreadWatcher::OnPingMessage(const BrowserThread::ID thread_id, Task* callback_task) { // This method is called on watched thread. DCHECK(BrowserThread::CurrentlyOn(thread_id)); - WatchDogThread::PostTask(FROM_HERE, callback_task); + BrowserThread::PostTask(BrowserThread::WATCHDOG, FROM_HERE, callback_task); } //------------------------------------------------------------------------------ @@ -192,15 +191,22 @@ ThreadWatcherList::ThreadWatcherList() : last_wakeup_time_(base::TimeTicks::Now()) { // Assert we are not running on WATCHDOG thread. Would be ideal to assert we // are on UI thread, but Unit tests are not running on UI thread. - DCHECK(!WatchDogThread::CurrentlyOnWatchDogThread()); - CHECK(!global_); + DCHECK(!BrowserThread::CurrentlyOn(BrowserThread::WATCHDOG)); + DCHECK(!global_); global_ = this; // Register Notifications observer. +#if defined(OS_WIN) MetricsService::SetupNotifications(®istrar_, this); +#endif } ThreadWatcherList::~ThreadWatcherList() { base::AutoLock auto_lock(lock_); + while (!registered_.empty()) { + RegistrationList::iterator it = registered_.begin(); + delete it->second; + registered_.erase(it->first); + } DCHECK(this == global_); global_ = NULL; } @@ -214,76 +220,30 @@ void ThreadWatcherList::Register(ThreadWatcher* watcher) { } // static -void ThreadWatcherList::StartWatchingAll() { - if (!WatchDogThread::CurrentlyOnWatchDogThread()) { - WatchDogThread::PostDelayedTask( - FROM_HERE, - NewRunnableFunction(&ThreadWatcherList::StartWatchingAll), - base::TimeDelta::FromSeconds(10).InMilliseconds()); - return; - } - - DCHECK(WatchDogThread::CurrentlyOnWatchDogThread()); - const base::TimeDelta kSleepTime = base::TimeDelta::FromSeconds(5); - const base::TimeDelta kUnresponsiveTime = base::TimeDelta::FromSeconds(10); - if (BrowserThread::IsMessageLoopValid(BrowserThread::UI)) { - ThreadWatcher::StartWatching(BrowserThread::UI, "UI", kSleepTime, - kUnresponsiveTime); - } - if (BrowserThread::IsMessageLoopValid(BrowserThread::IO)) { - ThreadWatcher::StartWatching(BrowserThread::IO, "IO", kSleepTime, - kUnresponsiveTime); - } - if (BrowserThread::IsMessageLoopValid(BrowserThread::DB)) { - ThreadWatcher::StartWatching(BrowserThread::DB, "DB", kSleepTime, - kUnresponsiveTime); - } - if (BrowserThread::IsMessageLoopValid(BrowserThread::FILE)) { - ThreadWatcher::StartWatching(BrowserThread::FILE, "FILE", kSleepTime, - kUnresponsiveTime); - } - if (BrowserThread::IsMessageLoopValid(BrowserThread::CACHE)) { - ThreadWatcher::StartWatching(BrowserThread::CACHE, "CACHE", kSleepTime, - kUnresponsiveTime); - } -} - -// static void ThreadWatcherList::StopWatchingAll() { - // Assert we are not running on WATCHDOG thread. Would be ideal to assert we - // are on UI thread, but Unit tests are not running on UI thread. - DCHECK(!WatchDogThread::CurrentlyOnWatchDogThread()); + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); if (!global_) return; - - // Remove all notifications for all watched threads. - RemoveNotifications(); - - // Delete all thread watcher objects on WatchDogThread. - WatchDogThread::PostTask( - FROM_HERE, - NewRunnableMethod(global_, &ThreadWatcherList::DeleteAll)); + base::AutoLock auto_lock(global_->lock_); + for (RegistrationList::iterator it = global_->registered_.begin(); + global_->registered_.end() != it; + ++it) + BrowserThread::PostTask( + BrowserThread::WATCHDOG, + FROM_HERE, + NewRunnableMethod( + it->second, &ThreadWatcher::DeActivateThreadWatching)); } // static void ThreadWatcherList::RemoveNotifications() { - // Assert we are not running on WATCHDOG thread. Would be ideal to assert we - // are on UI thread, but Unit tests are not running on UI thread. - DCHECK(!WatchDogThread::CurrentlyOnWatchDogThread()); + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); if (!global_) return; +#if defined(OS_WIN) base::AutoLock auto_lock(global_->lock_); global_->registrar_.RemoveAll(); -} - -void ThreadWatcherList::DeleteAll() { - DCHECK(WatchDogThread::CurrentlyOnWatchDogThread()); - base::AutoLock auto_lock(lock_); - while (!registered_.empty()) { - RegistrationList::iterator it = registered_.begin(); - delete it->second; - registered_.erase(it->first); - } +#endif } void ThreadWatcherList::Observe(NotificationType type, @@ -300,15 +260,15 @@ void ThreadWatcherList::Observe(NotificationType type, last_wakeup_time_ = now; } } - if (need_to_awaken) { - WatchDogThread::PostTask( + if (need_to_awaken) + BrowserThread::PostTask( + BrowserThread::WATCHDOG, FROM_HERE, NewRunnableMethod(this, &ThreadWatcherList::WakeUpAll)); - } } void ThreadWatcherList::WakeUpAll() { - DCHECK(WatchDogThread::CurrentlyOnWatchDogThread()); + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::WATCHDOG)); base::AutoLock auto_lock(lock_); for (RegistrationList::iterator it = global_->registered_.begin(); global_->registered_.end() != it; @@ -334,88 +294,42 @@ ThreadWatcher* ThreadWatcherList::PreLockedFind( //------------------------------------------------------------------------------ // WatchDogThread methods and members. -// static -base::Lock WatchDogThread::lock_; -// static -WatchDogThread* WatchDogThread::watchdog_thread_ = NULL; - // The WatchDogThread object must outlive any tasks posted to the IO thread // before the Quit task. DISABLE_RUNNABLE_METHOD_REFCOUNT(WatchDogThread); -WatchDogThread::WatchDogThread() : Thread("WATCHDOG") { +WatchDogThread::WatchDogThread() + : BrowserProcessSubThread(BrowserThread::WATCHDOG) { } WatchDogThread::~WatchDogThread() { + // Remove all notifications for all watched threads. + ThreadWatcherList::RemoveNotifications(); // We cannot rely on our base class to stop the thread since we want our // CleanUp function to run. Stop(); } -// static -bool WatchDogThread::CurrentlyOnWatchDogThread() { - base::AutoLock lock(lock_); - return watchdog_thread_ && - watchdog_thread_->message_loop() == MessageLoop::current(); -} - -// static -bool WatchDogThread::PostTask(const tracked_objects::Location& from_here, - Task* task) { - return PostTaskHelper(from_here, task, 0); -} - -// static -bool WatchDogThread::PostDelayedTask(const tracked_objects::Location& from_here, - Task* task, - int64 delay_ms) { - return PostTaskHelper(from_here, task, delay_ms); -} - -// static -bool WatchDogThread::PostTaskHelper( - const tracked_objects::Location& from_here, - Task* task, - int64 delay_ms) { - { - base::AutoLock lock(lock_); - - MessageLoop* message_loop = watchdog_thread_ ? - watchdog_thread_->message_loop() : NULL; - if (message_loop) { - message_loop->PostDelayedTask(from_here, task, delay_ms); - return true; - } - } - delete task; - - return false; -} - void WatchDogThread::Init() { // This thread shouldn't be allowed to perform any blocking disk I/O. base::ThreadRestrictions::SetIOAllowed(false); -#if defined(OS_WIN) - // Initializes the COM library on the current thread. - HRESULT result = CoInitialize(NULL); - CHECK(result == S_OK); -#endif - - base::AutoLock lock(lock_); - CHECK(!watchdog_thread_); - watchdog_thread_ = this; -} - -void WatchDogThread::CleanUp() { - base::AutoLock lock(lock_); - watchdog_thread_ = NULL; -} + BrowserProcessSubThread::Init(); -void WatchDogThread::CleanUpAfterMessageLoopDestruction() { #if defined(OS_WIN) - // Closes the COM library on the current thread. CoInitialize must - // be balanced by a corresponding call to CoUninitialize. - CoUninitialize(); + const base::TimeDelta kSleepTime = base::TimeDelta::FromSeconds(5); + const base::TimeDelta kUnresponsiveTime = base::TimeDelta::FromSeconds(10); + ThreadWatcher::StartWatching(BrowserThread::UI, "UI", kSleepTime, + kUnresponsiveTime); + ThreadWatcher::StartWatching(BrowserThread::IO, "IO", kSleepTime, + kUnresponsiveTime); + ThreadWatcher::StartWatching(BrowserThread::DB, "DB", kSleepTime, + kUnresponsiveTime); + ThreadWatcher::StartWatching(BrowserThread::FILE, "FILE", kSleepTime, + kUnresponsiveTime); + ThreadWatcher::StartWatching(BrowserThread::CACHE, "CACHE", kSleepTime, + kUnresponsiveTime); #endif } + +#endif // 0 diff --git a/chrome/browser/metrics/thread_watcher.h b/chrome/browser/metrics/thread_watcher.h index 0bd701f..f7765c8 100644 --- a/chrome/browser/metrics/thread_watcher.h +++ b/chrome/browser/metrics/thread_watcher.h @@ -8,13 +8,14 @@ #define CHROME_BROWSER_METRICS_THREAD_WATCHER_H_ #pragma once +#if 0 + #include <map> #include <string> #include <vector> #include "base/basictypes.h" #include "base/gtest_prod_util.h" -#include "base/message_loop.h" #include "base/metrics/histogram.h" #include "base/ref_counted.h" #include "base/scoped_ptr.h" @@ -22,6 +23,7 @@ #include "base/task.h" #include "base/threading/thread.h" #include "base/time.h" +#include "chrome/browser/browser_process_sub_thread.h" #include "chrome/browser/browser_thread.h" #include "chrome/common/notification_observer.h" #include "chrome/common/notification_registrar.h" @@ -79,12 +81,6 @@ class ThreadWatcher { // Returns true if we are montioring the thread. bool active() const { return active_; } - // Returns ping_time_ (used by unit tests). - base::TimeTicks ping_time() const { return ping_time_; } - - // Returns ping_sequence_number_ (used by unit tests). - uint64 ping_sequence_number() const { return ping_sequence_number_; } - protected: // Construct a ThreadWatcher for the given thread_id. sleep_time_ is the // wait time between ping messages. unresponsive_time_ is the wait time after @@ -111,21 +107,21 @@ class ThreadWatcher { // OnPongMessage. It also posts a task (OnCheckResponsiveness) to check // responsiveness of monitored thread that would be called after waiting // unresponsive_time_. - // This method is accessible on WatchDogThread. + // This method is accessible on WATCHDOG thread. virtual void PostPingMessage(); // This method handles a Pong Message from watched thread. It will track the // response time (pong time minus ping time) via histograms. It posts a // PostPingMessage task that would be called after waiting sleep_time_. It // increments ping_sequence_number_ by 1. - // This method is accessible on WatchDogThread. + // This method is accessible on WATCHDOG thread. virtual void OnPongMessage(uint64 ping_sequence_number); // This method will determine if the watched thread is responsive or not. If // the latest ping_sequence_number_ is not same as the ping_sequence_number // that is passed in, then we can assume that watched thread has responded // with a pong message. - // This method is accessible on WatchDogThread. + // This method is accessible on WATCHDOG thread. virtual bool OnCheckResponsiveness(uint64 ping_sequence_number); private: @@ -192,9 +188,8 @@ class ThreadWatcher { //------------------------------------------------------------------------------ // Class with a list of all active thread watchers. A thread watcher is active -// if it has been registered, which includes determing the histogram name. This -// class provides utility functions to start and stop watching all browser -// threads. Only one instance of this class exists. +// if it has been registered, which includes determing the histogram name. +// Only one instance of this class exists. class ThreadWatcherList : public NotificationObserver { public: // A map from BrowserThread to the actual instances. @@ -203,46 +198,33 @@ class ThreadWatcherList : public NotificationObserver { // This singleton holds the global list of registered ThreadWatchers. ThreadWatcherList(); // Destructor deletes all registered ThreadWatcher instances. - virtual ~ThreadWatcherList(); + ~ThreadWatcherList(); // Register() stores a pointer to the given ThreadWatcher in a global map. static void Register(ThreadWatcher* watcher); - // This method posts a task on WatchDogThread to start watching all browser - // threads. - // This method is accessible on UI thread. - static void StartWatchingAll(); - - // This method posts a task on WatchDogThread to RevokeAll tasks and to + // This method posts a task on WATCHDOG thread to RevokeAll tasks and to // deactive thread watching of other threads and tell NotificationService to // stop calling Observe. - // This method is accessible on UI thread. static void StopWatchingAll(); // RemoveAll NotificationTypes that are being observed. - // This method is accessible on UI thread. static void RemoveNotifications(); private: // Allow tests to access our innards for testing purposes. FRIEND_TEST(ThreadWatcherTest, Registration); - // Delete all thread watcher objects and remove them from global map. - // This method is accessible on WatchDogThread. - void DeleteAll(); - // This will ensure that the watching is actively taking place. It will wakeup // all thread watchers every 2 seconds. This is the implementation of // NotificationObserver. When a matching notification is posted to the // notification service, this method is called. - // This method is accessible on UI thread. virtual void Observe(NotificationType type, const NotificationSource& source, const NotificationDetails& details); // This will ensure that the watching is actively taking place, and awaken // all thread watchers that are registered. - // This method is accessible on WatchDogThread. virtual void WakeUpAll(); // The Find() method can be used to test to see if a given ThreadWatcher was @@ -268,50 +250,23 @@ class ThreadWatcherList : public NotificationObserver { }; //------------------------------------------------------------------------------ -// Class for WatchDogThread and in its Init method, we start watching UI, IO, +// Class for WATCHDOG thread and in its Init method, we start watching UI, IO, // DB, FILE, CACHED threads. -class WatchDogThread : public base::Thread { +class WatchDogThread : public BrowserProcessSubThread { public: - // Constructor. WatchDogThread(); - - // Destroys the thread and stops the thread. virtual ~WatchDogThread(); - // Callable on any thread. Returns whether you're currently on a - // watchdog_thread_. - static bool CurrentlyOnWatchDogThread(); - - // These are the same methods in message_loop.h, but are guaranteed to either - // get posted to the MessageLoop if it's still alive, or be deleted otherwise. - // They return true iff the watchdog thread existed and the task was posted. - // Note that even if the task is posted, there's no guarantee that it will - // run, since the target thread may already have a Quit message in its queue. - static bool PostTask(const tracked_objects::Location& from_here, Task* task); - static bool PostDelayedTask(const tracked_objects::Location& from_here, - Task* task, - int64 delay_ms); - protected: virtual void Init(); - virtual void CleanUp(); - virtual void CleanUpAfterMessageLoopDestruction(); - - private: - static bool PostTaskHelper( - const tracked_objects::Location& from_here, - Task* task, - int64 delay_ms); - - // This lock protects watchdog_thread_. - static base::Lock lock_; - - static WatchDogThread* watchdog_thread_; // The singleton of this class. DISALLOW_COPY_AND_ASSIGN(WatchDogThread); }; + DISABLE_RUNNABLE_METHOD_REFCOUNT(ThreadWatcher); DISABLE_RUNNABLE_METHOD_REFCOUNT(ThreadWatcherList); +#endif // 0 + #endif // CHROME_BROWSER_METRICS_THREAD_WATCHER_H_ diff --git a/chrome/browser/metrics/thread_watcher_unittest.cc b/chrome/browser/metrics/thread_watcher_unittest.cc index efb978b..52a4288 100644 --- a/chrome/browser/metrics/thread_watcher_unittest.cc +++ b/chrome/browser/metrics/thread_watcher_unittest.cc @@ -2,6 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#if 0 + #include "base/basictypes.h" #include "base/logging.h" #include "base/message_loop.h" @@ -11,7 +13,6 @@ #include "base/synchronization/lock.h" #include "base/threading/platform_thread.h" #include "base/time.h" -#include "build/build_config.h" #include "chrome/browser/metrics/thread_watcher.h" #include "testing/gtest/include/gtest/gtest.h" #include "testing/platform_test.h" @@ -51,8 +52,6 @@ class CustomThreadWatcher : public ThreadWatcher { uint64 pong_received_; uint64 success_response_; uint64 failed_response_; - base::TimeTicks saved_ping_time_; - uint64 saved_ping_sequence_number_; CustomThreadWatcher(const BrowserThread::ID thread_id, const std::string thread_name, @@ -66,9 +65,7 @@ class CustomThreadWatcher : public ThreadWatcher { ping_sent_(0), pong_received_(0), success_response_(0), - failed_response_(0), - saved_ping_time_(base::TimeTicks::Now()), - saved_ping_sequence_number_(0) { + failed_response_(0) { } State UpdateState(State new_state) { @@ -78,12 +75,6 @@ class CustomThreadWatcher : public ThreadWatcher { old_state = thread_watcher_state_; if (old_state != DEACTIVATED) thread_watcher_state_ = new_state; - if (new_state == SENT_PING) - ++ping_sent_; - if (new_state == RECEIVED_PONG) - ++pong_received_; - saved_ping_time_ = ping_time(); - saved_ping_sequence_number_ = ping_sequence_number(); } state_changed_.Broadcast(); // LOG(INFO) << "UpdateState: thread_name_: " << thread_name_ << @@ -121,9 +112,11 @@ class CustomThreadWatcher : public ThreadWatcher { State old_state = UpdateState(SENT_PING); EXPECT_TRUE(old_state == ACTIVATED || old_state == RECEIVED_PONG); ThreadWatcher::PostPingMessage(); + ++ping_sent_; } void OnPongMessage(uint64 ping_sequence_number) { + ++pong_received_; State old_state = UpdateState(RECEIVED_PONG); EXPECT_TRUE(old_state == SENT_PING || old_state == DEACTIVATED); ThreadWatcher::OnPongMessage(ping_sequence_number); @@ -153,7 +146,7 @@ class CustomThreadWatcher : public ThreadWatcher { } void VeryLongMethod(TimeDelta wait_time) { - DCHECK(!WatchDogThread::CurrentlyOnWatchDogThread()); + DCHECK(!BrowserThread::CurrentlyOn(BrowserThread::WATCHDOG)); TimeTicks end_time = TimeTicks::Now() + wait_time; { base::AutoLock auto_lock(custom_lock_); @@ -168,7 +161,7 @@ class CustomThreadWatcher : public ThreadWatcher { } State WaitForStateChange(const TimeDelta& wait_time, State expected_state) { - DCHECK(!WatchDogThread::CurrentlyOnWatchDogThread()); + DCHECK(!BrowserThread::CurrentlyOn(BrowserThread::WATCHDOG)); UpdateWaitState(STARTED_WAITING); State exit_state; @@ -203,7 +196,7 @@ class CustomThreadWatcher : public ThreadWatcher { CheckResponseState WaitForCheckResponse(const TimeDelta& wait_time, CheckResponseState expected_state) { - DCHECK(!WatchDogThread::CurrentlyOnWatchDogThread()); + DCHECK(!BrowserThread::CurrentlyOn(BrowserThread::WATCHDOG)); UpdateWaitState(STARTED_WAITING); CheckResponseState exit_state; @@ -252,9 +245,13 @@ class ThreadWatcherTest : public ::testing::Test { CustomThreadWatcher* webkit_watcher_; ThreadWatcherTest() { + } + + protected: + virtual void SetUp() { webkit_thread_.reset(new BrowserThread(BrowserThread::WEBKIT)); io_thread_.reset(new BrowserThread(BrowserThread::IO)); - watchdog_thread_.reset(new WatchDogThread()); + watchdog_thread_.reset(new BrowserThread(BrowserThread::WATCHDOG)); webkit_thread_->Start(); io_thread_->Start(); watchdog_thread_->Start(); @@ -271,10 +268,7 @@ class ThreadWatcherTest : public ::testing::Test { webkit_thread_id, webkit_thread_name, kSleepTime, kUnresponsiveTime); } - ~ThreadWatcherTest() { - ThreadWatcherList::StopWatchingAll(); - io_watcher_ = NULL; - webkit_watcher_ = NULL; + virtual void TearDown() { // io_thread_->Stop(); // webkit_thread_->Stop(); // watchdog_thread_->Stop(); @@ -287,7 +281,7 @@ class ThreadWatcherTest : public ::testing::Test { private: scoped_ptr<BrowserThread> webkit_thread_; scoped_ptr<BrowserThread> io_thread_; - scoped_ptr<WatchDogThread> watchdog_thread_; + scoped_ptr<BrowserThread> watchdog_thread_; ThreadWatcherList* thread_watcher_list_; }; @@ -325,13 +319,14 @@ TEST_F(ThreadWatcherTest, Registration) { // Test ActivateThreadWatching and DeActivateThreadWatching of IO thread. This // method also checks that pong message was sent by the watched thread and pong -// message was received by the WatchDogThread. It also checks that +// message was received by the WATCHDOG thread. It also checks that // OnCheckResponsiveness has verified the ping-pong mechanism and the watched // thread is not hung. TEST_F(ThreadWatcherTest, ThreadResponding) { TimeTicks time_before_ping = TimeTicks::Now(); // Activate watching IO thread. - WatchDogThread::PostTask( + BrowserThread::PostTask( + BrowserThread::WATCHDOG, FROM_HERE, NewRunnableMethod(io_watcher_, &ThreadWatcher::ActivateThreadWatching)); @@ -344,8 +339,8 @@ TEST_F(ThreadWatcherTest, ThreadResponding) { EXPECT_GT(io_watcher_->ping_sent_, static_cast<uint64>(0)); EXPECT_GT(io_watcher_->pong_received_, static_cast<uint64>(0)); EXPECT_TRUE(io_watcher_->active()); - EXPECT_GE(io_watcher_->saved_ping_time_, time_before_ping); - EXPECT_GE(io_watcher_->saved_ping_sequence_number_, static_cast<uint64>(0)); + EXPECT_GE(io_watcher_->ping_time_, time_before_ping); + EXPECT_GE(io_watcher_->ping_sequence_number_, static_cast<uint64>(0)); // Verify watched thread is responding with ping/pong messaging. io_watcher_->WaitForCheckResponse( @@ -354,7 +349,8 @@ TEST_F(ThreadWatcherTest, ThreadResponding) { EXPECT_EQ(io_watcher_->failed_response_, static_cast<uint64>(0)); // DeActivate thread watching for shutdown. - WatchDogThread::PostTask( + BrowserThread::PostTask( + BrowserThread::WATCHDOG, FROM_HERE, NewRunnableMethod(io_watcher_, &ThreadWatcher::DeActivateThreadWatching)); } @@ -375,7 +371,8 @@ TEST_F(ThreadWatcherTest, ThreadNotResponding) { kUnresponsiveTime * 10)); // Activate thread watching. - WatchDogThread::PostTask( + BrowserThread::PostTask( + BrowserThread::WATCHDOG, FROM_HERE, NewRunnableMethod(io_watcher_, &ThreadWatcher::ActivateThreadWatching)); @@ -386,7 +383,8 @@ TEST_F(ThreadWatcherTest, ThreadNotResponding) { EXPECT_GT(io_watcher_->failed_response_, static_cast<uint64>(0)); // DeActivate thread watching for shutdown. - WatchDogThread::PostTask( + BrowserThread::PostTask( + BrowserThread::WATCHDOG, FROM_HERE, NewRunnableMethod(io_watcher_, &ThreadWatcher::DeActivateThreadWatching)); } @@ -394,13 +392,14 @@ TEST_F(ThreadWatcherTest, ThreadNotResponding) { // Test watching of multiple threads with all threads not responding. TEST_F(ThreadWatcherTest, MultipleThreadsResponding) { // Check for WEBKIT thread to perform ping/pong messaging. - WatchDogThread::PostTask( + BrowserThread::PostTask( + BrowserThread::WATCHDOG, FROM_HERE, NewRunnableMethod( webkit_watcher_, &ThreadWatcher::ActivateThreadWatching)); - // Check for IO thread to perform ping/pong messaging. - WatchDogThread::PostTask( + BrowserThread::PostTask( + BrowserThread::WATCHDOG, FROM_HERE, NewRunnableMethod(io_watcher_, &ThreadWatcher::ActivateThreadWatching)); @@ -423,11 +422,12 @@ TEST_F(ThreadWatcherTest, MultipleThreadsResponding) { EXPECT_EQ(io_watcher_->failed_response_, static_cast<uint64>(0)); // DeActivate thread watching for shutdown. - WatchDogThread::PostTask( + BrowserThread::PostTask( + BrowserThread::WATCHDOG, FROM_HERE, NewRunnableMethod(io_watcher_, &ThreadWatcher::DeActivateThreadWatching)); - - WatchDogThread::PostTask( + BrowserThread::PostTask( + BrowserThread::WATCHDOG, FROM_HERE, NewRunnableMethod( webkit_watcher_, &ThreadWatcher::DeActivateThreadWatching)); @@ -446,13 +446,15 @@ TEST_F(ThreadWatcherTest, MultipleThreadsNotResponding) { kUnresponsiveTime * 10)); // Activate watching of WEBKIT thread. - WatchDogThread::PostTask( + BrowserThread::PostTask( + BrowserThread::WATCHDOG, FROM_HERE, NewRunnableMethod( webkit_watcher_, &ThreadWatcher::ActivateThreadWatching)); // Activate watching of IO thread. - WatchDogThread::PostTask( + BrowserThread::PostTask( + BrowserThread::WATCHDOG, FROM_HERE, NewRunnableMethod(io_watcher_, &ThreadWatcher::ActivateThreadWatching)); @@ -469,11 +471,15 @@ TEST_F(ThreadWatcherTest, MultipleThreadsNotResponding) { EXPECT_GT(io_watcher_->failed_response_, static_cast<uint64>(0)); // DeActivate thread watching for shutdown. - WatchDogThread::PostTask( + BrowserThread::PostTask( + BrowserThread::WATCHDOG, FROM_HERE, NewRunnableMethod(io_watcher_, &ThreadWatcher::DeActivateThreadWatching)); - WatchDogThread::PostTask( + BrowserThread::PostTask( + BrowserThread::WATCHDOG, FROM_HERE, NewRunnableMethod( webkit_watcher_, &ThreadWatcher::DeActivateThreadWatching)); } + +#endif // 0 diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi index 570d442..db88990 100644 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -1297,8 +1297,6 @@ 'browser/metrics/metrics_response.h', 'browser/metrics/metrics_service.cc', 'browser/metrics/metrics_service.h', - 'browser/metrics/thread_watcher.cc', - 'browser/metrics/thread_watcher.h', 'browser/metrics/user_metrics.cc', 'browser/metrics/user_metrics.h', 'browser/nacl_host/nacl_broker_host_win.cc', diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi index ebb1ba7..9a7ef72 100644 --- a/chrome/chrome_tests.gypi +++ b/chrome/chrome_tests.gypi @@ -1354,7 +1354,6 @@ 'browser/metrics/metrics_log_unittest.cc', 'browser/metrics/metrics_response_unittest.cc', 'browser/metrics/metrics_service_unittest.cc', - 'browser/metrics/thread_watcher_unittest.cc', 'browser/mock_plugin_exceptions_table_model.cc', 'browser/mock_plugin_exceptions_table_model.h', 'browser/net/connection_tester_unittest.cc', diff --git a/chrome/test/testing_browser_process.cc b/chrome/test/testing_browser_process.cc index 9a4e72f..adb2b10 100644 --- a/chrome/test/testing_browser_process.cc +++ b/chrome/test/testing_browser_process.cc @@ -58,10 +58,6 @@ base::Thread* TestingBrowserProcess::cache_thread() { return NULL; } -WatchDogThread* TestingBrowserProcess::watchdog_thread() { - return NULL; -} - ProfileManager* TestingBrowserProcess::profile_manager() { return profile_manager_.get(); } diff --git a/chrome/test/testing_browser_process.h b/chrome/test/testing_browser_process.h index 4c4d7ef..f302458 100644 --- a/chrome/test/testing_browser_process.h +++ b/chrome/test/testing_browser_process.h @@ -22,7 +22,6 @@ class IOThread; class GoogleURLTracker; class PrefService; -class WatchDogThread; namespace base { class WaitableEvent; @@ -59,8 +58,6 @@ class TestingBrowserProcess : public BrowserProcess { virtual base::Thread* cache_thread(); - virtual WatchDogThread* watchdog_thread(); - virtual ProfileManager* profile_manager(); virtual PrefService* local_state(); diff --git a/tools/valgrind/tsan/suppressions.txt b/tools/valgrind/tsan/suppressions.txt index a5a327f..ef42765 100644 --- a/tools/valgrind/tsan/suppressions.txt +++ b/tools/valgrind/tsan/suppressions.txt @@ -152,6 +152,13 @@ fun:base::RefCounted<ProtocolHandlerRegistry>::* } +{ + bug_73975 + ThreadSanitizer:Race + ... + fun:base::WeakPtrFactory<ThreadWatcher>::* +} + ############################ # Benign races { @@ -376,6 +383,20 @@ fun:base::ThreadCollisionWarner::Leave } +{ + bug_74029a + ThreadSanitzier:Race + ... + fun:CustomThreadWatcher::OnPongMessage* +} + +{ + bug_74029b + ThreadSanitzier:Race + ... + fun:ThreadWatcherTest_ThreadResponding_Test::TestBody* +} + ############################ # Benign races in ICU { |