diff options
author | Sebastien Hertz <shertz@google.com> | 2014-02-19 15:04:42 +0100 |
---|---|---|
committer | Sebastien Hertz <shertz@google.com> | 2014-02-19 15:41:23 +0100 |
commit | 99660e1c3d6117cfb8bac25b1a0413833ab15b2a (patch) | |
tree | 8b2e8da140f40b6dfb9c11cb7222a9e9ea05e8ef /runtime/jdwp | |
parent | 2c3458dbda97b70158ee7ef22d13ce473a2a2147 (diff) | |
download | art-99660e1c3d6117cfb8bac25b1a0413833ab15b2a.zip art-99660e1c3d6117cfb8bac25b1a0413833ab15b2a.tar.gz art-99660e1c3d6117cfb8bac25b1a0413833ab15b2a.tar.bz2 |
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
Diffstat (limited to 'runtime/jdwp')
-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 e372c26..ede81d2 100644 --- a/runtime/jdwp/jdwp_event.cc +++ b/runtime/jdwp/jdwp_event.cc @@ -697,6 +697,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); |