diff options
author | joi@chromium.org <joi@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-03-03 23:38:51 +0000 |
---|---|---|
committer | joi@chromium.org <joi@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-03-03 23:38:51 +0000 |
commit | 4006448408b35ca6be5aef5db1be1e966d31eee4 (patch) | |
tree | d5b94f868aed7307b289fe0ab937d49a21a762ef /base/threading | |
parent | bdcf77a58ac680951fc6bf4f02b3b46f172ab01b (diff) | |
download | chromium_src-4006448408b35ca6be5aef5db1be1e966d31eee4.zip chromium_src-4006448408b35ca6be5aef5db1be1e966d31eee4.tar.gz chromium_src-4006448408b35ca6be5aef5db1be1e966d31eee4.tar.bz2 |
Modify ThreadChecker and NonThreadSafe so that their
functionality is available in release builds if explicitly
requested by using their Impl types. The default usage remains
that they do nothing in release mode.
Also, update unit tests to run in release mode, verifying that the release versions of NonThreadSafe and ThreadChecker do nothing in release builds.
BUG=none
TEST=base unit tests
Review URL: http://codereview.chromium.org/6599004
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@76833 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'base/threading')
-rw-r--r-- | base/threading/non_thread_safe.cc | 28 | ||||
-rw-r--r-- | base/threading/non_thread_safe.h | 51 | ||||
-rw-r--r-- | base/threading/non_thread_safe_impl.cc | 23 | ||||
-rw-r--r-- | base/threading/non_thread_safe_impl.h | 39 | ||||
-rw-r--r-- | base/threading/non_thread_safe_unittest.cc | 69 | ||||
-rw-r--r-- | base/threading/thread_checker.h | 60 | ||||
-rw-r--r-- | base/threading/thread_checker_impl.cc (renamed from base/threading/thread_checker.cc) | 20 | ||||
-rw-r--r-- | base/threading/thread_checker_impl.h | 43 | ||||
-rw-r--r-- | base/threading/thread_checker_unittest.cc | 77 |
9 files changed, 252 insertions, 158 deletions
diff --git a/base/threading/non_thread_safe.cc b/base/threading/non_thread_safe.cc deleted file mode 100644 index 8b41bc0..0000000 --- a/base/threading/non_thread_safe.cc +++ /dev/null @@ -1,28 +0,0 @@ -// Copyright (c) 2010 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 "base/threading/non_thread_safe.h" - -// These checks are only done in debug builds. -#ifndef NDEBUG - -#include "base/logging.h" - -namespace base { - -NonThreadSafe::~NonThreadSafe() { - DCHECK(CalledOnValidThread()); -} - -bool NonThreadSafe::CalledOnValidThread() const { - return thread_checker_.CalledOnValidThread(); -} - -void NonThreadSafe::DetachFromThread() { - thread_checker_.DetachFromThread(); -} - -} // namespace base - -#endif // NDEBUG diff --git a/base/threading/non_thread_safe.h b/base/threading/non_thread_safe.h index 868a031..bc3be3c 100644 --- a/base/threading/non_thread_safe.h +++ b/base/threading/non_thread_safe.h @@ -6,13 +6,29 @@ #define BASE_THREADING_NON_THREAD_SAFE_H_ #pragma once -#include "base/threading/thread_checker.h" +#ifndef NDEBUG +#include "base/threading/non_thread_safe_impl.h" +#endif namespace base { -// A helper class used to help verify that methods of a class are -// called from the same thread. One can inherit from this class and use -// CalledOnValidThread() to verify. +// Do nothing implementation of NonThreadSafe, for release mode. +// +// Note: You should almost always use the NonThreadSafe class to get +// the right version of the class for your build configuration. +class NonThreadSafeDoNothing { + public: + bool CalledOnValidThread() const { + return true; + } + + protected: + void DetachFromThread() {} +}; + +// NonThreadSafe is a helper class used to help verify that methods of a +// class are called from the same thread. One can inherit from this class +// and use CalledOnValidThread() to verify. // // This is intended to be used with classes that appear to be thread safe, but // aren't. For example, a service or a singleton like the preferences system. @@ -29,33 +45,10 @@ namespace base { // In Release mode, CalledOnValidThread will always return true. // #ifndef NDEBUG -class NonThreadSafe { - public: - ~NonThreadSafe(); - - bool CalledOnValidThread() const; - - protected: - // Changes the thread that is checked for in CalledOnValidThread. The next - // call to CalledOnValidThread will attach this class to a new thread. It is - // up to the NonThreadSafe derived class to decide to expose this or not. - // This may be useful when an object may be created on one thread and then - // used exclusively on another thread. - void DetachFromThread(); - - private: - ThreadChecker thread_checker_; +class NonThreadSafe : public NonThreadSafeImpl { }; #else -// Do nothing in release mode. -class NonThreadSafe { - public: - bool CalledOnValidThread() const { - return true; - } - - protected: - void DetachFromThread() {} +class NonThreadSafe : public NonThreadSafeDoNothing { }; #endif // NDEBUG diff --git a/base/threading/non_thread_safe_impl.cc b/base/threading/non_thread_safe_impl.cc new file mode 100644 index 0000000..f9c18c2 --- /dev/null +++ b/base/threading/non_thread_safe_impl.cc @@ -0,0 +1,23 @@ +// Copyright (c) 2011 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 "base/threading/non_thread_safe_impl.h" + +#include "base/logging.h" + +namespace base { + +NonThreadSafeImpl::~NonThreadSafeImpl() { + DCHECK(CalledOnValidThread()); +} + +bool NonThreadSafeImpl::CalledOnValidThread() const { + return thread_checker_.CalledOnValidThread(); +} + +void NonThreadSafeImpl::DetachFromThread() { + thread_checker_.DetachFromThread(); +} + +} // namespace base diff --git a/base/threading/non_thread_safe_impl.h b/base/threading/non_thread_safe_impl.h new file mode 100644 index 0000000..12925f1 --- /dev/null +++ b/base/threading/non_thread_safe_impl.h @@ -0,0 +1,39 @@ +// Copyright (c) 2011 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. + +#ifndef BASE_THREADING_NON_THREAD_SAFE_IMPL_H_ +#define BASE_THREADING_NON_THREAD_SAFE_IMPL_H_ +#pragma once + +#include "base/threading/thread_checker_impl.h" + +namespace base { + +// Full implementation of NonThreadSafe, for debug mode or for occasional +// temporary use in release mode e.g. when you need to CHECK on a thread +// bug that only occurs in the wild. +// +// Note: You should almost always use the NonThreadSafe class to get +// the right version of the class for your build configuration. +class NonThreadSafeImpl { + public: + ~NonThreadSafeImpl(); + + bool CalledOnValidThread() const; + + protected: + // Changes the thread that is checked for in CalledOnValidThread. The next + // call to CalledOnValidThread will attach this class to a new thread. It is + // up to the NonThreadSafe derived class to decide to expose this or not. + // This may be useful when an object may be created on one thread and then + // used exclusively on another thread. + void DetachFromThread(); + + private: + ThreadCheckerImpl thread_checker_; +}; + +} // namespace base + +#endif // BASE_THREADING_NON_THREAD_SAFE_IMPL_H_ diff --git a/base/threading/non_thread_safe_unittest.cc b/base/threading/non_thread_safe_unittest.cc index 7d158cf..b79a4f4 100644 --- a/base/threading/non_thread_safe_unittest.cc +++ b/base/threading/non_thread_safe_unittest.cc @@ -9,8 +9,6 @@ #include "base/threading/simple_thread.h" #include "testing/gtest/include/gtest/gtest.h" -#ifndef NDEBUG - namespace base { // Simple class to exersice the basics of NonThreadSafe. @@ -22,13 +20,16 @@ class NonThreadSafeClass : public NonThreadSafe { // Verifies that it was called on the same thread as the constructor. void DoStuff() { - DCHECK(CalledOnValidThread()); + CHECK(CalledOnValidThread()); } void DetachFromThread() { NonThreadSafe::DetachFromThread(); } + static void MethodOnDifferentThreadImpl(); + static void DestructorOnDifferentThreadImpl(); + private: DISALLOW_COPY_AND_ASSIGN(NonThreadSafeClass); }; @@ -94,37 +95,57 @@ TEST(NonThreadSafeTest, DetachThenDestructOnDifferentThread) { delete_on_thread.Join(); } -#if GTEST_HAS_DEATH_TEST +#if GTEST_HAS_DEATH_TEST || NDEBUG -TEST(NonThreadSafeDeathTest, MethodNotAllowedOnDifferentThread) { - ASSERT_DEBUG_DEATH({ - scoped_ptr<NonThreadSafeClass> non_thread_safe_class( - new NonThreadSafeClass); +void NonThreadSafeClass::MethodOnDifferentThreadImpl() { + scoped_ptr<NonThreadSafeClass> non_thread_safe_class( + new NonThreadSafeClass); - // Verify that DoStuff asserts when called on a different thread. - CallDoStuffOnThread call_on_thread(non_thread_safe_class.get()); + // Verify that DoStuff asserts in debug builds only when called + // on a different thread. + CallDoStuffOnThread call_on_thread(non_thread_safe_class.get()); - call_on_thread.Start(); - call_on_thread.Join(); - }, ""); + call_on_thread.Start(); + call_on_thread.Join(); } -TEST(NonThreadSafeDeathTest, DestructorNotAllowedOnDifferentThread) { +#ifndef NDEBUG +TEST(NonThreadSafeDeathTest, MethodNotAllowedOnDifferentThreadInDebug) { ASSERT_DEBUG_DEATH({ - scoped_ptr<NonThreadSafeClass> non_thread_safe_class( - new NonThreadSafeClass); + NonThreadSafeClass::MethodOnDifferentThreadImpl(); + }, ""); +} +#else +TEST(NonThreadSafeTest, MethodAllowedOnDifferentThreadInRelease) { + NonThreadSafeClass::MethodOnDifferentThreadImpl(); +} +#endif // NDEBUG - // Verify that the destructor asserts when called on a different thread. - DeleteNonThreadSafeClassOnThread delete_on_thread( - non_thread_safe_class.release()); +void NonThreadSafeClass::DestructorOnDifferentThreadImpl() { + scoped_ptr<NonThreadSafeClass> non_thread_safe_class( + new NonThreadSafeClass); - delete_on_thread.Start(); - delete_on_thread.Join(); + // Verify that the destructor asserts in debug builds only + // when called on a different thread. + DeleteNonThreadSafeClassOnThread delete_on_thread( + non_thread_safe_class.release()); + + delete_on_thread.Start(); + delete_on_thread.Join(); +} + +#ifndef NDEBUG +TEST(NonThreadSafeDeathTest, DestructorNotAllowedOnDifferentThreadInDebug) { + ASSERT_DEBUG_DEATH({ + NonThreadSafeClass::DestructorOnDifferentThreadImpl(); }, ""); } +#else +TEST(NonThreadSafeTest, DestructorAllowedOnDifferentThreadInRelease) { + NonThreadSafeClass::DestructorOnDifferentThreadImpl(); +} +#endif // NDEBUG -#endif // GTEST_HAS_DEATH_TEST +#endif // GTEST_HAS_DEATH_TEST || NDEBUG } // namespace base - -#endif // NDEBUG diff --git a/base/threading/thread_checker.h b/base/threading/thread_checker.h index 712b5b5..33b6764e 100644 --- a/base/threading/thread_checker.h +++ b/base/threading/thread_checker.h @@ -7,22 +7,34 @@ #pragma once #ifndef NDEBUG -#include "base/synchronization/lock.h" -#include "base/threading/platform_thread.h" -#endif // NDEBUG +#include "base/threading/thread_checker_impl.h" +#endif namespace base { +// Do nothing implementation, for use in release mode. +// +// Note: You should almost always use the ThreadChecker class to get the +// right version for your build configuration. +class ThreadCheckerDoNothing { + public: + bool CalledOnValidThread() const { + return true; + } + + void DetachFromThread() {} +}; + // Before using this class, please consider using NonThreadSafe as it // makes it much easier to determine the nature of your class. // -// A helper class used to help verify that some methods of a class are -// called from the same thread. One can inherit from this class and use -// CalledOnValidThread() to verify. +// ThreadChecker is a helper class used to help verify that some methods of a +// class are called from the same thread. One can inherit from this class and +// use CalledOnValidThread() to verify. // -// Inheriting from class indicates that one must be careful when using the class -// with multiple threads. However, it is up to the class document to indicate -// how it can be used with threads. +// Inheriting from class indicates that one must be careful when using the +// class with multiple threads. However, it is up to the class document to +// indicate how it can be used with threads. // // Example: // class MyClass : public ThreadChecker { @@ -34,37 +46,11 @@ namespace base { // } // // In Release mode, CalledOnValidThread will always return true. -// #ifndef NDEBUG -class ThreadChecker { - public: - ThreadChecker(); - ~ThreadChecker(); - - bool CalledOnValidThread() const; - - // Changes the thread that is checked for in CalledOnValidThread. This may - // be useful when an object may be created on one thread and then used - // exclusively on another thread. - void DetachFromThread(); - - private: - void EnsureThreadIdAssigned() const; - - mutable base::Lock lock_; - // This is mutable so that CalledOnValidThread can set it. - // It's guarded by |lock_|. - mutable PlatformThreadId valid_thread_id_; +class ThreadChecker : public ThreadCheckerImpl { }; #else -// Do nothing in release mode. -class ThreadChecker { - public: - bool CalledOnValidThread() const { - return true; - } - - void DetachFromThread() {} +class ThreadChecker : public ThreadCheckerDoNothing { }; #endif // NDEBUG diff --git a/base/threading/thread_checker.cc b/base/threading/thread_checker_impl.cc index 28ba400..985433e 100644 --- a/base/threading/thread_checker.cc +++ b/base/threading/thread_checker_impl.cc @@ -1,32 +1,30 @@ -// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 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 "base/threading/thread_checker.h" - -// This code is only done in debug builds. -#ifndef NDEBUG +#include "base/threading/thread_checker_impl.h" namespace base { -ThreadChecker::ThreadChecker() : valid_thread_id_(kInvalidThreadId) { +ThreadCheckerImpl::ThreadCheckerImpl() + : valid_thread_id_(kInvalidThreadId) { EnsureThreadIdAssigned(); } -ThreadChecker::~ThreadChecker() {} +ThreadCheckerImpl::~ThreadCheckerImpl() {} -bool ThreadChecker::CalledOnValidThread() const { +bool ThreadCheckerImpl::CalledOnValidThread() const { EnsureThreadIdAssigned(); AutoLock auto_lock(lock_); return valid_thread_id_ == PlatformThread::CurrentId(); } -void ThreadChecker::DetachFromThread() { +void ThreadCheckerImpl::DetachFromThread() { AutoLock auto_lock(lock_); valid_thread_id_ = kInvalidThreadId; } -void ThreadChecker::EnsureThreadIdAssigned() const { +void ThreadCheckerImpl::EnsureThreadIdAssigned() const { AutoLock auto_lock(lock_); if (valid_thread_id_ != kInvalidThreadId) return; @@ -34,5 +32,3 @@ void ThreadChecker::EnsureThreadIdAssigned() const { } } // namespace base - -#endif // NDEBUG diff --git a/base/threading/thread_checker_impl.h b/base/threading/thread_checker_impl.h new file mode 100644 index 0000000..6d41fdc --- /dev/null +++ b/base/threading/thread_checker_impl.h @@ -0,0 +1,43 @@ +// Copyright (c) 2011 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. + +#ifndef BASE_THREADING_THREAD_CHECKER_IMPL_H_ +#define BASE_THREADING_THREAD_CHECKER_IMPL_H_ +#pragma once + +#include "base/synchronization/lock.h" +#include "base/threading/platform_thread.h" + +namespace base { + +// Real implementation of ThreadChecker, for use in debug mode, or +// for temporary use in release mode (e.g. to CHECK on a threading issue +// seen only in the wild). +// +// Note: You should almost always use the ThreadChecker class to get the +// right version for your build configuration. +class ThreadCheckerImpl { + public: + ThreadCheckerImpl(); + ~ThreadCheckerImpl(); + + bool CalledOnValidThread() const; + + // Changes the thread that is checked for in CalledOnValidThread. This may + // be useful when an object may be created on one thread and then used + // exclusively on another thread. + void DetachFromThread(); + + private: + void EnsureThreadIdAssigned() const; + + mutable base::Lock lock_; + // This is mutable so that CalledOnValidThread can set it. + // It's guarded by |lock_|. + mutable PlatformThreadId valid_thread_id_; +}; + +} // namespace base + +#endif // BASE_THREADING_THREAD_CHECKER_IMPL_H_ diff --git a/base/threading/thread_checker_unittest.cc b/base/threading/thread_checker_unittest.cc index 6ce5bf1..9a22a8e 100644 --- a/base/threading/thread_checker_unittest.cc +++ b/base/threading/thread_checker_unittest.cc @@ -9,11 +9,9 @@ #include "base/threading/simple_thread.h" #include "testing/gtest/include/gtest/gtest.h" -#ifndef NDEBUG - namespace base { -// Simple class to exersice the basics of ThreadChecker. +// Simple class to exercise the basics of ThreadChecker. // Both the destructor and DoStuff should verify that they were // called on the same thread as the constructor. class ThreadCheckerClass : public ThreadChecker { @@ -22,13 +20,16 @@ class ThreadCheckerClass : public ThreadChecker { // Verifies that it was called on the same thread as the constructor. void DoStuff() { - DCHECK(CalledOnValidThread()); + CHECK(CalledOnValidThread()); } void DetachFromThread() { ThreadChecker::DetachFromThread(); } + static void MethodOnDifferentThreadImpl(); + static void DetachThenCallFromDifferentThreadImpl(); + private: DISALLOW_COPY_AND_ASSIGN(ThreadCheckerClass); }; @@ -106,41 +107,61 @@ TEST(ThreadCheckerTest, DetachFromThread) { call_on_thread.Join(); } -#if GTEST_HAS_DEATH_TEST +#if GTEST_HAS_DEATH_TEST || NDEBUG -TEST(ThreadCheckerDeathTest, MethodNotAllowedOnDifferentThread) { - ASSERT_DEBUG_DEATH({ - scoped_ptr<ThreadCheckerClass> thread_checker_class( - new ThreadCheckerClass); +void ThreadCheckerClass::MethodOnDifferentThreadImpl() { + scoped_ptr<ThreadCheckerClass> thread_checker_class( + new ThreadCheckerClass); - // Verify that DoStuff asserts when called on a different thread. - CallDoStuffOnThread call_on_thread(thread_checker_class.get()); + // DoStuff should assert in debug builds only when called on a + // different thread. + CallDoStuffOnThread call_on_thread(thread_checker_class.get()); - call_on_thread.Start(); - call_on_thread.Join(); - }, ""); + call_on_thread.Start(); + call_on_thread.Join(); } -TEST(ThreadCheckerDeathTest, DetachFromThread) { +#ifndef NDEBUG +TEST(ThreadCheckerDeathTest, MethodNotAllowedOnDifferentThreadInDebug) { ASSERT_DEBUG_DEATH({ - scoped_ptr<ThreadCheckerClass> thread_checker_class( - new ThreadCheckerClass); + ThreadCheckerClass::MethodOnDifferentThreadImpl(); + }, ""); +} +#else +TEST(ThreadCheckerTest, MethodAllowedOnDifferentThreadInRelease) { + ThreadCheckerClass::MethodOnDifferentThreadImpl(); +} +#endif // NDEBUG - // Verify that DoStuff doesn't assert when called on a different thread - // after a call to DetachFromThread. - thread_checker_class->DetachFromThread(); - CallDoStuffOnThread call_on_thread(thread_checker_class.get()); +void ThreadCheckerClass::DetachThenCallFromDifferentThreadImpl() { + scoped_ptr<ThreadCheckerClass> thread_checker_class( + new ThreadCheckerClass); + + // DoStuff doesn't assert when called on a different thread + // after a call to DetachFromThread. + thread_checker_class->DetachFromThread(); + CallDoStuffOnThread call_on_thread(thread_checker_class.get()); - call_on_thread.Start(); - call_on_thread.Join(); + call_on_thread.Start(); + call_on_thread.Join(); - // Verify that DoStuff asserts after moving to another thread. - thread_checker_class->DoStuff(); + // DoStuff should assert in debug builds only after moving to + // another thread. + thread_checker_class->DoStuff(); +} + +#ifndef NDEBUG +TEST(ThreadCheckerDeathTest, DetachFromThreadInDebug) { + ASSERT_DEBUG_DEATH({ + ThreadCheckerClass::DetachThenCallFromDifferentThreadImpl(); }, ""); } +#else +TEST(ThreadCheckerTest, DetachFromThreadInRelease) { + ThreadCheckerClass::DetachThenCallFromDifferentThreadImpl(); +} +#endif // NDEBUG -#endif // GTEST_HAS_DEATH_TEST +#endif // GTEST_HAS_DEATH_TEST || NDEBUG } // namespace base - -#endif // NDEBUG |