From 556217477768af1b2abf6768f007c09f226bbe7e Mon Sep 17 00:00:00 2001 From: Brian Carlstrom Date: Mon, 17 Oct 2011 00:41:56 -0700 Subject: Add ParallelGC to reproduce system_server hang Change-Id: Ie563663df5fc744755102719378446b9efa16cf3 --- test/ParallelGC/ParallelGC.java | 49 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 test/ParallelGC/ParallelGC.java (limited to 'test/ParallelGC/ParallelGC.java') diff --git a/test/ParallelGC/ParallelGC.java b/test/ParallelGC/ParallelGC.java new file mode 100644 index 0000000000..13cb603bdd --- /dev/null +++ b/test/ParallelGC/ParallelGC.java @@ -0,0 +1,49 @@ +/* + * Copyright (C) 2011 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import java.util.ArrayList; +import java.util.List; + +class ParallelGC implements Runnable { + public static void main(String[] args) throws Exception { + Thread[] threads = new Thread[16]; + for (int i = 0; i < threads.length; i++) { + threads[i] = new Thread(new ParallelGC(i)); + } + for (Thread thread : threads) { + thread.start(); + } + for (Thread thread : threads) { + thread.join(); + } + } + + private final int id; + + private ParallelGC(int id) { + this.id = id; + } + + public void run() { + List l = new ArrayList(); + for (int i = 0; i < 1000; i++) { + l.add(new ArrayList(i)); + if (false) { + System.out.print(id); + } + } + } +} -- cgit v1.2.3-59-g8ed1b From accd83d1523545ac69bafd38e72a7d5cff8e2fac Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Mon, 17 Oct 2011 14:25:58 -0700 Subject: 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 --- build/Android.oattest.mk | 2 +- src/heap.cc | 4 +++ src/heap.h | 2 +- src/mutex.h | 5 +-- src/signal_catcher.cc | 12 ++++++-- src/thread.cc | 67 ++++++++++++++++++++++++----------------- src/thread.h | 6 ++++ src/thread_list.cc | 8 +++++ src/thread_list.h | 1 + test/ParallelGC/ParallelGC.java | 4 +-- 10 files changed, 74 insertions(+), 37 deletions(-) (limited to 'test/ParallelGC/ParallelGC.java') diff --git a/build/Android.oattest.mk b/build/Android.oattest.mk index 28cd958d2d..36475c397c 100644 --- a/build/Android.oattest.mk +++ b/build/Android.oattest.mk @@ -85,6 +85,6 @@ $(eval $(call declare-test-test-target,StackWalk,)) # $(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 73b6640911..d9db50b405 100644 --- a/src/heap.cc +++ b/src/heap.cc @@ -584,6 +584,10 @@ void Heap::GrowForUtilization() { 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 ffe2abfea5..ea4901d043 100644 --- a/src/heap.h +++ b/src/heap.h @@ -100,8 +100,8 @@ class Heap { // 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& GetSpaces() { diff --git a/src/mutex.h b/src/mutex.h index 37d5510126..eb905656a9 100644 --- a/src/mutex.h +++ b/src/mutex.h @@ -52,9 +52,10 @@ class Mutex { 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 a129ba17a6..06491e4119 100644 --- a/src/signal_catcher.cc +++ b/src/signal_catcher.cc @@ -62,7 +62,13 @@ bool SignalCatcher::ShouldHalt() { } 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 @@ void SignalCatcher::HandleSigQuit() { 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 @@ void SignalCatcher::HandleSigQuit() { 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 d68d1db1a8..af6c29ddb8 100644 --- a/src/thread.cc +++ b/src/thread.cc @@ -751,34 +751,9 @@ Thread::~Thread() { } 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(&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 @@ Thread::~Thread() { 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(&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 @@ void Thread::ThrowOutOfMemoryError(const char* msg) { 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 a7097ce8a9..112a1ef812 100644 --- a/src/thread.h +++ b/src/thread.h @@ -457,9 +457,15 @@ class PACKED Thread { 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 7b6d3baec5..77ee31aaaf 100644 --- a/src/thread_list.cc +++ b/src/thread_list.cc @@ -43,6 +43,10 @@ bool ThreadList::Contains(Thread* thread) { 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 @@ void ThreadList::Unregister() { 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 35b23e7265..f5e1a3a028 100644 --- a/src/thread_list.h +++ b/src/thread_list.h @@ -32,6 +32,7 @@ class ThreadList { ~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 13cb603bdd..eb9e04e70a 100644 --- a/test/ParallelGC/ParallelGC.java +++ b/test/ParallelGC/ParallelGC.java @@ -41,9 +41,7 @@ class ParallelGC implements Runnable { 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); } } } -- cgit v1.2.3-59-g8ed1b