diff --git a/webrtc/base/base.gyp b/webrtc/base/base.gyp index 94b9637b35..5cb1fd9bfe 100644 --- a/webrtc/base/base.gyp +++ b/webrtc/base/base.gyp @@ -268,6 +268,9 @@ 'testclient.h', 'thread.cc', 'thread.h', + 'thread_checker.h', + 'thread_checker_impl.cc', + 'thread_checker_impl.h', 'timeutils.cc', 'timeutils.h', 'timing.cc', diff --git a/webrtc/base/base_tests.gyp b/webrtc/base/base_tests.gyp index b5ab23f041..c5cc7b809a 100644 --- a/webrtc/base/base_tests.gyp +++ b/webrtc/base/base_tests.gyp @@ -96,6 +96,7 @@ # 'systeminfo_unittest.cc', 'task_unittest.cc', 'testclient_unittest.cc', + 'thread_checker_unittest.cc', 'thread_unittest.cc', 'timeutils_unittest.cc', 'urlencode_unittest.cc', diff --git a/webrtc/base/thread_checker.h b/webrtc/base/thread_checker.h new file mode 100644 index 0000000000..eee9315533 --- /dev/null +++ b/webrtc/base/thread_checker.h @@ -0,0 +1,91 @@ +/* + * Copyright (c) 2014 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +// Borrowed from Chromium's src/base/threading/thread_checker.h. + +#ifndef WEBRTC_BASE_THREAD_CHECKER_H_ +#define WEBRTC_BASE_THREAD_CHECKER_H_ + +// Apart from debug builds, we also enable the thread checker in +// builds with DCHECK_ALWAYS_ON so that trybots and waterfall bots +// with this define will get the same level of thread checking as +// debug bots. +// +// Note that this does not perfectly match situations where DCHECK is +// enabled. For example a non-official release build may have +// DCHECK_ALWAYS_ON undefined (and therefore ThreadChecker would be +// disabled) but have DCHECKs enabled at runtime. +#if (!defined(NDEBUG) || defined(DCHECK_ALWAYS_ON)) +#define ENABLE_THREAD_CHECKER 1 +#else +#define ENABLE_THREAD_CHECKER 0 +#endif + +#include "webrtc/base/thread_checker_impl.h" + +namespace rtc { + +// 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() {} +}; + +// ThreadChecker is a helper class used to help verify that some methods of a +// class are called from the same thread. It provides identical functionality to +// base::NonThreadSafe, but it is meant to be held as a member variable, rather +// than inherited from base::NonThreadSafe. +// +// While inheriting from base::NonThreadSafe may give a clear indication about +// the thread-safety of a class, it may also lead to violations of the style +// guide with regard to multiple inheritance. The choice between having a +// ThreadChecker member and inheriting from base::NonThreadSafe should be based +// on whether: +// - Derived classes need to know the thread they belong to, as opposed to +// having that functionality fully encapsulated in the base class. +// - Derived classes should be able to reassign the base class to another +// thread, via DetachFromThread. +// +// If neither of these are true, then having a ThreadChecker member and calling +// CalledOnValidThread is the preferable solution. +// +// Example: +// class MyClass { +// public: +// void Foo() { +// DCHECK(thread_checker_.CalledOnValidThread()); +// ... (do stuff) ... +// } +// +// private: +// ThreadChecker thread_checker_; +// } +// +// In Release mode, CalledOnValidThread will always return true. +#if ENABLE_THREAD_CHECKER +class ThreadChecker : public ThreadCheckerImpl { +}; +#else +class ThreadChecker : public ThreadCheckerDoNothing { +}; +#endif // ENABLE_THREAD_CHECKER + +#undef ENABLE_THREAD_CHECKER + +} // namespace rtc + +#endif // WEBRTC_BASE_THREAD_CHECKER_H_ diff --git a/webrtc/base/thread_checker_impl.cc b/webrtc/base/thread_checker_impl.cc new file mode 100644 index 0000000000..4a7455d307 --- /dev/null +++ b/webrtc/base/thread_checker_impl.cc @@ -0,0 +1,44 @@ +/* + * Copyright (c) 2014 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +// Borrowed from Chromium's src/base/threading/thread_checker_impl.cc. + +#include "webrtc/base/thread_checker_impl.h" + +#include "webrtc/base/thread.h" + +namespace rtc { + +ThreadCheckerImpl::ThreadCheckerImpl() + : valid_thread_() { + EnsureThreadIdAssigned(); +} + +ThreadCheckerImpl::~ThreadCheckerImpl() { +} + +bool ThreadCheckerImpl::CalledOnValidThread() const { + CritScope scoped_lock(&lock_); + EnsureThreadIdAssigned(); + return valid_thread_->IsCurrent(); +} + +void ThreadCheckerImpl::DetachFromThread() { + CritScope scoped_lock(&lock_); + valid_thread_ = NULL; +} + +void ThreadCheckerImpl::EnsureThreadIdAssigned() const { + if (!valid_thread_) { + valid_thread_ = Thread::Current(); + } +} + +} // namespace rtc diff --git a/webrtc/base/thread_checker_impl.h b/webrtc/base/thread_checker_impl.h new file mode 100644 index 0000000000..1d776b5ebc --- /dev/null +++ b/webrtc/base/thread_checker_impl.h @@ -0,0 +1,51 @@ +/* + * Copyright (c) 2014 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +// Borrowed from Chromium's src/base/threading/thread_checker_impl.h. + +#ifndef WEBRTC_BASE_THREAD_CHECKER_IMPL_H_ +#define WEBRTC_BASE_THREAD_CHECKER_IMPL_H_ + +#include "webrtc/base/criticalsection.h" + +namespace rtc { + +class Thread; + +// 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 CriticalSection lock_; + // This is mutable so that CalledOnValidThread can set it. + // It's guarded by |lock_|. + mutable const Thread* valid_thread_; +}; + +} // namespace rtc + +#endif // WEBRTC_BASE_THREAD_CHECKER_IMPL_H_ diff --git a/webrtc/base/thread_checker_unittest.cc b/webrtc/base/thread_checker_unittest.cc new file mode 100644 index 0000000000..13c1da5e20 --- /dev/null +++ b/webrtc/base/thread_checker_unittest.cc @@ -0,0 +1,205 @@ +/* + * Copyright (c) 2014 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +// Borrowed from Chromium's src/base/threading/thread_checker_unittest.cc. + +#include + +#include "testing/gtest/include/gtest/gtest.h" +#include "webrtc/base/thread.h" +#include "webrtc/base/thread_checker.h" +#include "webrtc/base/scoped_ptr.h" + +// Duplicated from base/threading/thread_checker.h so that we can be +// good citizens there and undef the macro. +#if !defined(NDEBUG) || defined(DCHECK_ALWAYS_ON) +#define ENABLE_THREAD_CHECKER 1 +#else +#define ENABLE_THREAD_CHECKER 0 +#endif + +namespace rtc { + +namespace { + +// 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 { + public: + ThreadCheckerClass() {} + + // Verifies that it was called on the same thread as the constructor. + void DoStuff() { + assert(CalledOnValidThread()); + } + + void DetachFromThread() { + ThreadChecker::DetachFromThread(); + } + + static void MethodOnDifferentThreadImpl(); + static void DetachThenCallFromDifferentThreadImpl(); + + private: + DISALLOW_COPY_AND_ASSIGN(ThreadCheckerClass); +}; + +// Calls ThreadCheckerClass::DoStuff on another thread. +class CallDoStuffOnThread : public Thread { + public: + explicit CallDoStuffOnThread(ThreadCheckerClass* thread_checker_class) + : Thread(), + thread_checker_class_(thread_checker_class) { + SetName("call_do_stuff_on_thread", NULL); + } + + virtual void Run() OVERRIDE { + thread_checker_class_->DoStuff(); + } + + // New method. Needed since Thread::Join is protected, and it is called by + // the TEST. + void Join() { + Thread::Join(); + } + + private: + ThreadCheckerClass* thread_checker_class_; + + DISALLOW_COPY_AND_ASSIGN(CallDoStuffOnThread); +}; + +// Deletes ThreadCheckerClass on a different thread. +class DeleteThreadCheckerClassOnThread : public Thread { + public: + explicit DeleteThreadCheckerClassOnThread( + ThreadCheckerClass* thread_checker_class) + : Thread(), + thread_checker_class_(thread_checker_class) { + SetName("delete_thread_checker_class_on_thread", NULL); + } + + virtual void Run() OVERRIDE { + thread_checker_class_.reset(); + } + + // New method. Needed since Thread::Join is protected, and it is called by + // the TEST. + void Join() { + Thread::Join(); + } + + private: + scoped_ptr thread_checker_class_; + + DISALLOW_COPY_AND_ASSIGN(DeleteThreadCheckerClassOnThread); +}; + +} // namespace + +TEST(ThreadCheckerTest, CallsAllowedOnSameThread) { + scoped_ptr thread_checker_class( + new ThreadCheckerClass); + + // Verify that DoStuff doesn't assert. + thread_checker_class->DoStuff(); + + // Verify that the destructor doesn't assert. + thread_checker_class.reset(); +} + +TEST(ThreadCheckerTest, DestructorAllowedOnDifferentThread) { + scoped_ptr thread_checker_class( + new ThreadCheckerClass); + + // Verify that the destructor doesn't assert + // when called on a different thread. + DeleteThreadCheckerClassOnThread delete_on_thread( + thread_checker_class.release()); + + delete_on_thread.Start(); + delete_on_thread.Join(); +} + +TEST(ThreadCheckerTest, DetachFromThread) { + scoped_ptr thread_checker_class( + new ThreadCheckerClass); + + // 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()); + + call_on_thread.Start(); + call_on_thread.Join(); +} + +#if GTEST_HAS_DEATH_TEST || !ENABLE_THREAD_CHECKER + +void ThreadCheckerClass::MethodOnDifferentThreadImpl() { + scoped_ptr thread_checker_class( + new ThreadCheckerClass); + + // 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(); +} + +#if ENABLE_THREAD_CHECKER +TEST(ThreadCheckerDeathTest, MethodNotAllowedOnDifferentThreadInDebug) { + ASSERT_DEATH({ + ThreadCheckerClass::MethodOnDifferentThreadImpl(); + }, ""); +} +#else +TEST(ThreadCheckerTest, MethodAllowedOnDifferentThreadInRelease) { + ThreadCheckerClass::MethodOnDifferentThreadImpl(); +} +#endif // ENABLE_THREAD_CHECKER + +void ThreadCheckerClass::DetachThenCallFromDifferentThreadImpl() { + scoped_ptr 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(); + + // DoStuff should assert in debug builds only after moving to + // another thread. + thread_checker_class->DoStuff(); +} + +#if ENABLE_THREAD_CHECKER +TEST(ThreadCheckerDeathTest, DetachFromThreadInDebug) { + ASSERT_DEATH({ + ThreadCheckerClass::DetachThenCallFromDifferentThreadImpl(); + }, ""); +} +#else +TEST(ThreadCheckerTest, DetachFromThreadInRelease) { + ThreadCheckerClass::DetachThenCallFromDifferentThreadImpl(); +} +#endif // ENABLE_THREAD_CHECKER + +#endif // GTEST_HAS_DEATH_TEST || !ENABLE_THREAD_CHECKER + +// Just in case we ever get lumped together with other compilation units. +#undef ENABLE_THREAD_CHECKER + +} // namespace rtc