JDWP: fix deadlock with GC

This CL fixes a deadlock where JDWP thread and running GC thread are
waiting for each other. Here is the sequence of the deadlock:
1. GC thread disables access to weak global references, then releases
   mutator lock.
2. JDWP thread takes mutator lock exclusively after suspending all
   threads.
3. GC thread waits for shared mutator lock which is held by JDWP
   thread.
4. JDWP thread clears weak global references but need to wait for GC
   thread to re-enable access to them.

To avoid that situation, we ensure the JDWP thread does not attempt
to delete weak global references while holding the mutator
exclusively so GC thread is not blocked.

Bug: 18995321
Change-Id: Ia7e82f463c27ffdcfd730c3117337a6a33d111e7

2 files changed, 21 insertions, 3 deletions

diff --git a/runtime/debugger.cc b/runtime/debugger.cc
index fe1e3a4aa5..229a1af6b1 100644
--- a/runtime/debugger.cc
+++ b/runtime/debugger.cc
@@ -819,10 +819,15 @@ void Dbg::Disconnected() {
     }
     gDebuggerActive = false;
   }
-  gRegistry->Clear();
-  gDebuggerConnected = false;
   CHECK_EQ(self->SetStateUnsafe(old_state), kRunnable);
   runtime->GetThreadList()->ResumeAll();
+
+  {
+    ScopedObjectAccess soa(self);
+    gRegistry->Clear();
+  }
+
+  gDebuggerConnected = false;
 }
 
 bool Dbg::IsDebuggerActive() {
diff --git a/runtime/jdwp/object_registry.cc b/runtime/jdwp/object_registry.cc
index 20db368c21..e415c3d1cd 100644
--- a/runtime/jdwp/object_registry.cc
+++ b/runtime/jdwp/object_registry.cc
@@ -104,7 +104,20 @@ bool ObjectRegistry::ContainsLocked(Thread* self, mirror::Object* o, int32_t ide
 }
 
 void ObjectRegistry::Clear() {
-  Thread* self = Thread::Current();
+  Thread* const self = Thread::Current();
+
+  // We must not hold the mutator lock exclusively if we want to delete weak global
+  // references. Otherwise this can lead to a deadlock with a running GC:
+  // 1. GC thread disables access to weak global references, then releases
+  //    mutator lock.
+  // 2. JDWP thread takes mutator lock exclusively after suspending all
+  //    threads.
+  // 3. GC thread waits for shared mutator lock which is held by JDWP
+  //    thread.
+  // 4. JDWP thread clears weak global references but need to wait for GC
+  //    thread to re-enable access to them.
+  Locks::mutator_lock_->AssertNotExclusiveHeld(self);
+
   MutexLock mu(self, lock_);
   VLOG(jdwp) << "Object registry contained " << object_to_entry_.size() << " entries";
   // Delete all the JNI references.