Fix JDWP Virtualmachine.Resume command
We used to call ThreadList::UndoDebuggerSuspensions but this is not
correct. Indeed Virtualmachine.Resume command should be performed
as many times as all threads have been suspended (either by the
Virtualmachine.Suspend command or the application of suspend policy
ALL for event).
This CL fixes this issue by adding ThreadList::ResumeAllForDebugger.
It decrements suspend count and debug suspend count by 1 (globally
and for each thread).
It also handles the case where Virtualmachine.Resume is called while
threads have not been suspended (global debug suspend count is 0). In
this case we just print a warning and do nothing.
Bug: 17974047
Change-Id: I25af22b1936760292d2a8c286e0867f2ae864108
diff --git a/runtime/debugger.cc b/runtime/debugger.cc
index 971ff89..6ccb53a 100644
--- a/runtime/debugger.cc
+++ b/runtime/debugger.cc
@@ -2386,7 +2386,7 @@
}
void Dbg::ResumeVM() {
- Runtime::Current()->GetThreadList()->UndoDebuggerSuspensions();
+ Runtime::Current()->GetThreadList()->ResumeAllForDebugger();
}
JDWP::JdwpError Dbg::SuspendThread(JDWP::ObjectId thread_id, bool request_suspension) {
diff --git a/runtime/debugger.h b/runtime/debugger.h
index cb7adae..48e457f 100644
--- a/runtime/debugger.h
+++ b/runtime/debugger.h
@@ -246,7 +246,9 @@
*/
static int64_t LastDebuggerActivity();
- static void UndoDebuggerSuspensions();
+ static void UndoDebuggerSuspensions()
+ LOCKS_EXCLUDED(Locks::thread_list_lock_,
+ Locks::thread_suspend_count_lock_);
/*
* Class, Object, Array
@@ -459,7 +461,9 @@
static void SuspendVM()
LOCKS_EXCLUDED(Locks::thread_list_lock_,
Locks::thread_suspend_count_lock_);
- static void ResumeVM();
+ static void ResumeVM()
+ LOCKS_EXCLUDED(Locks::thread_list_lock_,
+ Locks::thread_suspend_count_lock_);
static JDWP::JdwpError SuspendThread(JDWP::ObjectId thread_id, bool request_suspension = true)
LOCKS_EXCLUDED(Locks::mutator_lock_,
Locks::thread_list_lock_,
diff --git a/runtime/thread_list.cc b/runtime/thread_list.cc
index 35411e2..f714c13 100644
--- a/runtime/thread_list.cc
+++ b/runtime/thread_list.cc
@@ -661,6 +661,7 @@
{
MutexLock mu(self, *Locks::thread_suspend_count_lock_);
// Update global suspend all state for attaching threads.
+ DCHECK_GE(suspend_all_count_, debug_suspend_all_count_);
++suspend_all_count_;
++debug_suspend_all_count_;
// Increment everybody's suspend count (except our own).
@@ -752,6 +753,55 @@
VLOG(threads) << *self << " self-reviving (debugger)";
}
+void ThreadList::ResumeAllForDebugger() {
+ Thread* self = Thread::Current();
+ Thread* debug_thread = Dbg::GetDebugThread();
+ bool needs_resume = false;
+
+ VLOG(threads) << *self << " ResumeAllForDebugger starting...";
+
+ // Threads can't resume if we exclusively hold the mutator lock.
+ Locks::mutator_lock_->AssertNotExclusiveHeld(self);
+
+ {
+ MutexLock mu(self, *Locks::thread_list_lock_);
+ {
+ MutexLock mu(self, *Locks::thread_suspend_count_lock_);
+ // Update global suspend all state for attaching threads.
+ DCHECK_GE(suspend_all_count_, debug_suspend_all_count_);
+ needs_resume = (debug_suspend_all_count_ > 0);
+ if (needs_resume) {
+ --suspend_all_count_;
+ --debug_suspend_all_count_;
+ // Decrement everybody's suspend count (except our own).
+ for (const auto& thread : list_) {
+ if (thread == self || thread == debug_thread) {
+ continue;
+ }
+ if (thread->GetDebugSuspendCount() == 0) {
+ // This thread may have been individually resumed with ThreadReference.Resume.
+ continue;
+ }
+ VLOG(threads) << "requesting thread resume: " << *thread;
+ thread->ModifySuspendCount(self, -1, true);
+ }
+ } else {
+ // We've been asked to resume all threads without being asked to
+ // suspend them all before. Let's print a warning.
+ LOG(WARNING) << "Debugger attempted to resume all threads without "
+ << "having suspended them all before.";
+ }
+ }
+ }
+
+ if (needs_resume) {
+ MutexLock mu(self, *Locks::thread_suspend_count_lock_);
+ Thread::resume_cond_->Broadcast(self);
+ }
+
+ VLOG(threads) << *self << " ResumeAllForDebugger complete";
+}
+
void ThreadList::UndoDebuggerSuspensions() {
Thread* self = Thread::Current();
diff --git a/runtime/thread_list.h b/runtime/thread_list.h
index 9f47f9f..a7f2c53 100644
--- a/runtime/thread_list.h
+++ b/runtime/thread_list.h
@@ -105,6 +105,11 @@
void SuspendSelfForDebugger()
LOCKS_EXCLUDED(Locks::thread_suspend_count_lock_);
+ // Resume all threads
+ void ResumeAllForDebugger()
+ LOCKS_EXCLUDED(Locks::thread_list_lock_,
+ Locks::thread_suspend_count_lock_);
+
void UndoDebuggerSuspensions()
LOCKS_EXCLUDED(Locks::thread_list_lock_,
Locks::thread_suspend_count_lock_);