diff options
author | Mathieu Chartier <mathieuc@google.com> | 2014-04-10 18:06:32 -0700 |
---|---|---|
committer | Mathieu Chartier <mathieuc@google.com> | 2014-04-10 18:07:46 -0700 |
commit | c7a966dbba6902618ff0959d604c05d7570df8c8 (patch) | |
tree | 9c29dc008ad24e97ba26d5734fc7db4a41f7ca52 | |
parent | 22839a631bb3591a1c0037c388d51a4f18b5fb5e (diff) | |
download | art-c7a966dbba6902618ff0959d604c05d7570df8c8.zip art-c7a966dbba6902618ff0959d604c05d7570df8c8.tar.gz art-c7a966dbba6902618ff0959d604c05d7570df8c8.tar.bz2 |
Prevent deadlocks in Runtime::Abort.
If we have the threads suspended, attempting to use a
ScopedObjectAccess causes a deadlock. We now specifically avoid this
to prevent deadlocks.
Bug: 13747880
Change-Id: I45fd3fff917da98b22970e5351a9e25b143a4eed
-rw-r--r-- | runtime/runtime.cc | 33 | ||||
-rw-r--r-- | runtime/thread.cc | 4 |
2 files changed, 23 insertions, 14 deletions
diff --git a/runtime/runtime.cc b/runtime/runtime.cc index a19fa53..5c31d35 100644 --- a/runtime/runtime.cc +++ b/runtime/runtime.cc @@ -188,7 +188,7 @@ Runtime::~Runtime() { } struct AbortState { - void Dump(std::ostream& os) { + void Dump(std::ostream& os) NO_THREAD_SAFETY_ANALYSIS { if (gAborting > 1) { os << "Runtime aborting --- recursively, so no thread-specific detail!\n"; return; @@ -200,26 +200,33 @@ struct AbortState { return; } Thread* self = Thread::Current(); - if (self == NULL) { + if (self == nullptr) { os << "(Aborting thread was not attached to runtime!)\n"; } else { - // TODO: we're aborting and the ScopedObjectAccess may attempt to acquire the mutator_lock_ - // which may block indefinitely if there's a misbehaving thread holding it exclusively. - // The code below should be made robust to this. - ScopedObjectAccess soa(self); os << "Aborting thread:\n"; - self->Dump(os); - if (self->IsExceptionPending()) { - ThrowLocation throw_location; - mirror::Throwable* exception = self->GetException(&throw_location); - os << "Pending exception " << PrettyTypeOf(exception) - << " thrown by '" << throw_location.Dump() << "'\n" - << exception->Dump(); + if (Locks::mutator_lock_->IsExclusiveHeld(self) || Locks::mutator_lock_->IsSharedHeld(self)) { + DumpThread(os, self); + } else { + if (Locks::mutator_lock_->SharedTryLock(self)) { + DumpThread(os, self); + Locks::mutator_lock_->SharedUnlock(self); + } } } DumpAllThreads(os, self); } + void DumpThread(std::ostream& os, Thread* self) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) { + self->Dump(os); + if (self->IsExceptionPending()) { + ThrowLocation throw_location; + mirror::Throwable* exception = self->GetException(&throw_location); + os << "Pending exception " << PrettyTypeOf(exception) + << " thrown by '" << throw_location.Dump() << "'\n" + << exception->Dump(); + } + } + void DumpAllThreads(std::ostream& os, Thread* self) NO_THREAD_SAFETY_ANALYSIS { bool tll_already_held = Locks::thread_list_lock_->IsExclusiveHeld(self); bool ml_already_held = Locks::mutator_lock_->IsSharedHeld(self); diff --git a/runtime/thread.cc b/runtime/thread.cc index 5a2410a..131e2b6 100644 --- a/runtime/thread.cc +++ b/runtime/thread.cc @@ -711,7 +711,9 @@ void Thread::DumpState(std::ostream& os, const Thread* thread, pid_t tid) { bool is_daemon = false; Thread* self = Thread::Current(); - if (self != nullptr && thread != nullptr && thread->tlsPtr_.opeer != nullptr) { + // Don't do this if we are aborting since the GC may have all the threads suspended. This will + // cause ScopedObjectAccessUnchecked to deadlock. + if (gAborting == 0 && self != nullptr && thread != nullptr && thread->tlsPtr_.opeer != nullptr) { ScopedObjectAccessUnchecked soa(self); priority = soa.DecodeField(WellKnownClasses::java_lang_Thread_priority) ->GetInt(thread->tlsPtr_.opeer); |