From 1558b577907b613864e98f05862543557263e864 Mon Sep 17 00:00:00 2001 From: Sebastien Hertz Date: Wed, 25 Feb 2015 15:05:59 +0100 Subject: [PATCH] JDWP: allocate DebugInvokeReq only when requested Only allocates thread-local DebugInvokeReq when the debugger requests a thread to invoke a method. The JDWP thread allocates that structure then attaches it to the target thread. When the thread is resumed, it executes the method. Once the invocation completes, the thread detaches the DebugInvokeReq, signals the JDWP thread then suspends. Finally, the JDWP thread wakes up, prepares the reply with the invoke result (or exception) and deallocates the DebugInvokeReq. Also ensures GC safety for object returned by the invoke. We add the object to the JDWP object registry right after the invoke. We now reference that object with a JDWP ObjectID instead of an Object* in the DebugInvokeReq struct. This prevent from accessing a stale reference if the GC runs and moves the Object*. This CL includes the following changes: - Move former DebugInvokeReq::ready flag to Thread::tls_32bit_sized_values::ready_for_debug_invoke. It's needed to know whether a thread has been suspended by an event, thus ready to invoke a method from the debugger. - Remove DebugInvokeReq::invoke_needed: we now test if we attached a DebugInvokeReq* to the thread. - Rename misleading FinishMethod function to RequestMethod. Bug: 19142632 Bug: 18166750 Change-Id: I351fb4eb94bfe69fcafb544d21d55ff35a033000 --- runtime/debugger.cc | 137 +++++++++++++++++++------------------------ runtime/debugger.h | 36 +++++------- runtime/jdwp/jdwp_event.cc | 11 +--- runtime/jdwp/jdwp_handler.cc | 68 +++++++++++---------- runtime/oat.h | 2 +- runtime/thread.cc | 20 ++++++- runtime/thread.h | 29 ++++++++- runtime/thread_list.cc | 13 ++-- 8 files changed, 165 insertions(+), 151 deletions(-) diff --git a/runtime/debugger.cc b/runtime/debugger.cc index 964e84c36..e6df12e56 100644 --- a/runtime/debugger.cc +++ b/runtime/debugger.cc @@ -347,26 +347,9 @@ uint32_t Dbg::instrumentation_events_ = 0; static std::vector gBreakpoints GUARDED_BY(Locks::breakpoint_lock_); void DebugInvokeReq::VisitRoots(RootCallback* callback, void* arg, const RootInfo& root_info) { - if (receiver != nullptr) { - callback(&receiver, arg, root_info); - } - if (thread != nullptr) { - callback(&thread, arg, root_info); - } - if (klass != nullptr) { - callback(reinterpret_cast(&klass), arg, root_info); - } - if (method != nullptr) { - callback(reinterpret_cast(&method), arg, root_info); - } -} - -void DebugInvokeReq::Clear() { - invoke_needed = false; - receiver = nullptr; - thread = nullptr; - klass = nullptr; - method = nullptr; + receiver.VisitRootIfNonNull(callback, arg, root_info); // null for static method call. + klass.VisitRoot(callback, arg, root_info); + method.VisitRoot(callback, arg, root_info); } void SingleStepControl::VisitRoots(RootCallback* callback, void* arg, const RootInfo& root_info) { @@ -3517,10 +3500,14 @@ JDWP::JdwpError Dbg::ConfigureStep(JDWP::ObjectId thread_id, JDWP::JdwpStepSize }; // Allocate single step. - SingleStepControl* single_step_control = new SingleStepControl(step_size, step_depth, - visitor.stack_depth, - visitor.method); - CHECK(single_step_control != nullptr) << "Failed to allocate SingleStepControl"; + SingleStepControl* single_step_control = + new (std::nothrow) SingleStepControl(step_size, step_depth, + visitor.stack_depth, visitor.method); + if (single_step_control == nullptr) { + LOG(ERROR) << "Failed to allocate SingleStepControl"; + return JDWP::ERR_OUT_OF_MEMORY; + } + mirror::ArtMethod* m = single_step_control->GetMethod(); const int32_t line_number = visitor.line_number; if (!m->IsNative()) { @@ -3597,7 +3584,7 @@ JDWP::JdwpError Dbg::InvokeMethod(JDWP::ObjectId thread_id, JDWP::ObjectId objec ThreadList* thread_list = Runtime::Current()->GetThreadList(); Thread* targetThread = nullptr; - DebugInvokeReq* req = nullptr; + std::unique_ptr req; Thread* self = Thread::Current(); { ScopedObjectAccessUnchecked soa(self); @@ -3608,8 +3595,13 @@ JDWP::JdwpError Dbg::InvokeMethod(JDWP::ObjectId thread_id, JDWP::ObjectId objec LOG(ERROR) << "InvokeMethod request for invalid thread id " << thread_id; return error; } - req = targetThread->GetInvokeReq(); - if (!req->ready) { + if (targetThread->GetInvokeReq() != nullptr) { + // Thread is already invoking a method on behalf of the debugger. + LOG(ERROR) << "InvokeMethod request for thread already invoking a method: " << *targetThread; + return JDWP::ERR_ALREADY_INVOKING; + } + if (!targetThread->IsReadyForDebugInvoke()) { + // Thread is not suspended by an event so it cannot invoke a method. LOG(ERROR) << "InvokeMethod request for thread not stopped by event: " << *targetThread; return JDWP::ERR_INVALID_THREAD; } @@ -3643,11 +3635,10 @@ JDWP::JdwpError Dbg::InvokeMethod(JDWP::ObjectId thread_id, JDWP::ObjectId objec return JDWP::ERR_INVALID_OBJECT; } - mirror::Object* thread = gRegistry->Get(thread_id, &error); + gRegistry->Get(thread_id, &error); if (error != JDWP::ERR_NONE) { return JDWP::ERR_INVALID_OBJECT; } - // TODO: check that 'thread' is actually a java.lang.Thread! mirror::Class* c = DecodeClass(class_id, &error); if (c == nullptr) { @@ -3705,14 +3696,17 @@ JDWP::JdwpError Dbg::InvokeMethod(JDWP::ObjectId thread_id, JDWP::ObjectId objec } } - req->receiver = receiver; - req->thread = thread; - req->klass = c; - req->method = m; - req->arg_count = arg_count; - req->arg_values = arg_values; - req->options = options; - req->invoke_needed = true; + // Allocates a DebugInvokeReq. + req.reset(new (std::nothrow) DebugInvokeReq(receiver, c, m, options, arg_values, arg_count)); + if (req.get() == nullptr) { + LOG(ERROR) << "Failed to allocate DebugInvokeReq"; + return JDWP::ERR_OUT_OF_MEMORY; + } + + // Attach the DebugInvokeReq to the target thread so it executes the method when + // it is resumed. Once the invocation completes, it will detach it and signal us + // before suspending itself. + targetThread->SetDebugInvokeReq(req.get()); } // The fact that we've released the thread list lock is a bit risky --- if the thread goes @@ -3746,7 +3740,7 @@ JDWP::JdwpError Dbg::InvokeMethod(JDWP::ObjectId thread_id, JDWP::ObjectId objec gJdwpState->ReleaseJdwpTokenForCommand(); // Wait for the request to finish executing. - while (req->invoke_needed) { + while (targetThread->GetInvokeReq() != nullptr) { req->cond.Wait(self); } } @@ -3779,11 +3773,7 @@ JDWP::JdwpError Dbg::InvokeMethod(JDWP::ObjectId thread_id, JDWP::ObjectId objec // Copy the result. *pResultTag = req->result_tag; - if (IsPrimitiveTag(req->result_tag)) { - *pResultValue = req->result_value.GetJ(); - } else { - *pResultValue = gRegistry->Add(req->result_value.GetL()); - } + *pResultValue = req->result_value; *pExceptionId = req->exception; return req->error; } @@ -3793,60 +3783,55 @@ void Dbg::ExecuteMethod(DebugInvokeReq* pReq) { // We can be called while an exception is pending. We need // to preserve that across the method invocation. - StackHandleScope<2> hs(soa.Self()); - auto old_exception = hs.NewHandle(nullptr); - { - ThrowLocation old_throw_location; - mirror::Throwable* old_exception_obj = soa.Self()->GetException(); - old_exception.Assign(old_exception_obj); - soa.Self()->ClearException(); - } + StackHandleScope<4> hs(soa.Self()); + auto old_exception = hs.NewHandle(soa.Self()->GetException()); + soa.Self()->ClearException(); // Translate the method through the vtable, unless the debugger wants to suppress it. - MutableHandle m(hs.NewHandle(pReq->method)); - if ((pReq->options & JDWP::INVOKE_NONVIRTUAL) == 0 && pReq->receiver != nullptr) { - mirror::ArtMethod* actual_method = pReq->klass->FindVirtualMethodForVirtualOrInterface(m.Get()); + MutableHandle m(hs.NewHandle(pReq->method.Read())); + if ((pReq->options & JDWP::INVOKE_NONVIRTUAL) == 0 && pReq->receiver.Read() != nullptr) { + mirror::ArtMethod* actual_method = pReq->klass.Read()->FindVirtualMethodForVirtualOrInterface(m.Get()); if (actual_method != m.Get()) { - VLOG(jdwp) << "ExecuteMethod translated " << PrettyMethod(m.Get()) << " to " << PrettyMethod(actual_method); + VLOG(jdwp) << "ExecuteMethod translated " << PrettyMethod(m.Get()) + << " to " << PrettyMethod(actual_method); m.Assign(actual_method); } } VLOG(jdwp) << "ExecuteMethod " << PrettyMethod(m.Get()) - << " receiver=" << pReq->receiver + << " receiver=" << pReq->receiver.Read() << " arg_count=" << pReq->arg_count; CHECK(m.Get() != nullptr); CHECK_EQ(sizeof(jvalue), sizeof(uint64_t)); - pReq->result_value = InvokeWithJValues(soa, pReq->receiver, soa.EncodeMethod(m.Get()), - reinterpret_cast(pReq->arg_values)); + JValue result = InvokeWithJValues(soa, pReq->receiver.Read(), soa.EncodeMethod(m.Get()), + reinterpret_cast(pReq->arg_values)); - mirror::Throwable* exception = soa.Self()->GetException(); - soa.Self()->ClearException(); - pReq->exception = gRegistry->Add(exception); pReq->result_tag = BasicTagFromDescriptor(m.Get()->GetShorty()); + const bool is_object_result = (pReq->result_tag == JDWP::JT_OBJECT); + Handle object_result = hs.NewHandle(is_object_result ? result.GetL() : nullptr); + Handle exception = hs.NewHandle(soa.Self()->GetException()); + soa.Self()->ClearException(); + pReq->exception = gRegistry->Add(exception.Get()); if (pReq->exception != 0) { - VLOG(jdwp) << " JDWP invocation returning with exception=" << exception - << " " << exception->Dump(); - pReq->result_value.SetJ(0); - } else if (pReq->result_tag == JDWP::JT_OBJECT) { + VLOG(jdwp) << " JDWP invocation returning with exception=" << exception.Get() + << " " << exception->Dump(); + pReq->result_value = 0; + } else if (is_object_result) { /* if no exception thrown, examine object result more closely */ - JDWP::JdwpTag new_tag = TagFromObject(soa, pReq->result_value.GetL()); + JDWP::JdwpTag new_tag = TagFromObject(soa, object_result.Get()); if (new_tag != pReq->result_tag) { VLOG(jdwp) << " JDWP promoted result from " << pReq->result_tag << " to " << new_tag; pReq->result_tag = new_tag; } - /* - * Register the object. We don't actually need an ObjectId yet, - * but we do need to be sure that the GC won't move or discard the - * object when we switch out of RUNNING. The ObjectId conversion - * will add the object to the "do not touch" list. - * - * We can't use the "tracked allocation" mechanism here because - * the object is going to be handed off to a different thread. - */ - gRegistry->Add(pReq->result_value.GetL()); + // Register the object in the registry and reference its ObjectId. This ensures + // GC safety and prevents from accessing stale reference if the object is moved. + pReq->result_value = gRegistry->Add(object_result.Get()); + } else { + // Primitive result. + DCHECK(IsPrimitiveTag(pReq->result_tag)); + pReq->result_value = result.GetJ(); } if (old_exception.Get() != nullptr) { diff --git a/runtime/debugger.h b/runtime/debugger.h index 428ded7ff..0ac83f687 100644 --- a/runtime/debugger.h +++ b/runtime/debugger.h @@ -54,34 +54,28 @@ class ThrowLocation; * Invoke-during-breakpoint support. */ struct DebugInvokeReq { - DebugInvokeReq() - : ready(false), invoke_needed(false), - receiver(NULL), thread(NULL), klass(NULL), method(NULL), - arg_count(0), arg_values(NULL), options(0), error(JDWP::ERR_NONE), - result_tag(JDWP::JT_VOID), exception(0), + DebugInvokeReq(mirror::Object* invoke_receiver, mirror::Class* invoke_class, + mirror::ArtMethod* invoke_method, uint32_t invoke_options, + uint64_t* args, uint32_t args_count) + : receiver(invoke_receiver), klass(invoke_class), method(invoke_method), + arg_count(args_count), arg_values(args), options(invoke_options), + error(JDWP::ERR_NONE), result_tag(JDWP::JT_VOID), result_value(0), exception(0), lock("a DebugInvokeReq lock", kBreakpointInvokeLock), cond("a DebugInvokeReq condition variable", lock) { } - /* boolean; only set when we're in the tail end of an event handler */ - bool ready; - - /* boolean; set if the JDWP thread wants this thread to do work */ - bool invoke_needed; - /* request */ - mirror::Object* receiver; /* not used for ClassType.InvokeMethod */ - mirror::Object* thread; - mirror::Class* klass; - mirror::ArtMethod* method; - uint32_t arg_count; - uint64_t* arg_values; /* will be NULL if arg_count_ == 0 */ - uint32_t options; + GcRoot receiver; // not used for ClassType.InvokeMethod + GcRoot klass; + GcRoot method; + const uint32_t arg_count; + uint64_t* const arg_values; // will be NULL if arg_count_ == 0 + const uint32_t options; /* result */ JDWP::JdwpError error; JDWP::JdwpTag result_tag; - JValue result_value; + uint64_t result_value; // either a primitive value or an ObjectId JDWP::ObjectId exception; /* condition variable to wait on while the method executes */ @@ -91,8 +85,6 @@ struct DebugInvokeReq { void VisitRoots(RootCallback* callback, void* arg, const RootInfo& root_info) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); - void Clear(); - private: DISALLOW_COPY_AND_ASSIGN(DebugInvokeReq); }; @@ -581,6 +573,8 @@ class Dbg { LOCKS_EXCLUDED(Locks::thread_list_lock_) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); + // Invoke support for commands ClassType.InvokeMethod, ClassType.NewInstance and + // ObjectReference.InvokeMethod. static JDWP::JdwpError InvokeMethod(JDWP::ObjectId thread_id, JDWP::ObjectId object_id, JDWP::RefTypeId class_id, JDWP::MethodId method_id, uint32_t arg_count, uint64_t* arg_values, diff --git a/runtime/jdwp/jdwp_event.cc b/runtime/jdwp/jdwp_event.cc index fc08d2327..4bf714269 100644 --- a/runtime/jdwp/jdwp_event.cc +++ b/runtime/jdwp/jdwp_event.cc @@ -596,17 +596,15 @@ void JdwpState::SuspendByPolicy(JdwpSuspendPolicy suspend_policy, JDWP::ObjectId return; } - DebugInvokeReq* pReq = Dbg::GetInvokeReq(); while (true) { - pReq->ready = true; Dbg::SuspendSelf(); - pReq->ready = false; /* * The JDWP thread has told us (and possibly all other threads) to * resume. See if it has left anything in our DebugInvokeReq mailbox. */ - if (!pReq->invoke_needed) { + DebugInvokeReq* const pReq = Dbg::GetInvokeReq(); + if (pReq == nullptr) { /*LOGD("SuspendByPolicy: no invoke needed");*/ break; } @@ -614,10 +612,7 @@ void JdwpState::SuspendByPolicy(JdwpSuspendPolicy suspend_policy, JDWP::ObjectId /* grab this before posting/suspending again */ AcquireJdwpTokenForEvent(thread_self_id); - /* leave pReq->invoke_needed_ raised so we can check reentrancy */ Dbg::ExecuteMethod(pReq); - - pReq->error = ERR_NONE; } } @@ -650,7 +645,7 @@ void JdwpState::SendRequestAndPossiblySuspend(ExpandBuf* pReq, JdwpSuspendPolicy */ bool JdwpState::InvokeInProgress() { DebugInvokeReq* pReq = Dbg::GetInvokeReq(); - return pReq->invoke_needed; + return pReq != nullptr; } void JdwpState::AcquireJdwpTokenForCommand() { diff --git a/runtime/jdwp/jdwp_handler.cc b/runtime/jdwp/jdwp_handler.cc index 0ce4de7f6..c7083dced 100644 --- a/runtime/jdwp/jdwp_handler.cc +++ b/runtime/jdwp/jdwp_handler.cc @@ -91,9 +91,9 @@ static JdwpError WriteTaggedObjectList(ExpandBuf* reply, const std::vector returned " << resultTag - << StringPrintf(" %#" PRIx64 " (except=%#" PRIx64 ")", resultValue, exceptObjId); - - /* show detailed debug output */ - if (resultTag == JT_STRING && exceptObjId == 0) { - if (resultValue != 0) { - if (VLOG_IS_ON(jdwp)) { - std::string result_string; - JDWP::JdwpError error = Dbg::StringToUtf8(resultValue, &result_string); - CHECK_EQ(error, JDWP::ERR_NONE); - VLOG(jdwp) << " string '" << result_string << "'"; - } - } else { - VLOG(jdwp) << " string (null)"; + size_t width = Dbg::GetTagWidth(resultTag); + expandBufAdd1(pReply, resultTag); + if (width != 0) { + WriteValue(pReply, width, resultValue); + } + expandBufAdd1(pReply, JT_OBJECT); + expandBufAddObjectId(pReply, exceptObjId); + + VLOG(jdwp) << " --> returned " << resultTag + << StringPrintf(" %#" PRIx64 " (except=%#" PRIx64 ")", resultValue, exceptObjId); + + /* show detailed debug output */ + if (resultTag == JT_STRING && exceptObjId == 0) { + if (resultValue != 0) { + if (VLOG_IS_ON(jdwp)) { + std::string result_string; + JDWP::JdwpError error = Dbg::StringToUtf8(resultValue, &result_string); + CHECK_EQ(error, JDWP::ERR_NONE); + VLOG(jdwp) << " string '" << result_string << "'"; } + } else { + VLOG(jdwp) << " string (null)"; } } @@ -693,7 +691,7 @@ static JdwpError CT_InvokeMethod(JdwpState* state, Request* request, ExpandBuf* ObjectId thread_id = request->ReadThreadId(); MethodId method_id = request->ReadMethodId(); - return FinishInvoke(state, request, pReply, thread_id, 0, class_id, method_id, false); + return RequestInvoke(state, request, pReply, thread_id, 0, class_id, method_id, false); } /* @@ -717,7 +715,7 @@ static JdwpError CT_NewInstance(JdwpState* state, Request* request, ExpandBuf* p if (object_id == 0) { return ERR_OUT_OF_MEMORY; } - return FinishInvoke(state, request, pReply, thread_id, object_id, class_id, method_id, true); + return RequestInvoke(state, request, pReply, thread_id, object_id, class_id, method_id, true); } /* @@ -879,7 +877,7 @@ static JdwpError OR_InvokeMethod(JdwpState* state, Request* request, ExpandBuf* RefTypeId class_id = request->ReadRefTypeId(); MethodId method_id = request->ReadMethodId(); - return FinishInvoke(state, request, pReply, thread_id, object_id, class_id, method_id, false); + return RequestInvoke(state, request, pReply, thread_id, object_id, class_id, method_id, false); } static JdwpError OR_DisableCollection(JdwpState*, Request* request, ExpandBuf*) diff --git a/runtime/oat.h b/runtime/oat.h index 5540ade55..79cb02467 100644 --- a/runtime/oat.h +++ b/runtime/oat.h @@ -32,7 +32,7 @@ class InstructionSetFeatures; class PACKED(4) OatHeader { public: static constexpr uint8_t kOatMagic[] = { 'o', 'a', 't', '\n' }; - static constexpr uint8_t kOatVersion[] = { '0', '5', '7', '\0' }; + static constexpr uint8_t kOatVersion[] = { '0', '5', '8', '\0' }; static constexpr const char* kImageLocationKey = "image-location"; static constexpr const char* kDex2OatCmdLineKey = "dex2oat-cmdline"; diff --git a/runtime/thread.cc b/runtime/thread.cc index e31af1627..699e31f11 100644 --- a/runtime/thread.cc +++ b/runtime/thread.cc @@ -1138,8 +1138,6 @@ void Thread::Shutdown() { Thread::Thread(bool daemon) : tls32_(daemon), wait_monitor_(nullptr), interrupted_(false) { wait_mutex_ = new Mutex("a thread wait mutex"); wait_cond_ = new ConditionVariable("a thread wait condition variable", *wait_mutex_); - tlsPtr_.debug_invoke_req = new DebugInvokeReq; - tlsPtr_.single_step_control = nullptr; tlsPtr_.instrumentation_stack = new std::deque; tlsPtr_.name = new std::string(kThreadNameDuringStartup); tlsPtr_.nested_signal_state = static_cast(malloc(sizeof(jmp_buf))); @@ -1291,7 +1289,6 @@ Thread::~Thread() { CleanupCpu(); } - delete tlsPtr_.debug_invoke_req; if (tlsPtr_.single_step_control != nullptr) { delete tlsPtr_.single_step_control; } @@ -2416,4 +2413,21 @@ void Thread::DeactivateSingleStepControl() { delete ssc; } +void Thread::SetDebugInvokeReq(DebugInvokeReq* req) { + CHECK(Dbg::IsDebuggerActive()); + CHECK(GetInvokeReq() == nullptr) << "Debug invoke req already active in thread " << *this; + CHECK(Thread::Current() != this) << "Debug invoke can't be dispatched by the thread itself"; + CHECK(req != nullptr); + tlsPtr_.debug_invoke_req = req; +} + +void Thread::ClearDebugInvokeReq() { + CHECK(Dbg::IsDebuggerActive()); + CHECK(GetInvokeReq() != nullptr) << "Debug invoke req not active in thread " << *this; + CHECK(Thread::Current() == this) << "Debug invoke must be finished by the thread itself"; + // We do not own the DebugInvokeReq* so we must not delete it, it is the responsibility of + // the owner (the JDWP thread). + tlsPtr_.debug_invoke_req = nullptr; +} + } // namespace art diff --git a/runtime/thread.h b/runtime/thread.h index af02dc779..325c821df 100644 --- a/runtime/thread.h +++ b/runtime/thread.h @@ -707,6 +707,16 @@ class Thread { return tlsPtr_.single_step_control; } + // Indicates whether this thread is ready to invoke a method for debugging. This + // is only true if the thread has been suspended by a debug event. + bool IsReadyForDebugInvoke() const { + return tls32_.ready_for_debug_invoke; + } + + void SetReadyForDebugInvoke(bool ready) { + tls32_.ready_for_debug_invoke = ready; + } + // Activates single step control for debugging. The thread takes the // ownership of the given SingleStepControl*. It is deleted by a call // to DeactivateSingleStepControl or upon thread destruction. @@ -715,6 +725,17 @@ class Thread { // Deactivates single step control for debugging. void DeactivateSingleStepControl(); + // Sets debug invoke request for debugging. When the thread is resumed, + // it executes the method described by this request then suspends itself. + // The thread does not take ownership of the given DebugInvokeReq*, it is + // owned by the JDWP thread which is waiting for the execution of the + // method. + void SetDebugInvokeReq(DebugInvokeReq* req); + + // Clears debug invoke request for debugging. When the thread completes + // method invocation, it clears its debug invoke request, signals the + // JDWP thread and suspends itself. + void ClearDebugInvokeReq(); // Returns the fake exception used to activate deoptimization. static mirror::Throwable* GetDeoptimizationException() { @@ -966,7 +987,8 @@ class Thread { explicit tls_32bit_sized_values(bool is_daemon) : suspend_count(0), debug_suspend_count(0), thin_lock_thread_id(0), tid(0), daemon(is_daemon), throwing_OutOfMemoryError(false), no_thread_suspension(0), - thread_exit_check_count(0), handling_signal_(false), suspended_at_suspend_check(false) { + thread_exit_check_count(0), handling_signal_(false), suspended_at_suspend_check(false), + ready_for_debug_invoke(false) { } union StateAndFlags state_and_flags; @@ -1010,6 +1032,11 @@ class Thread { // used to distinguish runnable threads that are suspended due to // a normal suspend check from other threads. bool32_t suspended_at_suspend_check; + + // True if the thread has been suspended by a debugger event. This is + // used to invoke method from the debugger which is only allowed when + // the thread is suspended by an event. + bool32_t ready_for_debug_invoke; } tls32_; struct PACKED(8) tls_64bit_sized_values { diff --git a/runtime/thread_list.cc b/runtime/thread_list.cc index d4c1e8c39..ddfbebd59 100644 --- a/runtime/thread_list.cc +++ b/runtime/thread_list.cc @@ -860,7 +860,8 @@ void ThreadList::SuspendAllForDebugger() { } void ThreadList::SuspendSelfForDebugger() { - Thread* self = Thread::Current(); + Thread* const self = Thread::Current(); + self->SetReadyForDebugInvoke(true); // The debugger thread must not suspend itself due to debugger activity! Thread* debug_thread = Dbg::GetDebugThread(); @@ -881,11 +882,10 @@ void ThreadList::SuspendSelfForDebugger() { VLOG(threads) << *self << " self-suspending (debugger)"; // Tell JDWP we've completed invocation and are ready to suspend. - DebugInvokeReq* pReq = self->GetInvokeReq(); - DCHECK(pReq != NULL); - if (pReq->invoke_needed) { - // Clear this before signaling. - pReq->Clear(); + DebugInvokeReq* const pReq = self->GetInvokeReq(); + if (pReq != nullptr) { + // Clear debug invoke request before signaling. + self->ClearDebugInvokeReq(); VLOG(jdwp) << "invoke complete, signaling"; MutexLock mu(self, pReq->lock); @@ -916,6 +916,7 @@ void ThreadList::SuspendSelfForDebugger() { CHECK_EQ(self->GetSuspendCount(), 0); } + self->SetReadyForDebugInvoke(false); VLOG(threads) << *self << " self-reviving (debugger)"; } -- 2.11.0