Fix debugger deadlock.
Fixes a deadlock between the event thread lock and the processing request
lock where the JDWP thread and the event thread wait for each other. We now
move the call to StartProcessingRequest after the call to SetWaitForEventThread
in JdwpState::ProcessRequest so locks are taken in the same order in both
threads.
Also adds some thread safety annotations and JDWP verbose logs for debug (only
enabled if we pass the -verbose:jdwp option to the runtime).
Bug: 13165103
Change-Id: I4e8b6526d973bfee56e69e8e14eddf56adfa1939
diff --git a/runtime/jdwp/jdwp.h b/runtime/jdwp/jdwp.h
index e45cb6e..1ec795f 100644
--- a/runtime/jdwp/jdwp.h
+++ b/runtime/jdwp/jdwp.h
@@ -154,8 +154,9 @@
* release it in the "clear" call.
*/
// ObjectId GetWaitForEventThread();
- void SetWaitForEventThread(ObjectId threadId);
- void ClearWaitForEventThread();
+ void SetWaitForEventThread(ObjectId threadId)
+ LOCKS_EXCLUDED(event_thread_lock_, process_request_lock_);
+ void ClearWaitForEventThread() LOCKS_EXCLUDED(event_thread_lock);
/*
* These notify the debug code that something interesting has happened. This
@@ -346,7 +347,7 @@
// Used to synchronize request processing and event sending (to avoid sending an event before
// sending the reply of a command being processed).
- Mutex process_request_lock_ DEFAULT_MUTEX_ACQUIRED_AFTER;
+ Mutex process_request_lock_ ACQUIRED_AFTER(event_thread_lock_);
ConditionVariable process_request_cond_ GUARDED_BY(process_request_lock_);
bool processing_request_ GUARDED_BY(process_request_lock_);
diff --git a/runtime/jdwp/jdwp_handler.cc b/runtime/jdwp/jdwp_handler.cc
index 0ff78d0..4b170ba 100644
--- a/runtime/jdwp/jdwp_handler.cc
+++ b/runtime/jdwp/jdwp_handler.cc
@@ -1685,6 +1685,12 @@
SetWaitForEventThread(0);
/*
+ * We do not want events to be sent while we process a request. Indicate the JDWP thread starts
+ * to process a request so other threads wait for it to finish before sending an event.
+ */
+ StartProcessingRequest();
+
+ /*
* Tell the VM that we're running and shouldn't be interrupted by GC.
* Do this after anything that can stall indefinitely.
*/
@@ -1779,9 +1785,15 @@
Thread* self = Thread::Current();
CHECK_NE(self, GetDebugThread()) << "Events should not be posted by debug thread";
MutexLock mu(self, process_request_lock_);
+ bool waited = false;
while (processing_request_) {
+ VLOG(jdwp) << StringPrintf("wait for processing request");
+ waited = true;
process_request_cond_.Wait(self);
}
+ if (waited) {
+ VLOG(jdwp) << StringPrintf("finished waiting for processing request");
+ }
CHECK_EQ(processing_request_, false);
}
diff --git a/runtime/jdwp/jdwp_main.cc b/runtime/jdwp/jdwp_main.cc
index ba49c45..500585d 100644
--- a/runtime/jdwp/jdwp_main.cc
+++ b/runtime/jdwp/jdwp_main.cc
@@ -386,10 +386,14 @@
JdwpNetStateBase* netStateBase = reinterpret_cast<JdwpNetStateBase*>(netState);
JDWP::Request request(netStateBase->input_buffer_, netStateBase->input_count_);
- StartProcessingRequest();
ExpandBuf* pReply = expandBufAlloc();
ProcessRequest(request, pReply);
ssize_t cc = netStateBase->WritePacket(pReply);
+
+ /*
+ * We processed this request and sent its reply. Notify other threads waiting for us they can now
+ * send events.
+ */
EndProcessingRequest();
if (cc != (ssize_t) expandBufGetLength(pReply)) {
