Make lock debug checks optional.
Move all locking checks behing kDebugLocking. Disable locking checks
when not DEBUG. Will enable later so that we can get some idea of the
performance overhead. We want locking checks to be the default for a
while to make sure we solve any remaining potential deadlock issues.
Change-Id: I4d7622750cc29f9933c84259796de5a2df02b783
diff --git a/src/atomic.cc b/src/atomic.cc
index b923f91..fb4de9c 100644
--- a/src/atomic.cc
+++ b/src/atomic.cc
@@ -17,6 +17,7 @@
#include "atomic.h"
#include <pthread.h>
+#include <vector>
#include "mutex.h"
#include "stl_util.h"
diff --git a/src/mutex.cc b/src/mutex.cc
index cb344d4..c066bec 100644
--- a/src/mutex.cc
+++ b/src/mutex.cc
@@ -118,10 +118,12 @@
static void CheckUnattachedThread(MutexLevel level) {
// The check below enumerates the cases where we expect not to be able to sanity check locks
// on a thread. TODO: tighten this check.
- Runtime* runtime = Runtime::Current();
- CHECK(runtime == NULL || !runtime->IsStarted() || runtime->IsShuttingDown() ||
- level == kDefaultMutexLevel || level == kThreadListLock ||
- level == kLoggingLock || level == kAbortLock);
+ if (kDebugLocking) {
+ Runtime* runtime = Runtime::Current();
+ CHECK(runtime == NULL || !runtime->IsStarted() || runtime->IsShuttingDown() ||
+ level == kDefaultMutexLevel || level == kThreadListLock ||
+ level == kLoggingLock || level == kAbortLock);
+ }
}
void BaseMutex::RegisterAsLockedWithCurrentThread() {
@@ -130,20 +132,22 @@
CheckUnattachedThread(level_);
return;
}
- // Check if a bad Mutex of this level or lower is held.
- bool bad_mutexes_held = false;
- for (int i = level_; i >= 0; --i) {
- BaseMutex* held_mutex = self->GetHeldMutex(static_cast<MutexLevel>(i));
- if (UNLIKELY(held_mutex != NULL)) {
- LOG(ERROR) << "Lock level violation: holding \"" << held_mutex->name_ << "\" (level " << i
- << ") while locking \"" << name_ << "\" (level " << static_cast<int>(level_) << ")";
- if (i > kAbortLock) {
- // Only abort in the check below if this is more than abort level lock.
- bad_mutexes_held = true;
+ if (kDebugLocking) {
+ // Check if a bad Mutex of this level or lower is held.
+ bool bad_mutexes_held = false;
+ for (int i = level_; i >= 0; --i) {
+ BaseMutex* held_mutex = self->GetHeldMutex(static_cast<MutexLevel>(i));
+ if (UNLIKELY(held_mutex != NULL)) {
+ LOG(ERROR) << "Lock level violation: holding \"" << held_mutex->name_ << "\" (level " << i
+ << ") while locking \"" << name_ << "\" (level " << static_cast<int>(level_) << ")";
+ if (i > kAbortLock) {
+ // Only abort in the check below if this is more than abort level lock.
+ bad_mutexes_held = true;
+ }
}
}
+ CHECK(!bad_mutexes_held);
}
- CHECK(!bad_mutexes_held);
// Don't record monitors as they are outside the scope of analysis. They may be inspected off of
// the monitor list.
if (level_ != kMonitorLock) {
@@ -158,7 +162,9 @@
return;
}
if (level_ != kMonitorLock) {
- CHECK(self->GetHeldMutex(level_) == this) << "Unlocking on unacquired mutex: " << name_;
+ if (kDebugLocking) {
+ CHECK(self->GetHeldMutex(level_) == this) << "Unlocking on unacquired mutex: " << name_;
+ }
self->SetHeldMutex(level_, NULL);
}
}
@@ -169,20 +175,22 @@
CheckUnattachedThread(level_);
return;
}
- CHECK(self->GetHeldMutex(level_) == this) << "Waiting on unacquired mutex: " << name_;
- bool bad_mutexes_held = false;
- for (int i = kMaxMutexLevel; i >= 0; --i) {
- if (i != level_) {
- BaseMutex* held_mutex = self->GetHeldMutex(static_cast<MutexLevel>(i));
- if (held_mutex != NULL) {
- LOG(ERROR) << "Holding " << held_mutex->name_ << " (level " << i
- << ") while performing wait on: "
- << name_ << " (level " << static_cast<int>(level_) << ")";
- bad_mutexes_held = true;
+ if (kDebugLocking) {
+ CHECK(self->GetHeldMutex(level_) == this) << "Waiting on unacquired mutex: " << name_;
+ bool bad_mutexes_held = false;
+ for (int i = kMaxMutexLevel; i >= 0; --i) {
+ if (i != level_) {
+ BaseMutex* held_mutex = self->GetHeldMutex(static_cast<MutexLevel>(i));
+ if (held_mutex != NULL) {
+ LOG(ERROR) << "Holding " << held_mutex->name_ << " (level " << i
+ << ") while performing wait on: "
+ << name_ << " (level " << static_cast<int>(level_) << ")";
+ bad_mutexes_held = true;
+ }
}
}
+ CHECK(!bad_mutexes_held);
}
- CHECK(!bad_mutexes_held);
}
Mutex::Mutex(const char* name, MutexLevel level, bool recursive)
@@ -213,24 +221,26 @@
}
void Mutex::ExclusiveLock() {
- bool is_held = IsExclusiveHeld();
- CHECK(recursive_ || !is_held)
- << "Error attempt to recursively lock non-recursive lock \"" << name_ << "\"";
- if (!is_held) {
+ if (kDebugLocking && !recursive_) {
+ AssertNotHeld();
+ }
+ if (!recursive_ || !IsExclusiveHeld()) {
CHECK_MUTEX_CALL(pthread_mutex_lock, (&mutex_));
RegisterAsLockedWithCurrentThread();
}
recursion_count_++;
- DCHECK(recursion_count_ == 1 || recursive_) << "Unexpected recursion count on mutex: "
- << name_ << " " << recursion_count_;
- AssertHeld();
+ if (kDebugLocking) {
+ CHECK(recursion_count_ == 1 || recursive_) << "Unexpected recursion count on mutex: "
+ << name_ << " " << recursion_count_;
+ AssertHeld();
+ }
}
bool Mutex::ExclusiveTryLock() {
- bool is_held = IsExclusiveHeld();
- CHECK(recursive_ || !is_held)
- << "Error attempt to recursively lock non-recursive lock \"" << name_ << "\"";
- if (!is_held) {
+ if (kDebugLocking && !recursive_) {
+ AssertNotHeld();
+ }
+ if (!recursive_ || !IsExclusiveHeld()) {
int result = pthread_mutex_trylock(&mutex_);
if (result == EBUSY) {
return false;
@@ -242,7 +252,11 @@
RegisterAsLockedWithCurrentThread();
}
recursion_count_++;
- AssertHeld();
+ if (kDebugLocking) {
+ CHECK(recursion_count_ == 1 || recursive_) << "Unexpected recursion count on mutex: "
+ << name_ << " " << recursion_count_;
+ AssertHeld();
+ }
return true;
}
@@ -250,8 +264,10 @@
AssertHeld();
recursion_count_--;
if (!recursive_ || recursion_count_ == 0) {
- DCHECK(recursion_count_ == 0 || recursive_) << "Unexpected recursion count on mutex: "
- << name_ << " " << recursion_count_;
+ if (kDebugLocking) {
+ CHECK(recursion_count_ == 0 || recursive_) << "Unexpected recursion count on mutex: "
+ << name_ << " " << recursion_count_;
+ }
RegisterAsUnlockedWithCurrentThread();
CHECK_MUTEX_CALL(pthread_mutex_unlock, (&mutex_));
}
@@ -265,7 +281,9 @@
} else {
result = (self->GetHeldMutex(level_) == this);
// Sanity debug check that if we think it is locked, so does the pthread.
- DCHECK(result == (GetExclusiveOwnerTid() == static_cast<uint64_t>(GetTid())));
+ if (kDebugLocking) {
+ CHECK(result == (GetExclusiveOwnerTid() == static_cast<uint64_t>(GetTid())));
+ }
}
return result;
}
@@ -364,9 +382,11 @@
bool ReaderWriterMutex::IsExclusiveHeld() const {
bool result = (GetExclusiveOwnerTid() == static_cast<uint64_t>(GetTid()));
- // Sanity that if the pthread thinks we own the lock the Thread agrees.
- Thread* self = Thread::Current();
- DCHECK((self == NULL) || !result || (self->GetHeldMutex(level_) == this));
+ if (kDebugLocking) {
+ // Sanity that if the pthread thinks we own the lock the Thread agrees.
+ Thread* self = Thread::Current();
+ CHECK((self == NULL) || !result || (self->GetHeldMutex(level_) == this));
+ }
return result;
}
diff --git a/src/mutex.h b/src/mutex.h
index 5154d45..85d75ab 100644
--- a/src/mutex.h
+++ b/src/mutex.h
@@ -24,7 +24,6 @@
#include <string>
#include "globals.h"
-#include "gtest/gtest.h"
#include "logging.h"
#include "macros.h"
@@ -37,6 +36,8 @@
namespace art {
+const bool kDebugLocking = kIsDebugBuild;
+
class LOCKABLE Mutex;
class LOCKABLE ReaderWriterMutex;
@@ -216,7 +217,7 @@
// Assert that the Mutex is exclusively held by the current thread.
void AssertExclusiveHeld() {
- if (kIsDebugBuild) {
+ if (kDebugLocking) {
CHECK(IsExclusiveHeld());
}
}
@@ -224,7 +225,7 @@
// Assert that the Mutex is not held by the current thread.
void AssertNotHeldExclusive() {
- if (kIsDebugBuild) {
+ if (kDebugLocking) {
CHECK(!IsExclusiveHeld());
}
}
@@ -300,7 +301,7 @@
// Assert the current thread has exclusive access to the ReaderWriterMutex.
void AssertExclusiveHeld() {
- if (kIsDebugBuild) {
+ if (kDebugLocking) {
CHECK(IsExclusiveHeld());
}
}
@@ -308,7 +309,7 @@
// Assert the current thread doesn't have exclusive access to the ReaderWriterMutex.
void AssertNotExclusiveHeld() {
- if (kIsDebugBuild) {
+ if (kDebugLocking) {
CHECK(!IsExclusiveHeld());
}
}
@@ -319,7 +320,7 @@
// Assert the current thread has shared access to the ReaderWriterMutex.
void AssertSharedHeld() {
- if (kIsDebugBuild) {
+ if (kDebugLocking) {
CHECK(IsSharedHeld());
}
}
@@ -328,7 +329,7 @@
// Assert the current thread doesn't hold this ReaderWriterMutex either in shared or exclusive
// mode.
void AssertNotHeld() {
- if (kIsDebugBuild) {
+ if (kDebugLocking) {
CHECK(!IsSharedHeld());
}
}