diff options
Diffstat (limited to 'base')
| -rw-r--r-- | base/base.gypi | 6 | ||||
| -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 |
10 files changed, 256 insertions, 160 deletions
diff --git a/base/base.gypi b/base/base.gypi index 508c4a4..ef1d8d8 100644 --- a/base/base.gypi +++ b/base/base.gypi @@ -251,8 +251,9 @@ 'task_queue.cc', 'task_queue.h', 'template_util.h', - 'threading/non_thread_safe.cc', 'threading/non_thread_safe.h', + 'threading/non_thread_safe_impl.cc', + 'threading/non_thread_safe_impl.h', 'threading/platform_thread.h', 'threading/platform_thread_mac.mm', 'threading/platform_thread_posix.cc', @@ -261,8 +262,9 @@ 'threading/simple_thread.h', 'threading/thread.cc', 'threading/thread.h', - 'threading/thread_checker.cc', 'threading/thread_checker.h', + 'threading/thread_checker_impl.cc', + 'threading/thread_checker_impl.h', 'threading/thread_collision_warner.cc', 'threading/thread_collision_warner.h', 'threading/thread_local.h', 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 |