summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorSebastien Hertz <shertz@google.com>2016-11-22 14:55:04 +0100
committerSebastien Hertz <shertz@google.com>2016-11-30 09:59:56 +0100
commit2b25ca97ff20ab189567391e861dcd14204a1646 (patch)
treef463d5f3f3f0e1ad83c6930f9b8ed982f4136023
parentb33635d2f9f7ffe10ba94dd1cc695897350d3e5e (diff)
downloadart-2b25ca97ff20ab189567391e861dcd14204a1646.tar.gz
art-2b25ca97ff20ab189567391e861dcd14204a1646.tar.bz2
art-2b25ca97ff20ab189567391e861dcd14204a1646.zip
Fix event reporting from the debugger thread
The debugger thread may trigger events (like CLASS PREPARE after the initalization of a class while processing a command). This CL removes the incorrect CHECK that makes the runtime abort in this case. However, we do check that only the debugger thread can report an event while it is already processing a command. Bug: 33032664 Test: art/tools/run-jdwp-tests.sh '--mode=host' '--variant=X64' (cherry picked from commit af8bcf83535cd7bf5651ada2fb08a0ba9c5191d6) Change-Id: Id2e4362393203935e820586560800b300c6dd3a3
-rw-r--r--runtime/jdwp/jdwp_event.cc47
1 files changed, 25 insertions, 22 deletions
diff --git a/runtime/jdwp/jdwp_event.cc b/runtime/jdwp/jdwp_event.cc
index 06b67b3e4d..0d862fe672 100644
--- a/runtime/jdwp/jdwp_event.cc
+++ b/runtime/jdwp/jdwp_event.cc
@@ -621,8 +621,8 @@ void JdwpState::SendRequestAndPossiblySuspend(ExpandBuf* pReq, JdwpSuspendPolicy
Thread* const self = Thread::Current();
self->AssertThreadSuspensionIsAllowable();
CHECK(pReq != nullptr);
+ CHECK_EQ(threadId, Dbg::GetThreadSelfId()) << "Only the current thread can suspend itself";
/* send request and possibly suspend ourselves */
- JDWP::ObjectId thread_self_id = Dbg::GetThreadSelfId();
ScopedThreadSuspension sts(self, kWaitingForDebuggerSend);
if (suspend_policy != SP_NONE) {
AcquireJdwpTokenForEvent(threadId);
@@ -631,7 +631,7 @@ void JdwpState::SendRequestAndPossiblySuspend(ExpandBuf* pReq, JdwpSuspendPolicy
{
// Before suspending, we change our state to kSuspended so the debugger sees us as RUNNING.
ScopedThreadStateChange stsc(self, kSuspended);
- SuspendByPolicy(suspend_policy, thread_self_id);
+ SuspendByPolicy(suspend_policy, threadId);
}
}
@@ -658,13 +658,10 @@ void JdwpState::ReleaseJdwpTokenForCommand() {
}
void JdwpState::AcquireJdwpTokenForEvent(ObjectId threadId) {
- CHECK_NE(Thread::Current(), GetDebugThread()) << "Expected event thread";
- CHECK_NE(debug_thread_id_, threadId) << "Not expected debug thread";
SetWaitForJdwpToken(threadId);
}
void JdwpState::ReleaseJdwpTokenForEvent() {
- CHECK_NE(Thread::Current(), GetDebugThread()) << "Expected event thread";
ClearWaitForJdwpToken();
}
@@ -685,23 +682,28 @@ void JdwpState::SetWaitForJdwpToken(ObjectId threadId) {
/* this is held for very brief periods; contention is unlikely */
MutexLock mu(self, jdwp_token_lock_);
- CHECK_NE(jdwp_token_owner_thread_id_, threadId) << "Thread is already holding event thread lock";
+ if (jdwp_token_owner_thread_id_ == threadId) {
+ // Only the debugger thread may already hold the event token. For instance, it may trigger
+ // a CLASS_PREPARE event while processing a command that initializes a class.
+ CHECK_EQ(threadId, debug_thread_id_) << "Non-debugger thread is already holding event token";
+ } else {
+ /*
+ * If another thread is already doing stuff, wait for it. This can
+ * go to sleep indefinitely.
+ */
- /*
- * If another thread is already doing stuff, wait for it. This can
- * go to sleep indefinitely.
- */
- while (jdwp_token_owner_thread_id_ != 0) {
- VLOG(jdwp) << StringPrintf("event in progress (%#" PRIx64 "), %#" PRIx64 " sleeping",
- jdwp_token_owner_thread_id_, threadId);
- waited = true;
- jdwp_token_cond_.Wait(self);
- }
+ while (jdwp_token_owner_thread_id_ != 0) {
+ VLOG(jdwp) << StringPrintf("event in progress (%#" PRIx64 "), %#" PRIx64 " sleeping",
+ jdwp_token_owner_thread_id_, threadId);
+ waited = true;
+ jdwp_token_cond_.Wait(self);
+ }
- if (waited || threadId != debug_thread_id_) {
- VLOG(jdwp) << StringPrintf("event token grabbed (%#" PRIx64 ")", threadId);
+ if (waited || threadId != debug_thread_id_) {
+ VLOG(jdwp) << StringPrintf("event token grabbed (%#" PRIx64 ")", threadId);
+ }
+ jdwp_token_owner_thread_id_ = threadId;
}
- jdwp_token_owner_thread_id_ = threadId;
}
/*
@@ -1224,14 +1226,15 @@ void JdwpState::PostClassPrepare(mirror::Class* klass) {
VLOG(jdwp) << " suspend_policy=" << suspend_policy;
}
- if (thread_id == debug_thread_id_) {
+ ObjectId reported_thread_id = thread_id;
+ if (reported_thread_id == debug_thread_id_) {
/*
* JDWP says that, for a class prep in the debugger thread, we
* should set thread to null and if any threads were supposed
* to be suspended then we suspend all other threads.
*/
VLOG(jdwp) << " NOTE: class prepare in debugger thread!";
- thread_id = 0;
+ reported_thread_id = 0;
if (suspend_policy == SP_EVENT_THREAD) {
suspend_policy = SP_ALL;
}
@@ -1244,7 +1247,7 @@ void JdwpState::PostClassPrepare(mirror::Class* klass) {
for (const JdwpEvent* pEvent : match_list) {
expandBufAdd1(pReq, pEvent->eventKind);
expandBufAdd4BE(pReq, pEvent->requestId);
- expandBufAddObjectId(pReq, thread_id);
+ expandBufAddObjectId(pReq, reported_thread_id);
expandBufAdd1(pReq, tag);
expandBufAddRefTypeId(pReq, class_id);
expandBufAddUtf8String(pReq, signature);