commit a77f0f68a59870ff027dad654fe31eb3fdaf3d5d
author jeffhao <jeffhao@google.com> Wed Dec 05 17:19:31 2012 -0800
committer jeffhao <jeffhao@google.com> Thu Dec 06 10:14:46 2012 -0800
tree c041a68efb55db65ab8f195938097a887f5fcc01
parent a169e1d8d968800380245cda450505d969ebff76

Various fixes for JDWP.

- Moved lock of thread list lock into DecodeThread from its callers
- Fixed scope of various locks to prevent locking violations
- Added transition for current thread from runnable to suspended before
  suspending vm, and then a transition back
- Reworked lock ordering to allow JDWP locks to be held while grabbing
  the thread list lock
- Moved debugger PostException until after suspension is re-allowed

Change-Id: Ie53e47ff1538e6cd3125c48ddb4c13758b29be63

src/debugger.cc

diff --git a/src/debugger.cc b/src/debugger.cc
index 672b660..87e9c72 100644
--- a/src/debugger.cc
+++ b/src/debugger.cc
@@ -224,6 +224,7 @@
 }
 
 static Thread* DecodeThread(ScopedObjectAccessUnchecked& soa, JDWP::ObjectId threadId)
+    EXCLUSIVE_LOCKS_REQUIRED(Locks::thread_list_lock_)
     LOCKS_EXCLUDED(Locks::thread_suspend_count_lock_)
     SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
   Object* thread_peer = gRegistry->Get<Object*>(threadId);
@@ -1334,9 +1335,8 @@
 }
 
 bool Dbg::GetThreadName(JDWP::ObjectId threadId, std::string& name) {
-  Thread* self = Thread::Current();
-  MutexLock mu(self, *Locks::thread_list_lock_);
-  ScopedObjectAccessUnchecked soa(self);
+  ScopedObjectAccessUnchecked soa(Thread::Current());
+  MutexLock mu(soa.Self(), *Locks::thread_list_lock_);
   Thread* thread = DecodeThread(soa, threadId);
   if (thread == NULL) {
     return false;
@@ -1581,6 +1581,7 @@
 
 int Dbg::GetThreadFrameCount(JDWP::ObjectId threadId) {
   ScopedObjectAccess soa(Thread::Current());
+  MutexLock mu(soa.Self(), *Locks::thread_list_lock_);
   return GetStackDepth(DecodeThread(soa, threadId));
 }
 
@@ -1624,6 +1625,7 @@
   };
 
   ScopedObjectAccessUnchecked soa(Thread::Current());
+  MutexLock mu(soa.Self(), *Locks::thread_list_lock_);
   Thread* thread = DecodeThread(soa, thread_id);  // Caller already checked thread is suspended.
   GetFrameVisitor visitor(thread->GetManagedStack(), thread->GetInstrumentationStack(), start_frame, frame_count, buf);
   visitor.WalkStack();
@@ -1669,8 +1671,11 @@
 void Dbg::ResumeThread(JDWP::ObjectId threadId) {
   ScopedObjectAccessUnchecked soa(Thread::Current());
   Object* peer = gRegistry->Get<Object*>(threadId);
-  MutexLock mu(soa.Self(), *Locks::thread_list_lock_);
-  Thread* thread = Thread::FromManagedThread(soa, peer);
+  Thread* thread;
+  {
+    MutexLock mu(soa.Self(), *Locks::thread_list_lock_);
+    thread = Thread::FromManagedThread(soa, peer);
+  }
   if (thread == NULL) {
     LOG(WARNING) << "No such thread for resume: " << peer;
     return;
@@ -1878,6 +1883,7 @@
   };
 
   ScopedObjectAccessUnchecked soa(Thread::Current());
+  MutexLock mu(soa.Self(), *Locks::thread_list_lock_);
   Thread* thread = DecodeThread(soa, threadId);
   UniquePtr<Context> context(Context::Create());
   GetLocalVisitor visitor(thread->GetManagedStack(), thread->GetInstrumentationStack(), context.get(),
@@ -1961,6 +1967,7 @@
   };
 
   ScopedObjectAccessUnchecked soa(Thread::Current());
+  MutexLock mu(soa.Self(), *Locks::thread_list_lock_);
   Thread* thread = DecodeThread(soa, threadId);
   UniquePtr<Context> context(Context::Create());
   SetLocalVisitor visitor(thread->GetManagedStack(), thread->GetInstrumentationStack(), context.get(),
@@ -2165,12 +2172,13 @@
 JDWP::JdwpError Dbg::ConfigureStep(JDWP::ObjectId threadId, JDWP::JdwpStepSize step_size,
                                    JDWP::JdwpStepDepth step_depth) {
   ScopedObjectAccessUnchecked soa(Thread::Current());
+  MutexLock mu(soa.Self(), *Locks::thread_list_lock_);
   Thread* thread = DecodeThread(soa, threadId);
   if (thread == NULL) {
     return JDWP::ERR_INVALID_THREAD;
   }
 
-  MutexLock mu(soa.Self(), gBreakpointsLock);
+  MutexLock mu2(soa.Self(), gBreakpointsLock);
   // TODO: there's no theoretical reason why we couldn't support single-stepping
   // of multiple threads at once, but we never did so historically.
   if (gSingleStepControl.thread != NULL && thread != gSingleStepControl.thread) {