Merge "Fix a deadlock caused by my big threading change yesterday." into dalvik-dev
diff --git a/src/mutex.cc b/src/mutex.cc
index 963ac99..f7c3143 100644
--- a/src/mutex.cc
+++ b/src/mutex.cc
@@ -27,14 +27,23 @@
namespace art {
-static inline void CheckRank(MutexRank rank, bool is_locking) {
+static inline void CheckSafeToLockOrUnlock(MutexRank rank, bool is_locking) {
#ifndef NDEBUG
if (rank == -1) {
return;
}
Thread* self = Thread::Current();
if (self != NULL) {
- self->CheckRank(rank, is_locking);
+ self->CheckSafeToLockOrUnlock(rank, is_locking);
+ }
+#endif
+}
+
+static inline void CheckSafeToWait(MutexRank rank) {
+#ifndef NDEBUG
+ Thread* self = Thread::Current();
+ if (self != NULL) {
+ self->CheckSafeToWait(rank);
}
#endif
}
@@ -58,8 +67,8 @@
}
void Mutex::Lock() {
+ CheckSafeToLockOrUnlock(rank_, true);
CHECK_MUTEX_CALL(pthread_mutex_lock, (&mutex_));
- CheckRank(rank_, true);
AssertHeld();
}
@@ -72,15 +81,15 @@
errno = result;
PLOG(FATAL) << "pthread_mutex_trylock failed for " << name_;
}
- CheckRank(rank_, true);
+ CheckSafeToLockOrUnlock(rank_, true);
AssertHeld();
return true;
}
void Mutex::Unlock() {
AssertHeld();
+ CheckSafeToLockOrUnlock(rank_, false);
CHECK_MUTEX_CALL(pthread_mutex_unlock, (&mutex_));
- CheckRank(rank_, false);
}
pid_t Mutex::GetOwner() {
@@ -151,6 +160,7 @@
}
void ConditionVariable::Wait(Mutex& mutex) {
+ CheckSafeToWait(mutex.GetRank());
CHECK_MUTEX_CALL(pthread_cond_wait, (&cond_, mutex.GetImpl()));
}
@@ -160,6 +170,7 @@
#else
#define TIMEDWAIT pthread_cond_timedwait
#endif
+ CheckSafeToWait(mutex.GetRank());
int rc = TIMEDWAIT(&cond_, mutex.GetImpl(), &ts);
if (rc != 0 && rc != ETIMEDOUT) {
errno = rc;
diff --git a/src/mutex.h b/src/mutex.h
index b4774d7..475c7dd 100644
--- a/src/mutex.h
+++ b/src/mutex.h
@@ -56,6 +56,10 @@
return &mutex_;
}
+ MutexRank GetRank() const {
+ return rank_;
+ }
+
void AssertHeld() {
#if !defined(__APPLE__)
DCHECK_EQ(GetOwner(), GetTid());
diff --git a/src/thread.cc b/src/thread.cc
index 1c521c1..dcaf01f 100644
--- a/src/thread.cc
+++ b/src/thread.cc
@@ -599,6 +599,12 @@
WalkStack(&dumper);
}
+void Thread::SetStateWithoutSuspendCheck(Thread::State new_state) {
+ volatile void* raw = reinterpret_cast<volatile void*>(&state_);
+ volatile int32_t* addr = reinterpret_cast<volatile int32_t*>(raw);
+ android_atomic_release_store(new_state, addr);
+}
+
Thread::State Thread::SetState(Thread::State new_state) {
Thread::State old_state = state_;
if (old_state == new_state) {
@@ -1674,7 +1680,7 @@
return os;
}
-void Thread::CheckRank(MutexRank rank, bool is_locking) {
+void Thread::CheckSafeToLockOrUnlock(MutexRank rank, bool is_locking) {
if (is_locking) {
if (held_mutexes_[rank] == 0) {
bool bad_mutexes_held = false;
@@ -1693,4 +1699,19 @@
}
}
+void Thread::CheckSafeToWait(MutexRank rank) {
+ bool bad_mutexes_held = false;
+ for (int i = kMaxMutexRank; i >= 0; --i) {
+ if (i != rank && held_mutexes_[i] != 0) {
+ LOG(ERROR) << "holding " << static_cast<MutexRank>(i) << " while doing condition variable wait on " << rank;
+ bad_mutexes_held = true;
+ }
+ }
+ if (held_mutexes_[rank] == 0) {
+ LOG(ERROR) << "*not* holding " << rank << " while doing condition variable wait on it";
+ bad_mutexes_held = true;
+ }
+ CHECK(!bad_mutexes_held);
+}
+
} // namespace art
diff --git a/src/thread.h b/src/thread.h
index 02fa87c..7ad433c 100644
--- a/src/thread.h
+++ b/src/thread.h
@@ -122,6 +122,7 @@
}
State SetState(State new_state);
+ void SetStateWithoutSuspendCheck(State new_state);
bool IsDaemon();
bool IsSuspended();
@@ -422,7 +423,8 @@
return frame;
}
- void CheckRank(MutexRank rank, bool is_locking);
+ void CheckSafeToLockOrUnlock(MutexRank rank, bool is_locking);
+ void CheckSafeToWait(MutexRank rank);
private:
Thread();
diff --git a/src/thread_list.cc b/src/thread_list.cc
index 3f8789a..e780e2c 100644
--- a/src/thread_list.cc
+++ b/src/thread_list.cc
@@ -33,8 +33,13 @@
// Self may be null during shutdown, but in that case there's no point going to kVmWait.
thread_list->thread_list_lock_.Lock();
} else {
- ScopedThreadStateChange tsc(self, Thread::kVmWait);
+ Thread::State old_thread_state = self->SetState(Thread::kVmWait);
thread_list->thread_list_lock_.Lock();
+ // If we have the lock, by definition there's no GC in progress (though we
+ // might be taking the lock in order to start one). We avoid the suspend
+ // check here so we don't risk going to sleep on the thread suspend count lock
+ // while holding the thread list lock.
+ self->SetStateWithoutSuspendCheck(old_thread_state);
}
}
}
