diff --git a/webrtc/base/task_queue.h b/webrtc/base/task_queue.h index dad4f431b7..380dea7088 100644 --- a/webrtc/base/task_queue.h +++ b/webrtc/base/task_queue.h @@ -155,7 +155,7 @@ static std::unique_ptr NewClosure(const Closure& closure, // TaskQueue itself has been deleted or it may happen synchronously while the // TaskQueue instance is being deleted. This may vary from one OS to the next // so assumptions about lifetimes of pending tasks should not be made. -class TaskQueue { +class LOCKABLE TaskQueue { public: explicit TaskQueue(const char* queue_name); // TODO(tommi): Implement move semantics? diff --git a/webrtc/base/thread.h b/webrtc/base/thread.h index 1a0c447264..1afe9d5ebe 100644 --- a/webrtc/base/thread.h +++ b/webrtc/base/thread.h @@ -93,7 +93,7 @@ class Runnable { // WARNING! SUBCLASSES MUST CALL Stop() IN THEIR DESTRUCTORS! See ~Thread(). -class Thread : public MessageQueue { +class LOCKABLE Thread : public MessageQueue { public: // Create a new Thread and optionally assign it to the passed SocketServer. Thread(); diff --git a/webrtc/base/thread_annotations.h b/webrtc/base/thread_annotations.h index 612242d611..d49feb7da6 100644 --- a/webrtc/base/thread_annotations.h +++ b/webrtc/base/thread_annotations.h @@ -32,6 +32,9 @@ // is primarily used when the client cannot express the name of the lock. #define GUARDED_BY(x) THREAD_ANNOTATION_ATTRIBUTE__(guarded_by(x)) #define GUARDED_VAR THREAD_ANNOTATION_ATTRIBUTE__(guarded) +// Document if a variable/field is not shared and should be accessed from +// same thread/task queue. +#define ACCESS_ON(x) THREAD_ANNOTATION_ATTRIBUTE__(guarded_by(x)) // Document if the memory location pointed to by a pointer should be guarded // by a lock when dereferencing the pointer. Similar to GUARDED_VAR, @@ -58,6 +61,8 @@ // Document if a function expects certain locks to be held before it is called #define EXCLUSIVE_LOCKS_REQUIRED(...) \ THREAD_ANNOTATION_ATTRIBUTE__(exclusive_locks_required(__VA_ARGS__)) +// Document if a function expected to be called from same thread/task queue. +#define RUN_ON(x) THREAD_ANNOTATION_ATTRIBUTE__(exclusive_locks_required(x)) #define SHARED_LOCKS_REQUIRED(...) \ THREAD_ANNOTATION_ATTRIBUTE__(shared_locks_required(__VA_ARGS__)) diff --git a/webrtc/base/thread_checker.h b/webrtc/base/thread_checker.h index 6cd7d7b9e0..265b2affcb 100644 --- a/webrtc/base/thread_checker.h +++ b/webrtc/base/thread_checker.h @@ -28,6 +28,9 @@ #define ENABLE_THREAD_CHECKER 0 #endif +#include "webrtc/base/checks.h" +#include "webrtc/base/constructormagic.h" +#include "webrtc/base/thread_annotations.h" #include "webrtc/base/thread_checker_impl.h" namespace rtc { @@ -77,15 +80,101 @@ class ThreadCheckerDoNothing { // // In Release mode, CalledOnValidThread will always return true. #if ENABLE_THREAD_CHECKER -class ThreadChecker : public ThreadCheckerImpl { +class LOCKABLE ThreadChecker : public ThreadCheckerImpl { }; #else -class ThreadChecker : public ThreadCheckerDoNothing { +class LOCKABLE ThreadChecker : public ThreadCheckerDoNothing { }; #endif // ENABLE_THREAD_CHECKER #undef ENABLE_THREAD_CHECKER +namespace internal { +class SCOPED_LOCKABLE AnnounceOnThread { + public: + template + explicit AnnounceOnThread(const ThreadLikeObject* thread_like_object) + EXCLUSIVE_LOCK_FUNCTION(thread_like_object) {} + ~AnnounceOnThread() UNLOCK_FUNCTION() {} + + template + static bool IsCurrent(const ThreadLikeObject* thread_like_object) { + return thread_like_object->IsCurrent(); + } + static bool IsCurrent(const rtc::ThreadChecker* checker) { + return checker->CalledOnValidThread(); + } + + private: + RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(AnnounceOnThread); +}; + +} // namespace internal } // namespace rtc +// RUN_ON/ACCESS_ON/RTC_DCHECK_RUN_ON macros allows to annotate variables are +// accessed from same thread/task queue. +// Using tools designed to check mutexes, it checks at compile time everywhere +// variable is access, there is a run-time dcheck thread/task queue is correct. +// +// class ExampleThread { +// public: +// void NeedVar1() { +// RTC_DCHECK_RUN_ON(network_thread_); +// transport_->Send(); +// } +// +// private: +// rtc::Thread* network_thread_; +// int transport_ ACCESS_ON(network_thread_); +// }; +// +// class ExampleThreadChecker { +// public: +// int CalledFromPacer() RUN_ON(pacer_thread_checker_) { +// return var2_; +// } +// +// void CallMeFromPacer() { +// RTC_DCHECK_RUN_ON(&pacer_thread_checker_) +// << "Should be called from pacer"; +// CalledFromPacer(); +// } +// +// private: +// int pacer_var_ ACCESS_ON(pacer_thread_checker_); +// rtc::ThreadChecker pacer_thread_checker_; +// }; +// +// class TaskQueueExample { +// public: +// class Encoder { +// public: +// rtc::TaskQueue* Queue() { return encoder_queue_; } +// void Encode() { +// RTC_DCHECK_RUN_ON(encoder_queue_); +// DoSomething(var_); +// } +// +// private: +// rtc::TaskQueue* const encoder_queue_; +// Frame var_ ACCESS_ON(encoder_queue_); +// }; +// +// void Encode() { +// // Will fail at runtime when DCHECK is enabled: +// // encoder_->Encode(); +// // Will work: +// rtc::scoped_ref_ptr encoder = encoder_; +// encoder_->Queue()->PostTask([encoder] { encoder->Encode(); }); +// } +// +// private: +// rtc::scoped_ref_ptr encoder_; +// } + +#define RTC_DCHECK_RUN_ON(thread_like_object) \ + rtc::internal::AnnounceOnThread thread_announcer(thread_like_object); \ + RTC_DCHECK(rtc::internal::AnnounceOnThread::IsCurrent(thread_like_object)) + #endif // WEBRTC_BASE_THREAD_CHECKER_H_ diff --git a/webrtc/base/thread_checker_unittest.cc b/webrtc/base/thread_checker_unittest.cc index 372f6f4a77..0a855a1a94 100644 --- a/webrtc/base/thread_checker_unittest.cc +++ b/webrtc/base/thread_checker_unittest.cc @@ -15,6 +15,7 @@ #include "testing/gtest/include/gtest/gtest.h" #include "webrtc/base/checks.h" #include "webrtc/base/constructormagic.h" +#include "webrtc/base/task_queue.h" #include "webrtc/base/thread.h" #include "webrtc/base/thread_checker.h" @@ -194,6 +195,61 @@ TEST(ThreadCheckerTest, DetachFromThreadInRelease) { #endif // GTEST_HAS_DEATH_TEST || !ENABLE_THREAD_CHECKER +class ThreadAnnotateTest { + public: + // Next two function should create warnings when compile (e.g. if used with + // specific T). + // TODO(danilchap): Find a way to test they do not compile when thread + // annotation checks enabled. + template + void access_var_no_annotate() { + var_thread_ = 42; + } + + template + void access_fun_no_annotate() { + function(); + } + + // Functions below should be able to compile. + void access_var_annotate_thread() { + RTC_DCHECK_RUN_ON(thread_); + var_thread_ = 42; + } + + void access_var_annotate_checker() { + RTC_DCHECK_RUN_ON(&checker_); + var_checker_ = 44; + } + + void access_var_annotate_queue() { + RTC_DCHECK_RUN_ON(queue_); + var_queue_ = 46; + } + + void access_fun_annotate() { + RTC_DCHECK_RUN_ON(thread_); + function(); + } + + void access_fun_and_var() { + RTC_DCHECK_RUN_ON(thread_); + fun_acccess_var(); + } + + private: + void function() RUN_ON(thread_) {} + void fun_acccess_var() RUN_ON(thread_) { var_thread_ = 13; } + + rtc::Thread* thread_; + rtc::ThreadChecker checker_; + rtc::TaskQueue* queue_; + + int var_thread_ ACCESS_ON(thread_); + int var_checker_ GUARDED_BY(checker_); + int var_queue_ ACCESS_ON(queue_); +}; + // Just in case we ever get lumped together with other compilation units. #undef ENABLE_THREAD_CHECKER