Fix deadlock in VirtualMachine.AllThreads
We cannot add any object in the JDWP object registry while holding the
Locks::thread_list_lock. Indeed we may need to suspend a thread and take it,
causing a deadlock by waiting for ourself on this lock.
Bug: 17343664
(cherry picked from commit d35776413901a6a9d478e06dc354ea4f7d962e04)
Change-Id: I07d150b95a6d2b62c913bf2ca2ac217911b2f19d
diff --git a/runtime/debugger.cc b/runtime/debugger.cc
index c994f0b..d1e041c 100644
--- a/runtime/debugger.cc
+++ b/runtime/debugger.cc
@@ -2098,68 +2098,53 @@
return JDWP::ERR_NONE;
}
+static bool IsInDesiredThreadGroup(ScopedObjectAccessUnchecked& soa,
+ mirror::Object* desired_thread_group, mirror::Object* peer)
+ SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
+ // Do we want threads from all thread groups?
+ if (desired_thread_group == nullptr) {
+ return true;
+ }
+ mirror::ArtField* thread_group_field = soa.DecodeField(WellKnownClasses::java_lang_Thread_group);
+ DCHECK(thread_group_field != nullptr);
+ mirror::Object* group = thread_group_field->GetObject(peer);
+ return (group == desired_thread_group);
+}
+
void Dbg::GetThreads(JDWP::ObjectId thread_group_id, std::vector<JDWP::ObjectId>* thread_ids) {
- class ThreadListVisitor {
- public:
- ThreadListVisitor(const ScopedObjectAccessUnchecked& soa, mirror::Object* desired_thread_group,
- std::vector<JDWP::ObjectId>* thread_ids)
- SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
- : soa_(soa), desired_thread_group_(desired_thread_group), thread_ids_(thread_ids) {}
-
- static void Visit(Thread* t, void* arg) {
- reinterpret_cast<ThreadListVisitor*>(arg)->Visit(t);
- }
-
- // TODO: Enable annotalysis. We know lock is held in constructor, but abstraction confuses
- // annotalysis.
- void Visit(Thread* t) NO_THREAD_SAFETY_ANALYSIS {
- if (t == Dbg::GetDebugThread()) {
- // Skip the JDWP thread. Some debuggers get bent out of shape when they can't suspend and
- // query all threads, so it's easier if we just don't tell them about this thread.
- return;
- }
- if (t->IsStillStarting()) {
- // This thread is being started (and has been registered in the thread list). However, it is
- // not completely started yet so we must ignore it.
- return;
- }
- mirror::Object* peer = t->GetPeer();
- if (IsInDesiredThreadGroup(peer)) {
- thread_ids_->push_back(gRegistry->Add(peer));
- }
- }
-
- private:
- bool IsInDesiredThreadGroup(mirror::Object* peer)
- SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
- // peer might be nullptr if the thread is still starting up.
- if (peer == nullptr) {
- // We can't tell the debugger about this thread yet.
- // TODO: if we identified threads to the debugger by their Thread*
- // rather than their peer's mirror::Object*, we could fix this.
- // Doing so might help us report ZOMBIE threads too.
- return false;
- }
- // Do we want threads from all thread groups?
- if (desired_thread_group_ == nullptr) {
- return true;
- }
- mirror::Object* group = soa_.DecodeField(WellKnownClasses::java_lang_Thread_group)->GetObject(peer);
- return (group == desired_thread_group_);
- }
-
- const ScopedObjectAccessUnchecked& soa_;
- mirror::Object* const desired_thread_group_;
- std::vector<JDWP::ObjectId>* const thread_ids_;
- };
-
ScopedObjectAccessUnchecked soa(Thread::Current());
JDWP::JdwpError error;
mirror::Object* thread_group = gRegistry->Get<mirror::Object*>(thread_group_id, &error);
CHECK_EQ(error, JDWP::ERR_NONE);
- ThreadListVisitor tlv(soa, thread_group, thread_ids);
- MutexLock mu(soa.Self(), *Locks::thread_list_lock_);
- Runtime::Current()->GetThreadList()->ForEach(ThreadListVisitor::Visit, &tlv);
+ std::list<Thread*> all_threads_list;
+ {
+ MutexLock mu(Thread::Current(), *Locks::thread_list_lock_);
+ all_threads_list = Runtime::Current()->GetThreadList()->GetList();
+ }
+ for (Thread* t : all_threads_list) {
+ if (t == Dbg::GetDebugThread()) {
+ // Skip the JDWP thread. Some debuggers get bent out of shape when they can't suspend and
+ // query all threads, so it's easier if we just don't tell them about this thread.
+ continue;
+ }
+ if (t->IsStillStarting()) {
+ // This thread is being started (and has been registered in the thread list). However, it is
+ // not completely started yet so we must ignore it.
+ continue;
+ }
+ mirror::Object* peer = t->GetPeer();
+ if (peer == nullptr) {
+ // peer might be NULL if the thread is still starting up. We can't tell the debugger about
+ // this thread yet.
+ // TODO: if we identified threads to the debugger by their Thread*
+ // rather than their peer's mirror::Object*, we could fix this.
+ // Doing so might help us report ZOMBIE threads too.
+ continue;
+ }
+ if (IsInDesiredThreadGroup(soa, thread_group, peer)) {
+ thread_ids->push_back(gRegistry->Add(peer));
+ }
+ }
}
void Dbg::GetChildThreadGroups(JDWP::ObjectId thread_group_id,
diff --git a/runtime/jdwp/object_registry.h b/runtime/jdwp/object_registry.h
index f2f43c4..faddff1 100644
--- a/runtime/jdwp/object_registry.h
+++ b/runtime/jdwp/object_registry.h
@@ -63,7 +63,8 @@
JDWP::ObjectId Add(mirror::Object* o)
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) LOCKS_EXCLUDED(Locks::thread_list_lock_);
- JDWP::RefTypeId AddRefType(mirror::Class* c) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
+ JDWP::RefTypeId AddRefType(mirror::Class* c)
+ SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) LOCKS_EXCLUDED(Locks::thread_list_lock_);
template<typename T> T Get(JDWP::ObjectId id, JDWP::JdwpError* error)
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
