Fix a heap lock/thread list lock deadlock.
We had an uncaught OOME whose uncaught exception handler -- running
with the thread lock held -- was trying to cause a GC while some
other thread had the heap lock and was waiting for the thread list
lock.
Change-Id: I22177129562268837127d9edcc63ef5e93054bdf
diff --git a/build/Android.oattest.mk b/build/Android.oattest.mk
index 28cd958..36475c3 100644
--- a/build/Android.oattest.mk
+++ b/build/Android.oattest.mk
@@ -85,6 +85,6 @@
# $(eval $(call declare-test-test-target,StackWalk2,))
# TODO: Enable when ParallelGC does not cause hang
-# $(eval $(call declare-test-test-target,ParallelGC,))
+$(eval $(call declare-test-test-target,ParallelGC,))
########################################################################
diff --git a/src/heap.cc b/src/heap.cc
index 73b6640..d9db50b 100644
--- a/src/heap.cc
+++ b/src/heap.cc
@@ -584,6 +584,10 @@
SetIdealFootprint(target_size);
}
+uint32_t Heap::GetLockOwner() {
+ return lock_->GetOwner();
+}
+
void Heap::Lock() {
// Grab the lock, but put ourselves into Thread::kVmWait if it looks
// like we're going to have to wait on the mutex. This prevents
diff --git a/src/heap.h b/src/heap.h
index ffe2abf..ea4901d 100644
--- a/src/heap.h
+++ b/src/heap.h
@@ -100,8 +100,8 @@
// Blocks the caller until the garbage collector becomes idle.
static void WaitForConcurrentGcToComplete();
+ static uint32_t GetLockOwner(); // For SignalCatcher.
static void Lock();
-
static void Unlock();
static const std::vector<Space*>& GetSpaces() {
diff --git a/src/mutex.h b/src/mutex.h
index 37d5510..eb90565 100644
--- a/src/mutex.h
+++ b/src/mutex.h
@@ -52,9 +52,10 @@
DCHECK_NE(GetOwner(), GetTid());
}
- private:
pid_t GetOwner();
- pid_t GetTid();
+
+ private:
+ static pid_t GetTid();
std::string name_;
diff --git a/src/signal_catcher.cc b/src/signal_catcher.cc
index a129ba1..06491e4 100644
--- a/src/signal_catcher.cc
+++ b/src/signal_catcher.cc
@@ -62,7 +62,13 @@
}
void SignalCatcher::HandleSigQuit() {
- Runtime::Current()->GetThreadList()->SuspendAll();
+ Runtime* runtime = Runtime::Current();
+ ThreadList* thread_list = runtime->GetThreadList();
+
+ LOG(INFO) << "Heap lock owner: " << Heap::GetLockOwner() << "\n"
+ << "Thread lock owner: " << thread_list->GetLockOwner() << "\n";
+
+ thread_list->SuspendAll();
std::ostringstream os;
os << "\n"
@@ -75,7 +81,7 @@
os << "Cmd line: " << cmdline << "\n";
}
- Runtime::Current()->Dump(os);
+ runtime->Dump(os);
std::string maps;
if (ReadFileToString("/proc/self/maps", &maps)) {
@@ -84,7 +90,7 @@
os << "----- end " << getpid() << " -----";
- Runtime::Current()->GetThreadList()->ResumeAll();
+ thread_list->ResumeAll();
LOG(INFO) << os.str();
}
diff --git a/src/thread.cc b/src/thread.cc
index d68d1db..af6c29d 100644
--- a/src/thread.cc
+++ b/src/thread.cc
@@ -751,34 +751,9 @@
}
if (peer_ != NULL) {
- Object* group = gThread_group->GetObject(peer_);
-
- // Handle any pending exception.
- if (IsExceptionPending()) {
- // Get and clear the exception.
- Object* exception = GetException();
- ClearException();
-
- // If the thread has its own handler, use that.
- Object* handler = gThread_uncaughtHandler->GetObject(peer_);
- if (handler == NULL) {
- // Otherwise use the thread group's default handler.
- handler = group;
- }
-
- // Call the handler.
- Method* m = handler->GetClass()->FindVirtualMethodForVirtualOrInterface(gUncaughtExceptionHandler_uncaughtException);
- Object* args[2];
- args[0] = peer_;
- args[1] = exception;
- m->Invoke(this, handler, reinterpret_cast<byte*>(&args), NULL);
-
- // If the handler threw, clear that exception too.
- ClearException();
- }
-
// this.group.removeThread(this);
// group can be null if we're in the compiler or a test.
+ Object* group = gThread_group->GetObject(peer_);
if (group != NULL) {
Method* m = group->GetClass()->FindVirtualMethodForVirtualOrInterface(gThreadGroup_removeThread);
Object* args = peer_;
@@ -816,6 +791,35 @@
delete long_jump_context_;
}
+void Thread::HandleUncaughtExceptions() {
+ if (!IsExceptionPending()) {
+ return;
+ }
+
+ ScopedThreadStateChange tsc(this, Thread::kRunnable);
+
+ // Get and clear the exception.
+ Object* exception = GetException();
+ ClearException();
+
+ // If the thread has its own handler, use that.
+ Object* handler = gThread_uncaughtHandler->GetObject(peer_);
+ if (handler == NULL) {
+ // Otherwise use the thread group's default handler.
+ handler = gThread_group->GetObject(peer_);
+ }
+
+ // Call the handler.
+ Method* m = handler->GetClass()->FindVirtualMethodForVirtualOrInterface(gUncaughtExceptionHandler_uncaughtException);
+ Object* args[2];
+ args[0] = peer_;
+ args[1] = exception;
+ m->Invoke(this, handler, reinterpret_cast<byte*>(&args), NULL);
+
+ // If the handler threw, clear that exception too.
+ ClearException();
+}
+
size_t Thread::NumSirtReferences() {
size_t count = 0;
for (StackIndirectReferenceTable* cur = top_sirt_; cur; cur = cur->Link()) {
@@ -1186,13 +1190,22 @@
msg, (throwing_OutOfMemoryError_ ? " (recursive case)" : ""));
if (!throwing_OutOfMemoryError_) {
throwing_OutOfMemoryError_ = true;
- ThrowNewException("Ljava/lang/OutOfMemoryError;", msg);
+ ThrowNewException("Ljava/lang/OutOfMemoryError;", NULL);
} else {
SetException(pre_allocated_OutOfMemoryError_);
}
throwing_OutOfMemoryError_ = false;
}
+
+Thread* Thread::CurrentFromGdb() const {
+ return Thread::Current();
+}
+
+void Thread::DumpFromGdb() const {
+ Dump(std::cerr);
+}
+
class CatchBlockStackVisitor : public Thread::StackVisitor {
public:
CatchBlockStackVisitor(Class* to_find, Context* ljc)
diff --git a/src/thread.h b/src/thread.h
index a7097ce..112a1ef 100644
--- a/src/thread.h
+++ b/src/thread.h
@@ -457,9 +457,15 @@
void DumpState(std::ostream& os) const;
void DumpStack(std::ostream& os) const;
+ // Out-of-line conveniences for debugging in gdb.
+ Thread* CurrentFromGdb() const; // Like Thread::Current.
+ void DumpFromGdb() const; // Like Thread::Dump(std::cerr).
+
void Attach(const Runtime* runtime);
static void* CreateCallback(void* arg);
+ void HandleUncaughtExceptions();
+
void InitCpu();
void InitFunctionPointers();
void InitTid();
diff --git a/src/thread_list.cc b/src/thread_list.cc
index 7b6d3ba..77ee31a 100644
--- a/src/thread_list.cc
+++ b/src/thread_list.cc
@@ -43,6 +43,10 @@
return find(list_.begin(), list_.end(), thread) != list_.end();
}
+uint32_t ThreadList::GetLockOwner() {
+ return thread_list_lock_.GetOwner();
+}
+
void ThreadList::Dump(std::ostream& os) {
MutexLock mu(thread_list_lock_);
os << "DALVIK THREADS (" << list_.size() << "):\n";
@@ -267,6 +271,10 @@
LOG(INFO) << "ThreadList::Unregister() " << *self;
}
+ // This may need to call user-supplied managed code. Make sure we do this before we start tearing
+ // down the Thread* and removing it from the thread list (or start taking any locks).
+ self->HandleUncaughtExceptions();
+
MutexLock mu(thread_list_lock_);
// Remove this thread from the list.
diff --git a/src/thread_list.h b/src/thread_list.h
index 35b23e7..f5e1a3a 100644
--- a/src/thread_list.h
+++ b/src/thread_list.h
@@ -32,6 +32,7 @@
~ThreadList();
void Dump(std::ostream& os);
+ uint32_t GetLockOwner(); // For SignalCatcher.
// Thread suspension support.
void FullSuspendCheck(Thread* thread);
diff --git a/test/ParallelGC/ParallelGC.java b/test/ParallelGC/ParallelGC.java
index 13cb603..eb9e04e 100644
--- a/test/ParallelGC/ParallelGC.java
+++ b/test/ParallelGC/ParallelGC.java
@@ -41,9 +41,7 @@
List l = new ArrayList();
for (int i = 0; i < 1000; i++) {
l.add(new ArrayList(i));
- if (false) {
- System.out.print(id);
- }
+ System.out.print(id);
}
}
}