Move CriticalSection into rtc_base_approved.
This class is being used from both libjingle and webrtc but we recently had a regression when we added dependency on libjingle's Thread class. This cleans that up and moves the implementation of CriticalSection and helper classes into the source file.
I'm also improving debugging facilities and constness.
BUG=
R=magjed@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/51769004
Cr-Commit-Position: refs/heads/master@{#9089}
diff --git a/webrtc/base/BUILD.gn b/webrtc/base/BUILD.gn
index 363f8c3..35d33ab 100644
--- a/webrtc/base/BUILD.gn
+++ b/webrtc/base/BUILD.gn
@@ -114,6 +114,8 @@
"byteorder.h",
"checks.cc",
"checks.h",
+ "criticalsection.cc",
+ "criticalsection.h",
"event.cc",
"event.h",
"event_tracer.cc",
@@ -204,8 +206,6 @@
"cpumonitor.h",
"crc32.cc",
"crc32.h",
- "criticalsection.cc",
- "criticalsection.h",
"cryptstring.cc",
"cryptstring.h",
"diskcache.cc",
diff --git a/webrtc/base/base.gyp b/webrtc/base/base.gyp
index ce64bd0..51b7291 100644
--- a/webrtc/base/base.gyp
+++ b/webrtc/base/base.gyp
@@ -29,6 +29,8 @@
'target_name': 'rtc_base_approved',
'type': 'static_library',
'sources': [
+ '../overrides/webrtc/base/basictypes.h',
+ '../overrides/webrtc/base/constructormagic.h',
'basictypes.h',
'bitbuffer.cc',
'bitbuffer.h',
@@ -40,6 +42,8 @@
'checks.cc',
'checks.h',
'constructormagic.h',
+ 'criticalsection.cc',
+ 'criticalsection.h',
'event.cc',
'event.h',
'event_tracer.cc',
@@ -66,8 +70,6 @@
'timeutils.cc',
'timeutils.h',
'trace_event.h',
- '../overrides/webrtc/base/basictypes.h',
- '../overrides/webrtc/base/constructormagic.h',
],
'conditions': [
['build_with_chromium==1', {
@@ -134,8 +136,6 @@
'cpumonitor.h',
'crc32.cc',
'crc32.h',
- 'criticalsection.cc',
- 'criticalsection.h',
'cryptstring.cc',
'cryptstring.h',
'dbus.cc',
diff --git a/webrtc/base/criticalsection.cc b/webrtc/base/criticalsection.cc
index fcad5c3..f4822d9 100644
--- a/webrtc/base/criticalsection.cc
+++ b/webrtc/base/criticalsection.cc
@@ -11,13 +11,135 @@
#include "webrtc/base/criticalsection.h"
#include "webrtc/base/checks.h"
-#include "webrtc/base/thread.h"
namespace rtc {
+CriticalSection::CriticalSection() {
+#if defined(WEBRTC_WIN)
+ InitializeCriticalSection(&crit_);
+#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);
+ CS_DEBUG_CODE(thread_ = 0);
+ CS_DEBUG_CODE(recursion_count_ = 0);
+#endif
+}
+
+CriticalSection::~CriticalSection() {
+#if defined(WEBRTC_WIN)
+ DeleteCriticalSection(&crit_);
+#else
+ pthread_mutex_destroy(&mutex_);
+#endif
+}
+
+void CriticalSection::Enter() EXCLUSIVE_LOCK_FUNCTION() {
+#if defined(WEBRTC_WIN)
+ EnterCriticalSection(&crit_);
+#else
+ pthread_mutex_lock(&mutex_);
+#if CS_DEBUG_CHECKS
+ if (!recursion_count_) {
+ DCHECK(!thread_);
+ thread_ = pthread_self();
+ } else {
+ DCHECK(CurrentThreadIsOwner());
+ }
+ ++recursion_count_;
+#endif
+#endif
+}
+
+bool CriticalSection::TryEnter() EXCLUSIVE_TRYLOCK_FUNCTION(true) {
+#if defined(WEBRTC_WIN)
+ return TryEnterCriticalSection(&crit_) != FALSE;
+#else
+ if (pthread_mutex_trylock(&mutex_) != 0)
+ return false;
+#if CS_DEBUG_CHECKS
+ if (!recursion_count_) {
+ DCHECK(!thread_);
+ thread_ = pthread_self();
+ } else {
+ DCHECK(CurrentThreadIsOwner());
+ }
+ ++recursion_count_;
+#endif
+ return true;
+#endif
+}
+void CriticalSection::Leave() UNLOCK_FUNCTION() {
+ DCHECK(CurrentThreadIsOwner());
+#if defined(WEBRTC_WIN)
+ LeaveCriticalSection(&crit_);
+#else
+#if CS_DEBUG_CHECKS
+ --recursion_count_;
+ DCHECK(recursion_count_ >= 0);
+ if (!recursion_count_)
+ thread_ = 0;
+#endif
+ pthread_mutex_unlock(&mutex_);
+#endif
+}
+
+bool CriticalSection::CurrentThreadIsOwner() const {
+#if defined(WEBRTC_WIN)
+ return crit_.OwningThread == reinterpret_cast<HANDLE>(GetCurrentThreadId());
+#else
+#if CS_DEBUG_CHECKS
+ return pthread_equal(thread_, pthread_self());
+#else
+ return true;
+#endif // CS_DEBUG_CHECKS
+#endif
+}
+
+bool CriticalSection::IsLocked() const {
+#if defined(WEBRTC_WIN)
+ return crit_.LockCount != -1;
+#else
+#if CS_DEBUG_CHECKS
+ return thread_ != 0;
+#else
+ return true;
+#endif
+#endif
+}
+
+CritScope::CritScope(CriticalSection* cs) : cs_(cs) { cs_->Enter(); }
+CritScope::~CritScope() { cs_->Leave(); }
+
+TryCritScope::TryCritScope(CriticalSection* cs)
+ : cs_(cs), locked_(cs->TryEnter()) {
+ CS_DEBUG_CODE(lock_was_called_ = false);
+}
+
+TryCritScope::~TryCritScope() {
+ CS_DEBUG_CODE(DCHECK(lock_was_called_));
+ if (locked_)
+ cs_->Leave();
+}
+
+bool TryCritScope::locked() const {
+ CS_DEBUG_CODE(lock_was_called_ = true);
+ return locked_;
+}
+
void GlobalLockPod::Lock() {
+#if !defined(WEBRTC_WIN)
+ const struct timespec ts_null = {0};
+#endif
+
while (AtomicOps::CompareAndSwap(&lock_acquired, 0, 1)) {
- Thread::SleepMs(0);
+#if defined(WEBRTC_WIN)
+ ::Sleep(0);
+#else
+ nanosleep(&ts_null, nullptr);
+#endif
}
}
diff --git a/webrtc/base/criticalsection.h b/webrtc/base/criticalsection.h
index 1891f83..d4e6bd7 100644
--- a/webrtc/base/criticalsection.h
+++ b/webrtc/base/criticalsection.h
@@ -21,116 +21,56 @@
// exists as two separate projects, webrtc and libjingle.
#include <winsock2.h>
#include <windows.h>
-#endif
+#include <sal.h> // must come after windows headers.
+#endif // defined(WEBRTC_WIN)
#if defined(WEBRTC_POSIX)
#include <pthread.h>
#endif
#if (!defined(NDEBUG) || defined(DCHECK_ALWAYS_ON))
-#define CS_TRACK_OWNER 1
+#define CS_DEBUG_CHECKS 1
#endif
-#if CS_TRACK_OWNER
-#define TRACK_OWNER(x) x
-#else // !CS_TRACK_OWNER
-#define TRACK_OWNER(x)
-#endif // !CS_TRACK_OWNER
+#if CS_DEBUG_CHECKS
+#define CS_DEBUG_CODE(x) x
+#else // !CS_DEBUG_CHECKS
+#define CS_DEBUG_CODE(x)
+#endif // !CS_DEBUG_CHECKS
namespace rtc {
+class LOCKABLE CriticalSection {
+ public:
+ CriticalSection();
+ ~CriticalSection();
+
+ void Enter() EXCLUSIVE_LOCK_FUNCTION();
+ bool TryEnter() EXCLUSIVE_TRYLOCK_FUNCTION(true);
+ void Leave() UNLOCK_FUNCTION();
+
+ // Use only for DCHECKing.
+ bool CurrentThreadIsOwner() const;
+ // Use only for DCHECKing.
+ bool IsLocked() const;
+
+ private:
#if defined(WEBRTC_WIN)
-class LOCKABLE CriticalSection {
- public:
- CriticalSection() { InitializeCriticalSection(&crit_); }
- ~CriticalSection() { DeleteCriticalSection(&crit_); }
- void Enter() EXCLUSIVE_LOCK_FUNCTION() {
- EnterCriticalSection(&crit_);
- }
- bool TryEnter() EXCLUSIVE_TRYLOCK_FUNCTION(true) {
- return TryEnterCriticalSection(&crit_) != FALSE;
- }
- void Leave() UNLOCK_FUNCTION() {
- LeaveCriticalSection(&crit_);
- }
-
- // Used for debugging.
- bool CurrentThreadIsOwner() const {
- return crit_.OwningThread == reinterpret_cast<HANDLE>(GetCurrentThreadId());
- }
- // Use only for DCHECKing.
- bool IsLocked() { return crit_.LockCount != -1; }
-
- private:
CRITICAL_SECTION crit_;
-};
-#endif // WEBRTC_WIN
-
-#if defined(WEBRTC_POSIX)
-class LOCKABLE CriticalSection {
- public:
- CriticalSection() {
- 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);
- TRACK_OWNER(thread_ = 0);
- }
- ~CriticalSection() {
- pthread_mutex_destroy(&mutex_);
- }
- void Enter() EXCLUSIVE_LOCK_FUNCTION() {
- pthread_mutex_lock(&mutex_);
- TRACK_OWNER(thread_ = pthread_self());
- }
- bool TryEnter() EXCLUSIVE_TRYLOCK_FUNCTION(true) {
- if (pthread_mutex_trylock(&mutex_) == 0) {
- TRACK_OWNER(thread_ = pthread_self());
- return true;
- }
- return false;
- }
- void Leave() UNLOCK_FUNCTION() {
- TRACK_OWNER(thread_ = 0);
- pthread_mutex_unlock(&mutex_);
- }
-
- // Used for debugging.
- bool CurrentThreadIsOwner() const {
-#if CS_TRACK_OWNER
- return pthread_equal(thread_, pthread_self());
-#else
- return true;
-#endif // CS_TRACK_OWNER
- }
- // Use only for DCHECKing.
- bool IsLocked() {
-#if CS_TRACK_OWNER
- return thread_ != 0;
-#else
- return true;
-#endif
- }
-
- private:
+#elif defined(WEBRTC_POSIX)
pthread_mutex_t mutex_;
- TRACK_OWNER(pthread_t thread_);
+ CS_DEBUG_CODE(pthread_t thread_);
+ CS_DEBUG_CODE(int recursion_count_);
+#endif
};
-#endif // WEBRTC_POSIX
// CritScope, for serializing execution through a scope.
class SCOPED_LOCKABLE CritScope {
public:
- explicit CritScope(CriticalSection *pcrit) EXCLUSIVE_LOCK_FUNCTION(pcrit) {
- pcrit_ = pcrit;
- pcrit_->Enter();
- }
- ~CritScope() UNLOCK_FUNCTION() {
- pcrit_->Leave();
- }
+ explicit CritScope(CriticalSection* cs) EXCLUSIVE_LOCK_FUNCTION(cs);
+ ~CritScope() UNLOCK_FUNCTION();
private:
- CriticalSection *pcrit_;
+ CriticalSection* const cs_;
DISALLOW_COPY_AND_ASSIGN(CritScope);
};
@@ -143,21 +83,17 @@
// lock was taken. If you're not calling locked(), you're doing it wrong!
class TryCritScope {
public:
- explicit TryCritScope(CriticalSection *pcrit) {
- pcrit_ = pcrit;
- locked_ = pcrit_->TryEnter();
- }
- ~TryCritScope() {
- if (locked_) {
- pcrit_->Leave();
- }
- }
- bool locked() const {
- return locked_;
- }
+ explicit TryCritScope(CriticalSection* cs);
+ ~TryCritScope();
+#if defined(WEBRTC_WIN)
+ _Check_return_ bool locked() const;
+#else
+ bool locked() const __attribute__((warn_unused_result));
+#endif
private:
- CriticalSection *pcrit_;
- bool locked_;
+ CriticalSection* const cs_;
+ const bool locked_;
+ CS_DEBUG_CODE(mutable bool lock_was_called_);
DISALLOW_COPY_AND_ASSIGN(TryCritScope);
};
diff --git a/webrtc/base/messagequeue.cc b/webrtc/base/messagequeue.cc
index 53e451f..5cf448e 100644
--- a/webrtc/base/messagequeue.cc
+++ b/webrtc/base/messagequeue.cc
@@ -59,7 +59,7 @@
// MessageQueueManager methods should be non-reentrant, so we
// ASSERT that is the case. If any of these ASSERT, please
// contact bpm or jbeda.
-#if CS_TRACK_OWNER // CurrentThreadIsOwner returns true by default.
+#if CS_DEBUG_CHECKS // CurrentThreadIsOwner returns true by default.
ASSERT(!crit_.CurrentThreadIsOwner());
#endif
CritScope cs(&crit_);
@@ -73,7 +73,7 @@
return Instance()->RemoveInternal(message_queue);
}
void MessageQueueManager::RemoveInternal(MessageQueue *message_queue) {
-#if CS_TRACK_OWNER // CurrentThreadIsOwner returns true by default.
+#if CS_DEBUG_CHECKS // CurrentThreadIsOwner returns true by default.
ASSERT(!crit_.CurrentThreadIsOwner()); // See note above.
#endif
// If this is the last MessageQueue, destroy the manager as well so that
@@ -104,7 +104,7 @@
return Instance()->ClearInternal(handler);
}
void MessageQueueManager::ClearInternal(MessageHandler *handler) {
-#if CS_TRACK_OWNER // CurrentThreadIsOwner returns true by default.
+#if CS_DEBUG_CHECKS // CurrentThreadIsOwner returns true by default.
ASSERT(!crit_.CurrentThreadIsOwner()); // See note above.
#endif
CritScope cs(&crit_);
