Merge "JDWP: allocate DebugInvokeReq only when requested"

8 files changed, 165 insertions, 151 deletions

diff --git a/runtime/debugger.cc b/runtime/debugger.cc
index 964e84c367..e6df12e56d 100644
--- a/runtime/debugger.cc
+++ b/runtime/debugger.cc
@@ -347,26 +347,9 @@ uint32_t Dbg::instrumentation_events_ = 0;
 static std::vector<Breakpoint> 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<mirror::Object**>(&klass), arg, root_info);
-  }
-  if (method != nullptr) {
-    callback(reinterpret_cast<mirror::Object**>(&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<DebugInvokeReq> 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<mirror::Object*>(thread_id, &error);
+    gRegistry->Get<mirror::Object*>(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<mirror::Throwable>(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<mirror::Throwable>(soa.Self()->GetException());
+  soa.Self()->ClearException();
 
   // Translate the method through the vtable, unless the debugger wants to suppress it.
-  MutableHandle<mirror::ArtMethod> 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<mirror::ArtMethod> 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<jvalue*>(pReq->arg_values));
+  JValue result = InvokeWithJValues(soa, pReq->receiver.Read(), soa.EncodeMethod(m.Get()),
+                                    reinterpret_cast<jvalue*>(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<mirror::Object> object_result = hs.NewHandle(is_object_result ? result.GetL() : nullptr);
+  Handle<mirror::Throwable> 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 428ded7ffb..0ac83f687c 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<mirror::Object> receiver;      // not used for ClassType.InvokeMethod
+  GcRoot<mirror::Class> klass;
+  GcRoot<mirror::ArtMethod> 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 fc08d23274..4bf7142692 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 0ce4de7f61..c7083dcedc 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<Objec
  * If "is_constructor" is set, this returns "object_id" rather than the
  * expected-to-be-void return value of the called function.
  */
-static JdwpError FinishInvoke(JdwpState*, Request* request, ExpandBuf* pReply,
-                              ObjectId thread_id, ObjectId object_id,
-                              RefTypeId class_id, MethodId method_id, bool is_constructor)
+static JdwpError RequestInvoke(JdwpState*, Request* request, ExpandBuf* pReply,
+                               ObjectId thread_id, ObjectId object_id,
+                               RefTypeId class_id, MethodId method_id, bool is_constructor)
     SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
   CHECK(!is_constructor || object_id != 0);
 
@@ -131,37 +131,35 @@ static JdwpError FinishInvoke(JdwpState*, Request* request, ExpandBuf* pReply,
     return err;
   }
 
-  if (err == ERR_NONE) {
-    if (is_constructor) {
-      // If we invoked a constructor (which actually returns void), return the receiver,
-      // unless we threw, in which case we return NULL.
-      resultTag = JT_OBJECT;
-      resultValue = (exceptObjId == 0) ? object_id : 0;
-    }
+  if (is_constructor) {
+    // If we invoked a constructor (which actually returns void), return the receiver,
+    // unless we threw, in which case we return NULL.
+    resultTag = JT_OBJECT;
+    resultValue = (exceptObjId == 0) ? object_id : 0;
+  }
 
-    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)";
+  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 5540ade555..79cb024674 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 94d10590c7..da1fb6e710 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<instrumentation::InstrumentationStackFrame>;
   tlsPtr_.name = new std::string(kThreadNameDuringStartup);
   tlsPtr_.nested_signal_state = static_cast<jmp_buf*>(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 af02dc779e..325c821dfc 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 d4c1e8c39c..ddfbebd595 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)";
 }