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

Revert "WTF: CHECK if a mutex is recursively acquired."

This reverts commit e41ffb6c.

Reason for revert: WebAudio can still do recursive acquisition due to finalizing objects during an allocation in which the lock is held. See crbug.com/882314

Original change's description:
> 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/1212453
> Reviewed-by: Kentaro Hara <haraken@chromium.org>
> Commit-Queue: Jeremy Roman <jbroman@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#589504}

TBR=jbroman@chromium.org,yutak@chromium.org,haraken@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 856641,882314
Change-Id: I1e2732ee0c810d5fed73ba29874088abaeed2d59
Reviewed-on: https://chromium-review.googlesource.com/1225297Reviewed-by: default avatarJeremy Roman <jbroman@chromium.org>
Commit-Queue: Jeremy Roman <jbroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#591130}
parent 30425ef0
...@@ -56,7 +56,9 @@ typedef CONDITION_VARIABLE PlatformCondition; ...@@ -56,7 +56,9 @@ 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,7 +70,9 @@ MutexBase::MutexBase(bool recursive) { ...@@ -70,7 +70,9 @@ 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);
} }
...@@ -83,14 +85,18 @@ MutexBase::~MutexBase() { ...@@ -83,14 +85,18 @@ 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);
CHECK(!mutex_.recursion_count_) #if DCHECK_IS_ON()
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);
} }
...@@ -103,11 +109,13 @@ void MutexBase::unlock() { ...@@ -103,11 +109,13 @@ 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.
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)
...@@ -120,9 +128,11 @@ bool Mutex::TryLock() { ...@@ -120,9 +128,11 @@ 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) {
CHECK(!mutex_.recursion_count_) #if DCHECK_IS_ON()
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)
...@@ -142,10 +152,14 @@ ThreadCondition::~ThreadCondition() { ...@@ -142,10 +152,14 @@ 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_);
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_;
} }
...@@ -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.
CHECK(!mutex_.recursion_count_) DCHECK(!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;
} }
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_;
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