No compliation-flag-dependent members in CriticalSection Having members in a class which only exist when certain compliation flags are turned on (unless relating to the target platform) means that those flags must be the same when compiling the module as when including its headers from other modules. This means that code outside of WebRTC runs the risk of misjudging the size of an rtc::CriticalSection, or any class which has an rtc::CriticalSection as a member. (This rule is applied recursively.) If a mismatch occurs, memory corruption is likely. Having discussed this a bit, we have decided that the simplest solution is probably the best - always define those members, even when compilation flags (namely, CS_DEBUG_CHECKS) render it unused. BUG=webrtc:7867 Review-Url: https://codereview.webrtc.org/2957753002 Cr-Commit-Position: refs/heads/master@{#18811}
diff --git a/webrtc/base/criticalsection.cc b/webrtc/base/criticalsection.cc index b15bb82..5a7bc76 100644 --- a/webrtc/base/criticalsection.cc +++ b/webrtc/base/criticalsection.cc
@@ -20,41 +20,47 @@ CriticalSection::CriticalSection() { #if defined(WEBRTC_WIN) InitializeCriticalSection(&crit_); -#else -#if defined(WEBRTC_MAC) && !USE_NATIVE_MUTEX_ON_MAC +#elif defined(WEBRTC_POSIX) +# if defined(WEBRTC_MAC) && !USE_NATIVE_MUTEX_ON_MAC lock_queue_ = 0; owning_thread_ = 0; recursion_ = 0; semaphore_ = dispatch_semaphore_create(0); -#else +# else pthread_mutexattr_t mutex_attribute; pthread_mutexattr_init(&mutex_attribute); pthread_mutexattr_settype(&mutex_attribute, PTHREAD_MUTEX_RECURSIVE); pthread_mutex_init(&mutex_, &mutex_attribute); pthread_mutexattr_destroy(&mutex_attribute); -#endif +# endif CS_DEBUG_CODE(thread_ = 0); CS_DEBUG_CODE(recursion_count_ = 0); + RTC_UNUSED(thread_); + RTC_UNUSED(recursion_count_); +#else +# error Unsupported platform. #endif } CriticalSection::~CriticalSection() { #if defined(WEBRTC_WIN) DeleteCriticalSection(&crit_); -#else -#if defined(WEBRTC_MAC) && !USE_NATIVE_MUTEX_ON_MAC +#elif defined(WEBRTC_POSIX) +# if defined(WEBRTC_MAC) && !USE_NATIVE_MUTEX_ON_MAC dispatch_release(semaphore_); -#else +# else pthread_mutex_destroy(&mutex_); -#endif +# endif +#else +# error Unsupported platform. #endif } void CriticalSection::Enter() const EXCLUSIVE_LOCK_FUNCTION() { #if defined(WEBRTC_WIN) EnterCriticalSection(&crit_); -#else -#if defined(WEBRTC_MAC) && !USE_NATIVE_MUTEX_ON_MAC +#elif defined(WEBRTC_POSIX) +# if defined(WEBRTC_MAC) && !USE_NATIVE_MUTEX_ON_MAC int spin = 3000; PlatformThreadRef self = CurrentThreadRef(); bool have_lock = false; @@ -91,11 +97,11 @@ owning_thread_ = self; ++recursion_; -#else +# else pthread_mutex_lock(&mutex_); -#endif +# endif -#if CS_DEBUG_CHECKS +# if CS_DEBUG_CHECKS if (!recursion_count_) { RTC_DCHECK(!thread_); thread_ = CurrentThreadRef(); @@ -103,15 +109,17 @@ RTC_DCHECK(CurrentThreadIsOwner()); } ++recursion_count_; -#endif +# endif +#else +# error Unsupported platform. #endif } bool CriticalSection::TryEnter() const EXCLUSIVE_TRYLOCK_FUNCTION(true) { #if defined(WEBRTC_WIN) return TryEnterCriticalSection(&crit_) != FALSE; -#else -#if defined(WEBRTC_MAC) && !USE_NATIVE_MUTEX_ON_MAC +#elif defined(WEBRTC_POSIX) +# if defined(WEBRTC_MAC) && !USE_NATIVE_MUTEX_ON_MAC if (!IsThreadRefEqual(owning_thread_, CurrentThreadRef())) { if (AtomicOps::CompareAndSwap(&lock_queue_, 0, 1) != 0) return false; @@ -121,11 +129,11 @@ AtomicOps::Increment(&lock_queue_); } ++recursion_; -#else +# else if (pthread_mutex_trylock(&mutex_) != 0) return false; -#endif -#if CS_DEBUG_CHECKS +# endif +# if CS_DEBUG_CHECKS if (!recursion_count_) { RTC_DCHECK(!thread_); thread_ = CurrentThreadRef(); @@ -133,22 +141,25 @@ RTC_DCHECK(CurrentThreadIsOwner()); } ++recursion_count_; -#endif +# endif return true; +#else +# error Unsupported platform. #endif } + void CriticalSection::Leave() const UNLOCK_FUNCTION() { RTC_DCHECK(CurrentThreadIsOwner()); #if defined(WEBRTC_WIN) LeaveCriticalSection(&crit_); -#else -#if CS_DEBUG_CHECKS +#elif defined(WEBRTC_POSIX) +# if CS_DEBUG_CHECKS --recursion_count_; RTC_DCHECK(recursion_count_ >= 0); if (!recursion_count_) thread_ = 0; -#endif -#if defined(WEBRTC_MAC) && !USE_NATIVE_MUTEX_ON_MAC +# endif +# if defined(WEBRTC_MAC) && !USE_NATIVE_MUTEX_ON_MAC RTC_DCHECK(IsThreadRefEqual(owning_thread_, CurrentThreadRef())); RTC_DCHECK_GE(recursion_, 0); --recursion_; @@ -157,9 +168,11 @@ if (AtomicOps::Decrement(&lock_queue_) > 0 && !recursion_) dispatch_semaphore_signal(semaphore_); -#else +# else pthread_mutex_unlock(&mutex_); -#endif +# endif +#else +# error Unsupported platform. #endif } @@ -171,12 +184,14 @@ // 'type1' to 'type2' of greater size return crit_.OwningThread == reinterpret_cast<HANDLE>(static_cast<size_t>(GetCurrentThreadId())); -#else -#if CS_DEBUG_CHECKS +#elif defined(WEBRTC_POSIX) +# if CS_DEBUG_CHECKS return IsThreadRefEqual(thread_, CurrentThreadRef()); -#else +# else return true; -#endif // CS_DEBUG_CHECKS +# endif // CS_DEBUG_CHECKS +#else +# error Unsupported platform. #endif } @@ -186,6 +201,7 @@ TryCritScope::TryCritScope(const CriticalSection* cs) : cs_(cs), locked_(cs->TryEnter()) { CS_DEBUG_CODE(lock_was_called_ = false); + RTC_UNUSED(lock_was_called_); } TryCritScope::~TryCritScope() {
diff --git a/webrtc/base/criticalsection.h b/webrtc/base/criticalsection.h index a0f9a6b..d18f24f 100644 --- a/webrtc/base/criticalsection.h +++ b/webrtc/base/criticalsection.h
@@ -14,8 +14,9 @@ #include "webrtc/base/atomicops.h" #include "webrtc/base/checks.h" #include "webrtc/base/constructormagic.h" -#include "webrtc/base/thread_annotations.h" #include "webrtc/base/platform_thread_types.h" +#include "webrtc/base/thread_annotations.h" +#include "webrtc/typedefs.h" #if defined(WEBRTC_WIN) // Include winsock2.h before including <windows.h> to maintain consistency with @@ -67,7 +68,7 @@ #if defined(WEBRTC_WIN) mutable CRITICAL_SECTION crit_; #elif defined(WEBRTC_POSIX) -#if defined(WEBRTC_MAC) && !USE_NATIVE_MUTEX_ON_MAC +# if defined(WEBRTC_MAC) && !USE_NATIVE_MUTEX_ON_MAC // Number of times the lock has been locked + number of threads waiting. // TODO(tommi): We could use this number and subtract the recursion count // to find places where we have multiple threads contending on the same lock. @@ -79,11 +80,13 @@ mutable dispatch_semaphore_t semaphore_; // The thread that currently holds the lock. Required to handle recursion. mutable PlatformThreadRef owning_thread_; -#else +# else mutable pthread_mutex_t mutex_; -#endif - CS_DEBUG_CODE(mutable PlatformThreadRef thread_); - CS_DEBUG_CODE(mutable int recursion_count_); +# endif + mutable PlatformThreadRef thread_; // Only used by RTC_DCHECKs. + mutable int recursion_count_; // Only used by RTC_DCHECKs. +#else // !defined(WEBRTC_WIN) && !defined(WEBRTC_POSIX) +# error Unsupported platform. #endif }; @@ -110,13 +113,15 @@ ~TryCritScope(); #if defined(WEBRTC_WIN) _Check_return_ bool locked() const; -#else +#elif defined(WEBRTC_POSIX) bool locked() const __attribute__ ((__warn_unused_result__)); +#else // !defined(WEBRTC_WIN) && !defined(WEBRTC_POSIX) +# error Unsupported platform. #endif private: const CriticalSection* const cs_; const bool locked_; - CS_DEBUG_CODE(mutable bool lock_was_called_); + mutable bool lock_was_called_; // Only used by RTC_DCHECKs. RTC_DISALLOW_COPY_AND_ASSIGN(TryCritScope); };