Avoid interleaving JDWP requests and events.
To avoid bad synchronization between the debugger and the debuggee, we should
not send any events while processing a request (before sending its reply).
This particularly prevents from sending the VM_DEATH event (and consequently
closing the JDWP connection) after receiving the VM_Resume command but before
sending its reply.
Bug: 12581527
Change-Id: I197cc54e980a983faae4b545f25dfe7fe812e841
diff --git a/runtime/jdwp/jdwp.h b/runtime/jdwp/jdwp.h
index 334dca4..e45cb6e 100644
--- a/runtime/jdwp/jdwp.h
+++ b/runtime/jdwp/jdwp.h
@@ -295,6 +295,10 @@
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
void SendBufferedRequest(uint32_t type, const std::vector<iovec>& iov);
+ void StartProcessingRequest() LOCKS_EXCLUDED(process_request_lock_);
+ void EndProcessingRequest() LOCKS_EXCLUDED(process_request_lock_);
+ void WaitForProcessingRequest() LOCKS_EXCLUDED(process_request_lock_);
+
public: // TODO: fix privacy
const JdwpOptions* options_;
@@ -340,6 +344,12 @@
ConditionVariable event_thread_cond_ GUARDED_BY(event_thread_lock_);
ObjectId event_thread_id_;
+ // 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;
+ ConditionVariable process_request_cond_ GUARDED_BY(process_request_lock_);
+ bool processing_request_ GUARDED_BY(process_request_lock_);
+
bool ddm_is_active_;
bool should_exit_;
diff --git a/runtime/jdwp/jdwp_event.cc b/runtime/jdwp/jdwp_event.cc
index e372c26..ede81d2 100644
--- a/runtime/jdwp/jdwp_event.cc
+++ b/runtime/jdwp/jdwp_event.cc
@@ -697,6 +697,11 @@
Set1(buf+9, kJdwpEventCommandSet);
Set1(buf+10, kJdwpCompositeCommand);
+ // Prevents from interleaving commands and events. Otherwise we could end up in sending an event
+ // before sending the reply of the command being processed and would lead to bad synchronization
+ // between the debugger and the debuggee.
+ WaitForProcessingRequest();
+
SendRequest(pReq);
expandBufFree(pReq);
diff --git a/runtime/jdwp/jdwp_handler.cc b/runtime/jdwp/jdwp_handler.cc
index a514e69..0ff78d0 100644
--- a/runtime/jdwp/jdwp_handler.cc
+++ b/runtime/jdwp/jdwp_handler.cc
@@ -1747,6 +1747,44 @@
self->TransitionFromRunnableToSuspended(old_state);
}
+/*
+ * Indicates a request is about to be processed. If a thread wants to send an event in the meantime,
+ * it will need to wait until we processed this request (see EndProcessingRequest).
+ */
+void JdwpState::StartProcessingRequest() {
+ Thread* self = Thread::Current();
+ CHECK_EQ(self, GetDebugThread()) << "Requests are only processed by debug thread";
+ MutexLock mu(self, process_request_lock_);
+ CHECK_EQ(processing_request_, false);
+ processing_request_ = true;
+}
+
+/*
+ * Indicates a request has been processed (and we sent its reply). All threads waiting for us (see
+ * WaitForProcessingRequest) are waken up so they can send events again.
+ */
+void JdwpState::EndProcessingRequest() {
+ Thread* self = Thread::Current();
+ CHECK_EQ(self, GetDebugThread()) << "Requests are only processed by debug thread";
+ MutexLock mu(self, process_request_lock_);
+ CHECK_EQ(processing_request_, true);
+ processing_request_ = false;
+ process_request_cond_.Broadcast(self);
+}
+
+/*
+ * Waits for any request being processed so we do not send an event in the meantime.
+ */
+void JdwpState::WaitForProcessingRequest() {
+ Thread* self = Thread::Current();
+ CHECK_NE(self, GetDebugThread()) << "Events should not be posted by debug thread";
+ MutexLock mu(self, process_request_lock_);
+ while (processing_request_) {
+ process_request_cond_.Wait(self);
+ }
+ CHECK_EQ(processing_request_, false);
+}
+
} // namespace JDWP
} // namespace art
diff --git a/runtime/jdwp/jdwp_main.cc b/runtime/jdwp/jdwp_main.cc
index 928f53d..ba49c45 100644
--- a/runtime/jdwp/jdwp_main.cc
+++ b/runtime/jdwp/jdwp_main.cc
@@ -218,6 +218,9 @@
event_thread_lock_("JDWP event thread lock"),
event_thread_cond_("JDWP event thread condition variable", event_thread_lock_),
event_thread_id_(0),
+ process_request_lock_("JDWP process request lock"),
+ process_request_cond_("JDWP process request condition variable", process_request_lock_),
+ processing_request_(false),
ddm_is_active_(false),
should_exit_(false),
exit_status_(0) {
@@ -383,9 +386,12 @@
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);
+ EndProcessingRequest();
+
if (cc != (ssize_t) expandBufGetLength(pReply)) {
PLOG(ERROR) << "Failed sending reply to debugger";
expandBufFree(pReply);
