diff options
author | Sebastien Hertz <shertz@google.com> | 2014-02-19 16:38:35 +0000 |
---|---|---|
committer | Gerrit Code Review <noreply-gerritcodereview@google.com> | 2014-02-19 16:38:36 +0000 |
commit | bb4329458fba2cc7b1249f1a5fc82a58281e10a5 (patch) | |
tree | 1e7590eb5f9243ef8dbb2d6ca32dfb2e7111f53e | |
parent | e2ef1442ea711c21359a3d0fd57cba5f4490ce04 (diff) | |
parent | 99660e1c3d6117cfb8bac25b1a0413833ab15b2a (diff) | |
download | art-bb4329458fba2cc7b1249f1a5fc82a58281e10a5.zip art-bb4329458fba2cc7b1249f1a5fc82a58281e10a5.tar.gz art-bb4329458fba2cc7b1249f1a5fc82a58281e10a5.tar.bz2 |
Merge "Avoid interleaving JDWP requests and events."
-rw-r--r-- | runtime/jdwp/jdwp.h | 10 | ||||
-rw-r--r-- | runtime/jdwp/jdwp_event.cc | 5 | ||||
-rw-r--r-- | runtime/jdwp/jdwp_handler.cc | 38 | ||||
-rw-r--r-- | runtime/jdwp/jdwp_main.cc | 6 |
4 files changed, 59 insertions, 0 deletions
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 @@ struct JdwpState { 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 @@ struct JdwpState { 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 677b04b..427350e 100644 --- a/runtime/jdwp/jdwp_event.cc +++ b/runtime/jdwp/jdwp_event.cc @@ -689,6 +689,11 @@ void JdwpState::EventFinish(ExpandBuf* pReq) { 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 @@ void JdwpState::ProcessRequest(Request& request, ExpandBuf* pReply) { 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 @@ JdwpState::JdwpState(const JdwpOptions* options) 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 @@ bool JdwpState::HandlePacket() { 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); |