From 04cd466bd5a05702f1f760fe9a2f4d81b2baed26 Mon Sep 17 00:00:00 2001 From: "tommi@webrtc.org" Date: Mon, 26 Jan 2015 15:27:29 +0000 Subject: [PATCH] Move ThreadChecker into rtc_base_approved. To do this, I'm removing ThreadChecker's dependency on the 'Thread' class, so that the checker works with any thread and doesn't rely on TLS. Also simplifying CriticalSection's implementation on Windows since a critical section on Windows already knows what thread currently owns the lock. BUG= R=mflodman@webrtc.org Review URL: https://webrtc-codereview.appspot.com/40539004 git-svn-id: http://webrtc.googlecode.com/svn/trunk@8151 4adac7df-926f-26a2-2b94-8c16560cd09d --- webrtc/base/BUILD.gn | 6 ++-- webrtc/base/base.gyp | 6 ++-- webrtc/base/criticalsection.h | 35 ++++++++++-------------- webrtc/base/thread_checker_impl.cc | 44 +++++++++++++++++++++--------- webrtc/base/thread_checker_impl.h | 16 ++++++++--- 5 files changed, 63 insertions(+), 44 deletions(-) diff --git a/webrtc/base/BUILD.gn b/webrtc/base/BUILD.gn index b51234770..54fcdca97 100644 --- a/webrtc/base/BUILD.gn +++ b/webrtc/base/BUILD.gn @@ -121,6 +121,9 @@ static_library("rtc_base_approved") { "stringutils.h", "template_util.h", "thread_annotations.h", + "thread_checker.h", + "thread_checker_impl.cc", + "thread_checker_impl.h", "timeutils.cc", "timeutils.h", ] @@ -283,9 +286,6 @@ static_library("rtc_base") { "taskrunner.h", "thread.cc", "thread.h", - "thread_checker.h", - "thread_checker_impl.cc", - "thread_checker_impl.h", "timing.cc", "timing.h", "urlencode.cc", diff --git a/webrtc/base/base.gyp b/webrtc/base/base.gyp index a5efdf622..c837b8789 100644 --- a/webrtc/base/base.gyp +++ b/webrtc/base/base.gyp @@ -56,6 +56,9 @@ 'stringutils.h', 'template_util.h', 'thread_annotations.h', + 'thread_checker.h', + 'thread_checker_impl.cc', + 'thread_checker_impl.h', 'timeutils.cc', 'timeutils.h', ], @@ -281,9 +284,6 @@ 'testclient.h', 'thread.cc', 'thread.h', - 'thread_checker.h', - 'thread_checker_impl.cc', - 'thread_checker_impl.h', 'timing.cc', 'timing.h', 'transformadapter.cc', diff --git a/webrtc/base/criticalsection.h b/webrtc/base/criticalsection.h index a2d9bca0e..1c582b89c 100644 --- a/webrtc/base/criticalsection.h +++ b/webrtc/base/criticalsection.h @@ -37,39 +37,27 @@ namespace rtc { #if defined(WEBRTC_WIN) class LOCKABLE CriticalSection { public: - CriticalSection() { - InitializeCriticalSection(&crit_); - // Windows docs say 0 is not a valid thread id - TRACK_OWNER(thread_ = 0); - } - ~CriticalSection() { - DeleteCriticalSection(&crit_); - } + CriticalSection() { InitializeCriticalSection(&crit_); } + ~CriticalSection() { DeleteCriticalSection(&crit_); } void Enter() EXCLUSIVE_LOCK_FUNCTION() { EnterCriticalSection(&crit_); - TRACK_OWNER(thread_ = GetCurrentThreadId()); } bool TryEnter() EXCLUSIVE_TRYLOCK_FUNCTION(true) { - if (TryEnterCriticalSection(&crit_) != FALSE) { - TRACK_OWNER(thread_ = GetCurrentThreadId()); - return true; - } - return false; + return TryEnterCriticalSection(&crit_) != FALSE; } void Leave() UNLOCK_FUNCTION() { - TRACK_OWNER(thread_ = 0); LeaveCriticalSection(&crit_); } -#if CS_TRACK_OWNER - bool CurrentThreadIsOwner() const { return thread_ == GetCurrentThreadId(); } -#endif // CS_TRACK_OWNER + // Used for debugging. + bool CurrentThreadIsOwner() const { + return crit_.OwningThread == reinterpret_cast(GetCurrentThreadId()); + } private: CRITICAL_SECTION crit_; - TRACK_OWNER(DWORD thread_); // The section's owning thread id }; -#endif // WEBRTC_WIN +#endif // WEBRTC_WIN #if defined(WEBRTC_POSIX) class LOCKABLE CriticalSection { @@ -101,9 +89,14 @@ class LOCKABLE CriticalSection { pthread_mutex_unlock(&mutex_); } + // Used for debugging. + bool CurrentThreadIsOwner() const { #if CS_TRACK_OWNER - bool CurrentThreadIsOwner() const { return pthread_equal(thread_, pthread_self()); } + return pthread_equal(thread_, pthread_self()); +#else + return true; #endif // CS_TRACK_OWNER + } private: pthread_mutex_t mutex_; diff --git a/webrtc/base/thread_checker_impl.cc b/webrtc/base/thread_checker_impl.cc index 4a7455d30..48478e713 100644 --- a/webrtc/base/thread_checker_impl.cc +++ b/webrtc/base/thread_checker_impl.cc @@ -12,13 +12,35 @@ #include "webrtc/base/thread_checker_impl.h" -#include "webrtc/base/thread.h" +#include "webrtc/base/checks.h" + +#if defined(WEBRTC_LINUX) +#include +#endif namespace rtc { +namespace { +PlatformThreadId CurrentThreadId() { +#if defined(WEBRTC_WIN) + return GetCurrentThreadId(); +#elif defined(WEBRTC_POSIX) + // Pthreads doesn't have the concept of a thread ID, so we have to reach down + // into the kernel. +#if defined(WEBRTC_MAC) + return pthread_mach_thread_np(pthread_self()); +#elif defined(WEBRTC_LINUX) + return syscall(__NR_gettid); +#elif defined(WEBRTC_ANDROID) + return gettid(); +#else + // Default implementation for nacl and solaris. + return reinterpret_cast(pthread_self()); +#endif +#endif // defined(WEBRTC_POSIX) +} +} // namespace -ThreadCheckerImpl::ThreadCheckerImpl() - : valid_thread_() { - EnsureThreadIdAssigned(); +ThreadCheckerImpl::ThreadCheckerImpl() : valid_thread_(CurrentThreadId()) { } ThreadCheckerImpl::~ThreadCheckerImpl() { @@ -26,19 +48,15 @@ ThreadCheckerImpl::~ThreadCheckerImpl() { bool ThreadCheckerImpl::CalledOnValidThread() const { CritScope scoped_lock(&lock_); - EnsureThreadIdAssigned(); - return valid_thread_->IsCurrent(); + const PlatformThreadId current_thread = CurrentThreadId(); + if (!valid_thread_) // Set if previously detached. + valid_thread_ = current_thread; + return valid_thread_ == current_thread; } void ThreadCheckerImpl::DetachFromThread() { CritScope scoped_lock(&lock_); - valid_thread_ = NULL; -} - -void ThreadCheckerImpl::EnsureThreadIdAssigned() const { - if (!valid_thread_) { - valid_thread_ = Thread::Current(); - } + valid_thread_ = 0; } } // namespace rtc diff --git a/webrtc/base/thread_checker_impl.h b/webrtc/base/thread_checker_impl.h index 1d776b5eb..5f851c4d4 100644 --- a/webrtc/base/thread_checker_impl.h +++ b/webrtc/base/thread_checker_impl.h @@ -13,11 +13,21 @@ #ifndef WEBRTC_BASE_THREAD_CHECKER_IMPL_H_ #define WEBRTC_BASE_THREAD_CHECKER_IMPL_H_ +#if defined(WEBRTC_POSIX) +#include +#include +#endif + #include "webrtc/base/criticalsection.h" namespace rtc { -class Thread; +// Used for identifying the current thread. Always an integer value. +#if defined(WEBRTC_WIN) +typedef DWORD PlatformThreadId; +#elif defined(WEBRTC_POSIX) +typedef pid_t PlatformThreadId; +#endif // Real implementation of ThreadChecker, for use in debug mode, or // for temporary use in release mode (e.g. to CHECK on a threading issue @@ -38,12 +48,10 @@ class ThreadCheckerImpl { 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_; + mutable PlatformThreadId valid_thread_; }; } // namespace rtc