Commit e41ffb6c authored by Jeremy Roman's avatar Jeremy Roman Committed by Commit Bot

WTF: CHECK if a mutex is recursively acquired.

This is a final attempt to flush out any recursive acquisition
before support is totally removed for WTF::RecursiveMutex.

For pthreads, this requires maintaining the recursion_count_
variable even in non-DCHECK builds. This is still quite cheap,
and is temporary in any case.

Bug: 856641
Change-Id: I2b550fd58e8c4cf6293ad7628dd49aa366b41ae9
Reviewed-on: https://chromium-review.googlesource.com/1212453Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Commit-Queue: Jeremy Roman <jbroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589504}
parent cb64b882
...@@ -56,9 +56,7 @@ typedef CONDITION_VARIABLE PlatformCondition; ...@@ -56,9 +56,7 @@ typedef CONDITION_VARIABLE PlatformCondition;
#elif defined(OS_POSIX) || defined(OS_FUCHSIA) #elif defined(OS_POSIX) || defined(OS_FUCHSIA)
struct PlatformMutex { struct PlatformMutex {
pthread_mutex_t internal_mutex_; pthread_mutex_t internal_mutex_;
#if DCHECK_IS_ON()
size_t recursion_count_; size_t recursion_count_;
#endif
}; };
typedef pthread_cond_t PlatformCondition; typedef pthread_cond_t PlatformCondition;
#endif #endif
......
...@@ -70,9 +70,7 @@ MutexBase::MutexBase(bool recursive) { ...@@ -70,9 +70,7 @@ MutexBase::MutexBase(bool recursive) {
int result = pthread_mutex_init(&mutex_.internal_mutex_, &attr); int result = pthread_mutex_init(&mutex_.internal_mutex_, &attr);
DCHECK_EQ(result, 0); DCHECK_EQ(result, 0);
#if DCHECK_IS_ON()
mutex_.recursion_count_ = 0; mutex_.recursion_count_ = 0;
#endif
pthread_mutexattr_destroy(&attr); pthread_mutexattr_destroy(&attr);
} }
...@@ -85,18 +83,14 @@ MutexBase::~MutexBase() { ...@@ -85,18 +83,14 @@ MutexBase::~MutexBase() {
void MutexBase::lock() { void MutexBase::lock() {
int result = pthread_mutex_lock(&mutex_.internal_mutex_); int result = pthread_mutex_lock(&mutex_.internal_mutex_);
DCHECK_EQ(result, 0); DCHECK_EQ(result, 0);
#if DCHECK_IS_ON() CHECK(!mutex_.recursion_count_)
DCHECK(!mutex_.recursion_count_)
<< "WTF does not support recursive mutex acquisition!"; << "WTF does not support recursive mutex acquisition!";
++mutex_.recursion_count_; ++mutex_.recursion_count_;
#endif
} }
void MutexBase::unlock() { void MutexBase::unlock() {
#if DCHECK_IS_ON()
DCHECK(mutex_.recursion_count_); DCHECK(mutex_.recursion_count_);
--mutex_.recursion_count_; --mutex_.recursion_count_;
#endif
int result = pthread_mutex_unlock(&mutex_.internal_mutex_); int result = pthread_mutex_unlock(&mutex_.internal_mutex_);
DCHECK_EQ(result, 0); DCHECK_EQ(result, 0);
} }
...@@ -109,13 +103,11 @@ void MutexBase::unlock() { ...@@ -109,13 +103,11 @@ void MutexBase::unlock() {
bool Mutex::TryLock() { bool Mutex::TryLock() {
int result = pthread_mutex_trylock(&mutex_.internal_mutex_); int result = pthread_mutex_trylock(&mutex_.internal_mutex_);
if (result == 0) { if (result == 0) {
#if DCHECK_IS_ON()
// The Mutex class is not recursive, so the recursionCount should be // The Mutex class is not recursive, so the recursionCount should be
// zero after getting the lock. // zero after getting the lock.
DCHECK(!mutex_.recursion_count_) CHECK(!mutex_.recursion_count_)
<< "WTF does not support recursive mutex acquisition!"; << "WTF does not support recursive mutex acquisition!";
++mutex_.recursion_count_; ++mutex_.recursion_count_;
#endif
return true; return true;
} }
if (result == EBUSY) if (result == EBUSY)
...@@ -128,11 +120,9 @@ bool Mutex::TryLock() { ...@@ -128,11 +120,9 @@ bool Mutex::TryLock() {
bool RecursiveMutex::TryLock() { bool RecursiveMutex::TryLock() {
int result = pthread_mutex_trylock(&mutex_.internal_mutex_); int result = pthread_mutex_trylock(&mutex_.internal_mutex_);
if (result == 0) { if (result == 0) {
#if DCHECK_IS_ON() CHECK(!mutex_.recursion_count_)
DCHECK(!mutex_.recursion_count_)
<< "WTF does not support recursive mutex acquisition!"; << "WTF does not support recursive mutex acquisition!";
++mutex_.recursion_count_; ++mutex_.recursion_count_;
#endif
return true; return true;
} }
if (result == EBUSY) if (result == EBUSY)
...@@ -152,14 +142,10 @@ ThreadCondition::~ThreadCondition() { ...@@ -152,14 +142,10 @@ ThreadCondition::~ThreadCondition() {
void ThreadCondition::Wait() { void ThreadCondition::Wait() {
base::ScopedBlockingCall scoped_blocking_call(base::BlockingType::MAY_BLOCK); base::ScopedBlockingCall scoped_blocking_call(base::BlockingType::MAY_BLOCK);
#if DCHECK_IS_ON()
--mutex_.recursion_count_; --mutex_.recursion_count_;
#endif
int result = pthread_cond_wait(&condition_, &mutex_.internal_mutex_); int result = pthread_cond_wait(&condition_, &mutex_.internal_mutex_);
DCHECK_EQ(result, 0); DCHECK_EQ(result, 0);
#if DCHECK_IS_ON()
++mutex_.recursion_count_; ++mutex_.recursion_count_;
#endif
} }
void ThreadCondition::Signal() { void ThreadCondition::Signal() {
......
...@@ -122,7 +122,7 @@ MutexBase::~MutexBase() { ...@@ -122,7 +122,7 @@ MutexBase::~MutexBase() {
void MutexBase::lock() { void MutexBase::lock() {
EnterCriticalSection(&mutex_.internal_mutex_); EnterCriticalSection(&mutex_.internal_mutex_);
DCHECK(!mutex_.recursion_count_) CHECK(!mutex_.recursion_count_)
<< "WTF does not support recursive mutex acquisition!"; << "WTF does not support recursive mutex acquisition!";
++mutex_.recursion_count_; ++mutex_.recursion_count_;
} }
...@@ -149,7 +149,7 @@ bool Mutex::TryLock() { ...@@ -149,7 +149,7 @@ bool Mutex::TryLock() {
// check in the lock method (presumably due to performance?). This // check in the lock method (presumably due to performance?). This
// means lock() will succeed even if the current thread has already // means lock() will succeed even if the current thread has already
// entered the critical section. // entered the critical section.
DCHECK(!mutex_.recursion_count_) CHECK(!mutex_.recursion_count_)
<< "WTF does not support recursive mutex acquisition!"; << "WTF does not support recursive mutex acquisition!";
if (mutex_.recursion_count_ > 0) { if (mutex_.recursion_count_ > 0) {
LeaveCriticalSection(&mutex_.internal_mutex_); LeaveCriticalSection(&mutex_.internal_mutex_);
...@@ -169,7 +169,7 @@ bool RecursiveMutex::TryLock() { ...@@ -169,7 +169,7 @@ bool RecursiveMutex::TryLock() {
if (result == 0) { // We didn't get the lock. if (result == 0) { // We didn't get the lock.
return false; return false;
} }
DCHECK(!mutex_.recursion_count_) CHECK(!mutex_.recursion_count_)
<< "WTF does not support recursive mutex acquisition!"; << "WTF does not support recursive mutex acquisition!";
++mutex_.recursion_count_; ++mutex_.recursion_count_;
return true; return true;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment