commit f15f4a0136a7b797117355768851bc421b3afc93
author Elliott Hughes <enh@google.com> Wed Jan 09 10:09:38 2013 -0800
committer Elliott Hughes <enh@google.com> Wed Jan 09 10:28:56 2013 -0800
tree 160e0a242b91520ddd005af35680a40fab400449
parent 88d630950cb5c6a1cb6457ce03a17c074ae13628

Remove Dbg::IsSuspended, which was checking the wrong thing.

It was also unnecessarily racy.

Change-Id: I9d59ac81ffb5b178ca9d2a00d895a272321adec9

src/debugger.cc

diff --git a/src/debugger.cc b/src/debugger.cc
index deef322..85ebf66 100644
--- a/src/debugger.cc
+++ b/src/debugger.cc
@@ -1494,19 +1494,6 @@
   return JDWP::ERR_NONE;
 }
 
-JDWP::JdwpError Dbg::IsSuspended(JDWP::ObjectId thread_id, bool& result) {
-  ScopedObjectAccess soa(Thread::Current());
-  MutexLock mu(soa.Self(), *Locks::thread_list_lock_);
-  Thread* thread;
-  JDWP::JdwpError error = DecodeThread(soa, thread_id, thread);
-  if (error != JDWP::ERR_NONE) {
-    return error;
-  }
-  MutexLock mu2(soa.Self(), *Locks::thread_suspend_count_lock_);
-  result = thread->IsSuspended();
-  return JDWP::ERR_NONE;
-}
-
 void Dbg::GetThreads(JDWP::ObjectId thread_group_id, std::vector<JDWP::ObjectId>& thread_ids) {
   class ThreadListVisitor {
    public:
@@ -1609,6 +1596,11 @@
   return visitor.depth;
 }
 
+static bool IsSuspendedForDebugger(ScopedObjectAccessUnchecked& soa, Thread* thread) {
+  MutexLock mu(soa.Self(), *Locks::thread_suspend_count_lock_);
+  return thread->IsSuspended() && thread->GetDebugSuspendCount() > 0;
+}
+
 JDWP::JdwpError Dbg::GetThreadFrameCount(JDWP::ObjectId thread_id, size_t& result) {
   ScopedObjectAccess soa(Thread::Current());
   MutexLock mu(soa.Self(), *Locks::thread_list_lock_);
@@ -1617,6 +1609,9 @@
   if (error != JDWP::ERR_NONE) {
     return error;
   }
+  if (!IsSuspendedForDebugger(soa, thread)) {
+    return JDWP::ERR_THREAD_NOT_SUSPENDED;
+  }
   result = GetStackDepth(thread);
   return JDWP::ERR_NONE;
 }
@@ -1662,7 +1657,6 @@
     JDWP::ExpandBuf* buf_;
   };
 
-  // Caller already checked thread is suspended.
   ScopedObjectAccessUnchecked soa(Thread::Current());
   MutexLock mu(soa.Self(), *Locks::thread_list_lock_);
   Thread* thread;
@@ -1670,6 +1664,9 @@
   if (error != JDWP::ERR_NONE) {
     return error;
   }
+  if (!IsSuspendedForDebugger(soa, thread)) {
+    return JDWP::ERR_THREAD_NOT_SUSPENDED;
+  }
   GetFrameVisitor visitor(thread->GetManagedStack(), thread->GetInstrumentationStack(),
                           start_frame, frame_count, buf);
   visitor.WalkStack();

src/debugger.h

diff --git a/src/debugger.h b/src/debugger.h
index fb70d880..a07cb11 100644
--- a/src/debugger.h
+++ b/src/debugger.h
@@ -227,7 +227,6 @@
 
   static JDWP::JdwpError GetThreadStatus(JDWP::ObjectId thread_id, JDWP::JdwpThreadStatus* pThreadStatus, JDWP::JdwpSuspendStatus* pSuspendStatus);
   static JDWP::JdwpError GetThreadDebugSuspendCount(JDWP::ObjectId thread_id, JDWP::ExpandBuf* pReply);
-  static JDWP::JdwpError IsSuspended(JDWP::ObjectId thread_id, bool& result);
   //static void WaitForSuspend(JDWP::ObjectId thread_id);
 
   // Fills 'thread_ids' with the threads in the given thread group. If thread_group_id == 0,

src/jdwp/jdwp_handler.cc

diff --git a/src/jdwp/jdwp_handler.cc b/src/jdwp/jdwp_handler.cc
index 890bc29..1732ea5 100644
--- a/src/jdwp/jdwp_handler.cc
+++ b/src/jdwp/jdwp_handler.cc
@@ -1010,18 +1010,8 @@
   uint32_t start_frame = Read4BE(&buf);
   uint32_t length = Read4BE(&buf);
 
-  bool is_suspended;
-  JdwpError error = Dbg::IsSuspended(thread_id, is_suspended);
-  if (error != ERR_NONE) {
-    return error;
-  }
-  if (!is_suspended) {
-    LOG(WARNING) << StringPrintf("  Rejecting req for frames in running thread %#llx", thread_id);
-    return ERR_THREAD_NOT_SUSPENDED;
-  }
-
   size_t actual_frame_count;
-  error = Dbg::GetThreadFrameCount(thread_id, actual_frame_count);
+  JdwpError error = Dbg::GetThreadFrameCount(thread_id, actual_frame_count);
   if (error != ERR_NONE) {
     return error;
   }
@@ -1051,18 +1041,8 @@
     SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
   ObjectId thread_id = ReadObjectId(&buf);
 
-  bool is_suspended;
-  JdwpError error = Dbg::IsSuspended(thread_id, is_suspended);
-  if (error != ERR_NONE) {
-    return error;
-  }
-  if (!is_suspended) {
-    LOG(WARNING) << StringPrintf("  Rejecting req for frames in running thread %#llx", thread_id);
-    return ERR_THREAD_NOT_SUSPENDED;
-  }
-
   size_t frame_count;
-  error = Dbg::GetThreadFrameCount(thread_id, frame_count);
+  JdwpError error = Dbg::GetThreadFrameCount(thread_id, frame_count);
   if (error != ERR_NONE) {
     return error;
   }