Revert 5516 "Thread annotation of talk_base::CriticalSection."

r5516 failed compilation on builds with enable_webrtc=0.

> Thread annotation of talk_base::CriticalSection.
> 
> Also enabling -Wthread-safety in talk/build/common.gypi for clang on
> Linux. Thread annotations are compile-time checks that for instance
> certain locks are held before accessing a value.
> 
> BUG=
> TEST=Local GUARDED_BY() annotations.
> R=andresp@webrtc.org, fischman@webrtc.org
> 
> Review URL: https://webrtc-codereview.appspot.com/8189004

TBR=pbos@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/8409004

git-svn-id: http://webrtc.googlecode.com/svn/trunk@5523 4adac7df-926f-26a2-2b94-8c16560cd09d
This commit is contained in:
wjia@webrtc.org 2014-02-10 23:20:15 +00:00
parent 16c08f03da
commit dd82fa726c
5 changed files with 24 additions and 30 deletions

View File

@ -29,7 +29,6 @@
#define TALK_BASE_CRITICALSECTION_H__ #define TALK_BASE_CRITICALSECTION_H__
#include "talk/base/constructormagic.h" #include "talk/base/constructormagic.h"
#include "webrtc/system_wrappers/interface/thread_annotations.h"
#ifdef WIN32 #ifdef WIN32
#include "talk/base/win32.h" #include "talk/base/win32.h"
@ -52,7 +51,7 @@
namespace talk_base { namespace talk_base {
#ifdef WIN32 #ifdef WIN32
class LOCKABLE CriticalSection { class CriticalSection {
public: public:
CriticalSection() { CriticalSection() {
InitializeCriticalSection(&crit_); InitializeCriticalSection(&crit_);
@ -62,18 +61,18 @@ class LOCKABLE CriticalSection {
~CriticalSection() { ~CriticalSection() {
DeleteCriticalSection(&crit_); DeleteCriticalSection(&crit_);
} }
void Enter() EXCLUSIVE_LOCK_FUNCTION() { void Enter() {
EnterCriticalSection(&crit_); EnterCriticalSection(&crit_);
TRACK_OWNER(thread_ = GetCurrentThreadId()); TRACK_OWNER(thread_ = GetCurrentThreadId());
} }
bool TryEnter() EXCLUSIVE_TRYLOCK_FUNCTION(true) { bool TryEnter() {
if (TryEnterCriticalSection(&crit_) != FALSE) { if (TryEnterCriticalSection(&crit_) != FALSE) {
TRACK_OWNER(thread_ = GetCurrentThreadId()); TRACK_OWNER(thread_ = GetCurrentThreadId());
return true; return true;
} }
return false; return false;
} }
void Leave() UNLOCK_FUNCTION() { void Leave() {
TRACK_OWNER(thread_ = 0); TRACK_OWNER(thread_ = 0);
LeaveCriticalSection(&crit_); LeaveCriticalSection(&crit_);
} }
@ -89,7 +88,7 @@ class LOCKABLE CriticalSection {
#endif // WIN32 #endif // WIN32
#ifdef POSIX #ifdef POSIX
class LOCKABLE CriticalSection { class CriticalSection {
public: public:
CriticalSection() { CriticalSection() {
pthread_mutexattr_t mutex_attribute; pthread_mutexattr_t mutex_attribute;
@ -102,18 +101,18 @@ class LOCKABLE CriticalSection {
~CriticalSection() { ~CriticalSection() {
pthread_mutex_destroy(&mutex_); pthread_mutex_destroy(&mutex_);
} }
void Enter() EXCLUSIVE_LOCK_FUNCTION() { void Enter() {
pthread_mutex_lock(&mutex_); pthread_mutex_lock(&mutex_);
TRACK_OWNER(thread_ = pthread_self()); TRACK_OWNER(thread_ = pthread_self());
} }
bool TryEnter() EXCLUSIVE_TRYLOCK_FUNCTION(true) { bool TryEnter() {
if (pthread_mutex_trylock(&mutex_) == 0) { if (pthread_mutex_trylock(&mutex_) == 0) {
TRACK_OWNER(thread_ = pthread_self()); TRACK_OWNER(thread_ = pthread_self());
return true; return true;
} }
return false; return false;
} }
void Leave() UNLOCK_FUNCTION() { void Leave() {
TRACK_OWNER(thread_ = 0); TRACK_OWNER(thread_ = 0);
pthread_mutex_unlock(&mutex_); pthread_mutex_unlock(&mutex_);
} }
@ -129,13 +128,13 @@ class LOCKABLE CriticalSection {
#endif // POSIX #endif // POSIX
// CritScope, for serializing execution through a scope. // CritScope, for serializing execution through a scope.
class SCOPED_LOCKABLE CritScope { class CritScope {
public: public:
explicit CritScope(CriticalSection *pcrit) EXCLUSIVE_LOCK_FUNCTION(pcrit) { explicit CritScope(CriticalSection *pcrit) {
pcrit_ = pcrit; pcrit_ = pcrit;
pcrit_->Enter(); pcrit_->Enter();
} }
~CritScope() UNLOCK_FUNCTION() { ~CritScope() {
pcrit_->Leave(); pcrit_->Leave();
} }
private: private:

View File

@ -36,14 +36,14 @@ namespace talk_base {
// This class provides shared-exclusive lock. It can be used in cases like // This class provides shared-exclusive lock. It can be used in cases like
// multiple-readers/single-writer model. // multiple-readers/single-writer model.
class LOCKABLE SharedExclusiveLock { class SharedExclusiveLock {
public: public:
SharedExclusiveLock(); SharedExclusiveLock();
// Locking/unlocking methods. It is encouraged to use SharedScope or // Locking/unlocking methods. It is encouraged to use SharedScope or
// ExclusiveScope for protection. // ExclusiveScope for protection.
void LockExclusive() EXCLUSIVE_LOCK_FUNCTION(); void LockExclusive();
void UnlockExclusive() UNLOCK_FUNCTION(); void UnlockExclusive();
void LockShared(); void LockShared();
void UnlockShared(); void UnlockShared();
@ -56,14 +56,13 @@ class LOCKABLE SharedExclusiveLock {
DISALLOW_COPY_AND_ASSIGN(SharedExclusiveLock); DISALLOW_COPY_AND_ASSIGN(SharedExclusiveLock);
}; };
class SCOPED_LOCKABLE SharedScope { class SharedScope {
public: public:
explicit SharedScope(SharedExclusiveLock* lock) SHARED_LOCK_FUNCTION(lock) explicit SharedScope(SharedExclusiveLock* lock) : lock_(lock) {
: lock_(lock) {
lock_->LockShared(); lock_->LockShared();
} }
~SharedScope() UNLOCK_FUNCTION() { ~SharedScope() {
lock_->UnlockShared(); lock_->UnlockShared();
} }
@ -73,15 +72,13 @@ class SCOPED_LOCKABLE SharedScope {
DISALLOW_COPY_AND_ASSIGN(SharedScope); DISALLOW_COPY_AND_ASSIGN(SharedScope);
}; };
class SCOPED_LOCKABLE ExclusiveScope { class ExclusiveScope {
public: public:
explicit ExclusiveScope(SharedExclusiveLock* lock) explicit ExclusiveScope(SharedExclusiveLock* lock) : lock_(lock) {
EXCLUSIVE_LOCK_FUNCTION(lock)
: lock_(lock) {
lock_->LockExclusive(); lock_->LockExclusive();
} }
~ExclusiveScope() UNLOCK_FUNCTION() { ~ExclusiveScope() {
lock_->UnlockExclusive(); lock_->UnlockExclusive();
} }

View File

@ -132,17 +132,16 @@ class SignalThread
DISALLOW_IMPLICIT_CONSTRUCTORS(Worker); DISALLOW_IMPLICIT_CONSTRUCTORS(Worker);
}; };
class SCOPED_LOCKABLE EnterExit { class EnterExit {
public: public:
explicit EnterExit(SignalThread* t) EXCLUSIVE_LOCK_FUNCTION(t->cs_) explicit EnterExit(SignalThread* t) : t_(t) {
: t_(t) {
t_->cs_.Enter(); t_->cs_.Enter();
// If refcount_ is zero then the object has already been deleted and we // If refcount_ is zero then the object has already been deleted and we
// will be double-deleting it in ~EnterExit()! (shouldn't happen) // will be double-deleting it in ~EnterExit()! (shouldn't happen)
ASSERT(t_->refcount_ != 0); ASSERT(t_->refcount_ != 0);
++t_->refcount_; ++t_->refcount_;
} }
~EnterExit() UNLOCK_FUNCTION() { ~EnterExit() {
bool d = (0 == --t_->refcount_); bool d = (0 == --t_->refcount_);
t_->cs_.Leave(); t_->cs_.Leave();
if (d) if (d)

View File

@ -89,7 +89,6 @@
# LateBindingSymbolTable::TableInfo from # LateBindingSymbolTable::TableInfo from
# latebindingsymboltable.cc.def and remove below flag. # latebindingsymboltable.cc.def and remove below flag.
'-Wno-address-of-array-temporary', '-Wno-address-of-array-temporary',
'-Wthread-safety',
], ],
}], }],
], ],

View File

@ -77,7 +77,7 @@ class MediaMonitorT : public MediaMonitor {
media_info_.Clear(); media_info_.Clear();
media_channel_->GetStats(&media_info_); media_channel_->GetStats(&media_info_);
} }
virtual void Update() EXCLUSIVE_LOCKS_REQUIRED(crit_) { virtual void Update() {
MI stats(media_info_); MI stats(media_info_);
crit_.Leave(); crit_.Leave();
SignalUpdate(media_channel_, stats); SignalUpdate(media_channel_, stats);