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

git-svn-id: http://webrtc.googlecode.com/svn/trunk@5516 4adac7df-926f-26a2-2b94-8c16560cd09d
This commit is contained in:
pbos@webrtc.org
2014-02-10 13:58:37 +00:00
parent 9cba2e4802
commit 0a7085ffc2
5 changed files with 30 additions and 24 deletions

View File

@ -29,6 +29,7 @@
#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"
@ -51,7 +52,7 @@
namespace talk_base { namespace talk_base {
#ifdef WIN32 #ifdef WIN32
class CriticalSection { class LOCKABLE CriticalSection {
public: public:
CriticalSection() { CriticalSection() {
InitializeCriticalSection(&crit_); InitializeCriticalSection(&crit_);
@ -61,18 +62,18 @@ class CriticalSection {
~CriticalSection() { ~CriticalSection() {
DeleteCriticalSection(&crit_); DeleteCriticalSection(&crit_);
} }
void Enter() { void Enter() EXCLUSIVE_LOCK_FUNCTION() {
EnterCriticalSection(&crit_); EnterCriticalSection(&crit_);
TRACK_OWNER(thread_ = GetCurrentThreadId()); TRACK_OWNER(thread_ = GetCurrentThreadId());
} }
bool TryEnter() { bool TryEnter() EXCLUSIVE_TRYLOCK_FUNCTION(true) {
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() { void Leave() UNLOCK_FUNCTION() {
TRACK_OWNER(thread_ = 0); TRACK_OWNER(thread_ = 0);
LeaveCriticalSection(&crit_); LeaveCriticalSection(&crit_);
} }
@ -88,7 +89,7 @@ class CriticalSection {
#endif // WIN32 #endif // WIN32
#ifdef POSIX #ifdef POSIX
class CriticalSection { class LOCKABLE CriticalSection {
public: public:
CriticalSection() { CriticalSection() {
pthread_mutexattr_t mutex_attribute; pthread_mutexattr_t mutex_attribute;
@ -101,18 +102,18 @@ class CriticalSection {
~CriticalSection() { ~CriticalSection() {
pthread_mutex_destroy(&mutex_); pthread_mutex_destroy(&mutex_);
} }
void Enter() { void Enter() EXCLUSIVE_LOCK_FUNCTION() {
pthread_mutex_lock(&mutex_); pthread_mutex_lock(&mutex_);
TRACK_OWNER(thread_ = pthread_self()); TRACK_OWNER(thread_ = pthread_self());
} }
bool TryEnter() { bool TryEnter() EXCLUSIVE_TRYLOCK_FUNCTION(true) {
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() { void Leave() UNLOCK_FUNCTION() {
TRACK_OWNER(thread_ = 0); TRACK_OWNER(thread_ = 0);
pthread_mutex_unlock(&mutex_); pthread_mutex_unlock(&mutex_);
} }
@ -128,13 +129,13 @@ class CriticalSection {
#endif // POSIX #endif // POSIX
// CritScope, for serializing execution through a scope. // CritScope, for serializing execution through a scope.
class CritScope { class SCOPED_LOCKABLE CritScope {
public: public:
explicit CritScope(CriticalSection *pcrit) { explicit CritScope(CriticalSection *pcrit) EXCLUSIVE_LOCK_FUNCTION(pcrit) {
pcrit_ = pcrit; pcrit_ = pcrit;
pcrit_->Enter(); pcrit_->Enter();
} }
~CritScope() { ~CritScope() UNLOCK_FUNCTION() {
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 SharedExclusiveLock { class LOCKABLE 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(); void LockExclusive() EXCLUSIVE_LOCK_FUNCTION();
void UnlockExclusive(); void UnlockExclusive() UNLOCK_FUNCTION();
void LockShared(); void LockShared();
void UnlockShared(); void UnlockShared();
@ -56,13 +56,14 @@ class SharedExclusiveLock {
DISALLOW_COPY_AND_ASSIGN(SharedExclusiveLock); DISALLOW_COPY_AND_ASSIGN(SharedExclusiveLock);
}; };
class SharedScope { class SCOPED_LOCKABLE SharedScope {
public: public:
explicit SharedScope(SharedExclusiveLock* lock) : lock_(lock) { explicit SharedScope(SharedExclusiveLock* lock) SHARED_LOCK_FUNCTION(lock)
: lock_(lock) {
lock_->LockShared(); lock_->LockShared();
} }
~SharedScope() { ~SharedScope() UNLOCK_FUNCTION() {
lock_->UnlockShared(); lock_->UnlockShared();
} }
@ -72,13 +73,15 @@ class SharedScope {
DISALLOW_COPY_AND_ASSIGN(SharedScope); DISALLOW_COPY_AND_ASSIGN(SharedScope);
}; };
class ExclusiveScope { class SCOPED_LOCKABLE ExclusiveScope {
public: public:
explicit ExclusiveScope(SharedExclusiveLock* lock) : lock_(lock) { explicit ExclusiveScope(SharedExclusiveLock* lock)
EXCLUSIVE_LOCK_FUNCTION(lock)
: lock_(lock) {
lock_->LockExclusive(); lock_->LockExclusive();
} }
~ExclusiveScope() { ~ExclusiveScope() UNLOCK_FUNCTION() {
lock_->UnlockExclusive(); lock_->UnlockExclusive();
} }

View File

@ -132,16 +132,17 @@ class SignalThread
DISALLOW_IMPLICIT_CONSTRUCTORS(Worker); DISALLOW_IMPLICIT_CONSTRUCTORS(Worker);
}; };
class EnterExit { class SCOPED_LOCKABLE EnterExit {
public: public:
explicit EnterExit(SignalThread* t) : t_(t) { explicit EnterExit(SignalThread* t) EXCLUSIVE_LOCK_FUNCTION(t->cs_)
: 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() { ~EnterExit() UNLOCK_FUNCTION() {
bool d = (0 == --t_->refcount_); bool d = (0 == --t_->refcount_);
t_->cs_.Leave(); t_->cs_.Leave();
if (d) if (d)

View File

@ -89,6 +89,7 @@
# 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() { virtual void Update() EXCLUSIVE_LOCKS_REQUIRED(crit_) {
MI stats(media_info_); MI stats(media_info_);
crit_.Leave(); crit_.Leave();
SignalUpdate(media_channel_, stats); SignalUpdate(media_channel_, stats);