diff --git a/webrtc/BUILD.gn b/webrtc/BUILD.gn index f151f108e7..f31eae11f6 100644 --- a/webrtc/BUILD.gn +++ b/webrtc/BUILD.gn @@ -450,6 +450,7 @@ if (rtc_include_tests) { "base/rtccertificate_unittest.cc", "base/rtccertificategenerator_unittest.cc", "base/scopedptrcollection_unittest.cc", + "base/sequenced_task_checker_unittest.cc", "base/sha1digest_unittest.cc", "base/sharedexclusivelock_unittest.cc", "base/signalthread_unittest.cc", diff --git a/webrtc/base/BUILD.gn b/webrtc/base/BUILD.gn index cd4bc1fe43..621a57bf49 100644 --- a/webrtc/base/BUILD.gn +++ b/webrtc/base/BUILD.gn @@ -192,6 +192,9 @@ static_library("rtc_task_queue") { configs += [ "..:common_config" ] sources = [ + "sequenced_task_checker.h", + "sequenced_task_checker_impl.cc", + "sequenced_task_checker_impl.h", "task_queue.h", "task_queue_posix.h", ] diff --git a/webrtc/base/base.gyp b/webrtc/base/base.gyp index 93be3bc706..b81ad125eb 100644 --- a/webrtc/base/base.gyp +++ b/webrtc/base/base.gyp @@ -136,6 +136,9 @@ 'rtc_base_approved', ], 'sources': [ + 'sequenced_task_checker.h', + 'sequenced_task_checker_impl.cc', + 'sequenced_task_checker_impl.h', 'task_queue.h', 'task_queue_posix.h', ], diff --git a/webrtc/base/sequenced_task_checker.h b/webrtc/base/sequenced_task_checker.h new file mode 100644 index 0000000000..9fc3b4a964 --- /dev/null +++ b/webrtc/base/sequenced_task_checker.h @@ -0,0 +1,87 @@ +/* + * Copyright (c) 2016 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. + */ + +#ifndef WEBRTC_BASE_SEQUENCED_TASK_CHECKER_H_ +#define WEBRTC_BASE_SEQUENCED_TASK_CHECKER_H_ + +// Apart from debug builds, we also enable the sequence checker in +// builds with DCHECK_ALWAYS_ON so that trybots and waterfall bots +// with this define will get the same level of checking as debug bots. +// +// Note that this does not perfectly match situations where RTC_DCHECK is +// enabled. For example a non-official release build may have +// DCHECK_ALWAYS_ON undefined (and therefore SequencedTaskChecker would be +// disabled) but have RTC_DCHECKs enabled at runtime. +#if (!defined(NDEBUG) || defined(DCHECK_ALWAYS_ON)) +#define ENABLE_SEQUENCED_TASK_CHECKER 1 +#else +#define ENABLE_SEQUENCED_TASK_CHECKER 0 +#endif + +#include "webrtc/base/checks.h" +#include "webrtc/base/constructormagic.h" +#include "webrtc/base/thread_annotations.h" +#include "webrtc/base/sequenced_task_checker_impl.h" + +namespace rtc { + +// Do nothing implementation, for use in release mode. +// +// Note: You should almost always use the SequencedTaskChecker class to get the +// right version for your build configuration. +class SequencedTaskCheckerDoNothing { + public: + bool CalledSequentially() const { return true; } + + void Detach() {} +}; + +// SequencedTaskChecker is a helper class used to help verify that some methods +// of a class are called on the same task queue or thread. A +// SequencedTaskChecker is bound to a a task queue if the object is +// created on a task queue, or a thread otherwise. +// +// +// Example: +// class MyClass { +// public: +// void Foo() { +// RTC_DCHECK(sequence_checker_.CalledSequentially()); +// ... (do stuff) ... +// } +// +// private: +// SequencedTaskChecker sequence_checker_; +// } +// +// In Release mode, CalledOnValidThread will always return true. +#if ENABLE_SEQUENCED_TASK_CHECKER +class LOCKABLE SequencedTaskChecker : public SequencedTaskCheckerImpl {}; +#else +class LOCKABLE SequencedTaskChecker : public SequencedTaskCheckerDoNothing {}; +#endif // ENABLE_SEQUENCED_TASK_CHECKER_H_ + +namespace internal { +class SCOPED_LOCKABLE SequencedTaskCheckerScope { + public: + explicit SequencedTaskCheckerScope(const SequencedTaskChecker* checker) + EXCLUSIVE_LOCK_FUNCTION(checker); + ~SequencedTaskCheckerScope() UNLOCK_FUNCTION(); +}; + +} // namespace internal + +#define RTC_DCHECK_CALLED_SEQUENTIALLY(x) \ + rtc::internal::SequencedTaskCheckerScope checker(x) + +#undef ENABLE_SEQUENCED_TASK_CHECKER + +} // namespace rtc +#endif // WEBRTC_BASE_SEQUENCED_TASK_CHECKER_H_ diff --git a/webrtc/base/sequenced_task_checker_impl.cc b/webrtc/base/sequenced_task_checker_impl.cc new file mode 100644 index 0000000000..971cc65d38 --- /dev/null +++ b/webrtc/base/sequenced_task_checker_impl.cc @@ -0,0 +1,52 @@ +/* + * Copyright (c) 2016 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. + */ + +#include "webrtc/base/sequenced_task_checker_impl.h" + +#include "webrtc/base/platform_thread.h" +#include "webrtc/base/sequenced_task_checker.h" + +namespace rtc { + +SequencedTaskCheckerImpl::SequencedTaskCheckerImpl() + : attached_(true), valid_queue_(TaskQueue::Current()) {} + +SequencedTaskCheckerImpl::~SequencedTaskCheckerImpl() {} + +bool SequencedTaskCheckerImpl::CalledSequentially() const { + TaskQueue* current_queue = TaskQueue::Current(); + CritScope scoped_lock(&lock_); + if (!attached_) { // true if previously detached. + valid_queue_ = current_queue; + attached_ = true; + } + if (!valid_queue_) + return thread_checker_.CalledOnValidThread(); + return valid_queue_ == current_queue; +} + +void SequencedTaskCheckerImpl::Detach() { + CritScope scoped_lock(&lock_); + attached_ = false; + valid_queue_ = nullptr; + thread_checker_.DetachFromThread(); +} + +namespace internal { + +SequencedTaskCheckerScope::SequencedTaskCheckerScope( + const SequencedTaskChecker* checker) { + RTC_DCHECK(checker->CalledSequentially()); +} + +SequencedTaskCheckerScope::~SequencedTaskCheckerScope() {} + +} // namespace internal +} // namespace rtc diff --git a/webrtc/base/sequenced_task_checker_impl.h b/webrtc/base/sequenced_task_checker_impl.h new file mode 100644 index 0000000000..9b76ed6070 --- /dev/null +++ b/webrtc/base/sequenced_task_checker_impl.h @@ -0,0 +1,44 @@ +/* + * Copyright (c) 2016 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. + */ + +#ifndef WEBRTC_BASE_SEQUENCED_TASK_CHECKER_IMPL_H_ +#define WEBRTC_BASE_SEQUENCED_TASK_CHECKER_IMPL_H_ + +#include "webrtc/base/task_queue.h" +#include "webrtc/base/thread_checker.h" + +namespace rtc { + +// Real implementation of SequencedTaskChecker, for use in debug mode, or +// for temporary use in release mode. +// +// Note: You should almost always use the SequencedTaskChecker class to get the +// right version for your build configuration. +class SequencedTaskCheckerImpl { + public: + SequencedTaskCheckerImpl(); + ~SequencedTaskCheckerImpl(); + + bool CalledSequentially() const; + + // Changes the task queue or thread that is checked for in IsCurrent. This + // may be useful when an object may be created on one task queue / thread and + // then used exclusively on another thread. + void Detach(); + + private: + CriticalSection lock_; + ThreadChecker thread_checker_; + mutable bool attached_; + mutable TaskQueue* valid_queue_; +}; + +} // namespace rtc +#endif // WEBRTC_BASE_SEQUENCED_TASK_CHECKER_IMPL_H_ diff --git a/webrtc/base/sequenced_task_checker_unittest.cc b/webrtc/base/sequenced_task_checker_unittest.cc new file mode 100644 index 0000000000..2450bd5175 --- /dev/null +++ b/webrtc/base/sequenced_task_checker_unittest.cc @@ -0,0 +1,273 @@ +/* + * Copyright (c) 2016 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. + */ +#include "testing/gtest/include/gtest/gtest.h" +#include "webrtc/base/checks.h" +#include "webrtc/base/constructormagic.h" +#include "webrtc/base/platform_thread.h" +#include "webrtc/base/sequenced_task_checker.h" +#include "webrtc/base/task_queue.h" + +namespace rtc { + +namespace { +// Calls SequencedTaskChecker::CalledSequentially on another thread. +class CallCalledSequentiallyOnThread { + public: + CallCalledSequentiallyOnThread(bool expect_true, + SequencedTaskChecker* sequenced_task_checker) + : expect_true_(expect_true), + thread_has_run_event_(false, false), + thread_(&Run, this, "call_do_stuff_on_thread"), + sequenced_task_checker_(sequenced_task_checker) { + thread_.Start(); + } + ~CallCalledSequentiallyOnThread() { + EXPECT_TRUE(thread_has_run_event_.Wait(1000)); + thread_.Stop(); + } + + private: + static bool Run(void* obj) { + CallCalledSequentiallyOnThread* call_stuff_on_thread = + static_cast(obj); + EXPECT_EQ( + call_stuff_on_thread->expect_true_, + call_stuff_on_thread->sequenced_task_checker_->CalledSequentially()); + call_stuff_on_thread->thread_has_run_event_.Set(); + return false; + } + + const bool expect_true_; + Event thread_has_run_event_; + PlatformThread thread_; + SequencedTaskChecker* const sequenced_task_checker_; + + RTC_DISALLOW_COPY_AND_ASSIGN(CallCalledSequentiallyOnThread); +}; + +// Deletes SequencedTaskChecker on a different thread. +class DeleteSequencedCheckerOnThread { + public: + explicit DeleteSequencedCheckerOnThread( + std::unique_ptr sequenced_task_checker) + : thread_(&Run, this, "delete_sequenced_task_checker_on_thread"), + thread_has_run_event_(false, false), + sequenced_task_checker_(std::move(sequenced_task_checker)) { + thread_.Start(); + } + + ~DeleteSequencedCheckerOnThread() { + EXPECT_TRUE(thread_has_run_event_.Wait(1000)); + thread_.Stop(); + } + + private: + static bool Run(void* obj) { + DeleteSequencedCheckerOnThread* instance = + static_cast(obj); + instance->sequenced_task_checker_.reset(); + instance->thread_has_run_event_.Set(); + return false; + } + + private: + PlatformThread thread_; + Event thread_has_run_event_; + std::unique_ptr sequenced_task_checker_; + + RTC_DISALLOW_COPY_AND_ASSIGN(DeleteSequencedCheckerOnThread); +}; + +void RunMethodOnDifferentThread(bool expect_true) { + std::unique_ptr sequenced_task_checker( + new SequencedTaskChecker()); + + CallCalledSequentiallyOnThread call_on_thread(expect_true, + sequenced_task_checker.get()); +} + +void RunMethodOnDifferentTaskQueue(bool expect_true) { + std::unique_ptr sequenced_task_checker( + new SequencedTaskChecker()); + + static const char kQueueName[] = "MethodNotAllowedOnDifferentTq"; + TaskQueue queue(kQueueName); + Event done_event(false, false); + queue.PostTask([&sequenced_task_checker, &done_event, expect_true] { + if (expect_true) + EXPECT_TRUE(sequenced_task_checker->CalledSequentially()); + else + EXPECT_FALSE(sequenced_task_checker->CalledSequentially()); + done_event.Set(); + }); + EXPECT_TRUE(done_event.Wait(1000)); +} + +void DetachThenCallFromDifferentTaskQueue(bool expect_true) { + std::unique_ptr sequenced_task_checker( + new SequencedTaskChecker()); + + sequenced_task_checker->Detach(); + + Event done_event(false, false); + TaskQueue queue1("DetachThenCallFromDifferentTaskQueueImpl1"); + queue1.PostTask([&sequenced_task_checker, &done_event] { + EXPECT_TRUE(sequenced_task_checker->CalledSequentially()); + done_event.Set(); + }); + EXPECT_TRUE(done_event.Wait(1000)); + + // CalledSequentially should return false in debug builds after moving to + // another task queue. + TaskQueue queue2("DetachThenCallFromDifferentTaskQueueImpl2"); + queue2.PostTask([&sequenced_task_checker, &done_event, expect_true] { + if (expect_true) + EXPECT_TRUE(sequenced_task_checker->CalledSequentially()); + else + EXPECT_FALSE(sequenced_task_checker->CalledSequentially()); + done_event.Set(); + }); + EXPECT_TRUE(done_event.Wait(1000)); +} +} // namespace + +TEST(SequencedTaskCheckerTest, CallsAllowedOnSameThread) { + std::unique_ptr sequenced_task_checker( + new SequencedTaskChecker()); + + EXPECT_TRUE(sequenced_task_checker->CalledSequentially()); + + // Verify that the destructor doesn't assert. + sequenced_task_checker.reset(); +} + +TEST(SequencedTaskCheckerTest, DestructorAllowedOnDifferentThread) { + std::unique_ptr sequenced_task_checker( + new SequencedTaskChecker()); + + // Verify that the destructor doesn't assert when called on a different + // thread. + DeleteSequencedCheckerOnThread delete_on_thread( + std::move(sequenced_task_checker)); +} + +TEST(SequencedTaskCheckerTest, DetachFromThread) { + std::unique_ptr sequenced_task_checker( + new SequencedTaskChecker()); + + sequenced_task_checker->Detach(); + CallCalledSequentiallyOnThread call_on_thread(true, + sequenced_task_checker.get()); +} + +TEST(SequencedTaskCheckerTest, DetachFromThreadAndUseOnTaskQueue) { + std::unique_ptr sequenced_task_checker( + new SequencedTaskChecker()); + + sequenced_task_checker->Detach(); + static const char kQueueName[] = "DetachFromThreadAndUseOnTaskQueue"; + TaskQueue queue(kQueueName); + Event done_event(false, false); + queue.PostTask([&sequenced_task_checker, &done_event] { + EXPECT_TRUE(sequenced_task_checker->CalledSequentially()); + done_event.Set(); + }); + EXPECT_TRUE(done_event.Wait(1000)); +} + +TEST(SequencedTaskCheckerTest, DetachFromTaskQueueAndUseOnThread) { + TaskQueue queue("DetachFromTaskQueueAndUseOnThread"); + Event done_event(false, false); + queue.PostTask([&done_event] { + std::unique_ptr sequenced_task_checker( + new SequencedTaskChecker()); + + sequenced_task_checker->Detach(); + CallCalledSequentiallyOnThread call_on_thread(true, + sequenced_task_checker.get()); + done_event.Set(); + }); + EXPECT_TRUE(done_event.Wait(1000)); +} + +#if !NDEBUG || DCHECK_ALWAYS_ON +TEST(SequencedTaskCheckerTest, MethodNotAllowedOnDifferentThreadInDebug) { + RunMethodOnDifferentThread(false); +} +#else +TEST(SequencedTaskCheckerTest, MethodAllowedOnDifferentThreadInRelease) { + RunMethodOnDifferentThread(true); +} +#endif + +#if !NDEBUG || DCHECK_ALWAYS_ON +TEST(SequencedTaskCheckerTest, MethodNotAllowedOnDifferentTaskQueueInDebug) { + RunMethodOnDifferentTaskQueue(false); +} +#else +TEST(SequencedTaskCheckerTest, MethodAllowedOnDifferentTaskQueueInRelease) { + RunMethodOnDifferentTaskQueue(true); +} +#endif + +#if !NDEBUG || DCHECK_ALWAYS_ON +TEST(SequencedTaskCheckerTest, DetachFromTaskQueueInDebug) { + DetachThenCallFromDifferentTaskQueue(false); +} +#else +TEST(SequencedTaskCheckerTest, DetachFromTaskQueueInRelease) { + DetachThenCallFromDifferentTaskQueue(true); +} +#endif + +class TestAnnotations { + public: + TestAnnotations() : test_var_(false) {} + + void ModifyTestVar() { + RTC_DCHECK_CALLED_SEQUENTIALLY(&checker_); + test_var_ = true; + } + + private: + bool test_var_ GUARDED_BY(&checker_); + SequencedTaskChecker checker_; +}; + +TEST(SequencedTaskCheckerTest, TestAnnotations) { + TestAnnotations annotations; + annotations.ModifyTestVar(); +} + +#if GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID) + +void TestAnnotationsOnWrongQueue() { + TestAnnotations annotations; + static const char kQueueName[] = "TestAnnotationsOnWrongQueueDebug"; + TaskQueue queue(kQueueName); + Event done_event(false, false); + queue.PostTask([&annotations, &done_event] { + annotations.ModifyTestVar(); + done_event.Set(); + }); + EXPECT_TRUE(done_event.Wait(1000)); +} + +#if RTC_DCHECK_IS_ON +TEST(SequencedTaskCheckerTest, TestAnnotationsOnWrongQueueDebug) { + ASSERT_DEATH({ TestAnnotationsOnWrongQueue(); }, ""); +} +#else +TEST(SequencedTaskCheckerTest, TestAnnotationsOnWrongQueueRelease) { + TestAnnotationsOnWrongQueue(); +} +#endif +#endif // GTEST_HAS_DEATH_TEST +} // namespace rtc diff --git a/webrtc/webrtc_tests.gypi b/webrtc/webrtc_tests.gypi index 0047f6930c..56ae1cc978 100644 --- a/webrtc/webrtc_tests.gypi +++ b/webrtc/webrtc_tests.gypi @@ -71,6 +71,7 @@ 'base/rtccertificate_unittest.cc', 'base/rtccertificategenerator_unittest.cc', 'base/scopedptrcollection_unittest.cc', + 'base/sequenced_task_checker_unittest.cc', 'base/sha1digest_unittest.cc', 'base/sharedexclusivelock_unittest.cc', 'base/signalthread_unittest.cc',