JDWP: asynchronous invoke command handling
The JDWP thread used to wait for the result of a method invocation
running in an event thread. But doing that prevents the JDWP thread
from processing incoming commands from the debugger if the event
thread gets suspended by a debug event occurring in another thread.
In Android Studio (or another IDE), this leads to the debugger being
blocked (with the famous message "Waiting until last debugger command
completes" of Android Studio / IntelliJ) because it is actually
waiting for the reply of its latest command while the JDWP thread
cannot process it.
This CL changes the way invoke commands (ClassType.InvokeCommand,
ClassType.NewInstance and ObjectReference.InvokeCommand) are handled
in the ART runtime.
The JDWP thread no longer waits for the event thread to complete the
method invocation. It now simply waits for the next JDWP command to
process. This means it does not send any reply for invoke commands,
except if the information given by the debugger is wrong. In this
case, it still sends a reply with the appropriate error code.
The event thread is now responsible for sending the reply (containing
the result and the exception object of the invoked method) before
going back to the suspended state.
In other words, we add special handling for invoke commands so they
are handled asynchronously while other commands remained handled
synchronously. In the future, we may want to handle all commands
asynchronously (using a queue of reply/event for instance) to remove
the special handling code this CL is adding.
Now the JDWP thread can process commands while a thread is invoking
a method, it is possible for the debugger to detach (by sending a
VirtualMachine.Dispose command) before the invocation completes. In
that situation, we must not suspend threads again (including the
event thread that executed the method) because they would all remain
suspended forever.
Also minor cleanup of the use of JDWP constants and update comments.
Bug: 21515842
Bug: 18899981
Change-Id: I15e00fb068340f3d69dc9225d8d2065246e68c58
diff --git a/runtime/jdwp/jdwp_handler.cc b/runtime/jdwp/jdwp_handler.cc
index f7f70f6..d4e2656 100644
--- a/runtime/jdwp/jdwp_handler.cc
+++ b/runtime/jdwp/jdwp_handler.cc
@@ -52,17 +52,6 @@
return StringPrintf("%#" PRIx64 " (%s)", ref_type_id, signature.c_str());
}
-// Helper function: write a variable-width value into the output input buffer.
-static void WriteValue(ExpandBuf* pReply, int width, uint64_t value) {
- switch (width) {
- case 1: expandBufAdd1(pReply, value); break;
- case 2: expandBufAdd2BE(pReply, value); break;
- case 4: expandBufAdd4BE(pReply, value); break;
- case 8: expandBufAdd8BE(pReply, value); break;
- default: LOG(FATAL) << width; break;
- }
-}
-
static JdwpError WriteTaggedObject(ExpandBuf* reply, ObjectId object_id)
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
uint8_t tag;
@@ -92,7 +81,7 @@
* If "is_constructor" is set, this returns "object_id" rather than the
* expected-to-be-void return value of the called function.
*/
-static JdwpError RequestInvoke(JdwpState*, Request* request, ExpandBuf* pReply,
+static JdwpError RequestInvoke(JdwpState*, Request* request,
ObjectId thread_id, ObjectId object_id,
RefTypeId class_id, MethodId method_id, bool is_constructor)
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
@@ -122,49 +111,15 @@
(options & INVOKE_SINGLE_THREADED) ? " (SINGLE_THREADED)" : "",
(options & INVOKE_NONVIRTUAL) ? " (NONVIRTUAL)" : "");
- JdwpTag resultTag;
- uint64_t resultValue;
- ObjectId exceptObjId;
- JdwpError err = Dbg::InvokeMethod(thread_id, object_id, class_id, method_id, arg_count,
- argValues.get(), argTypes.get(), options, &resultTag,
- &resultValue, &exceptObjId);
- if (err != ERR_NONE) {
- return err;
+ JDWP::JdwpError error = Dbg::PrepareInvokeMethod(request->GetId(), thread_id, object_id,
+ class_id, method_id, arg_count,
+ argValues.get(), argTypes.get(), options);
+ if (error == JDWP::ERR_NONE) {
+ // We successfully requested the invoke. The event thread now owns the arguments array in its
+ // DebugInvokeReq mailbox.
+ argValues.release();
}
-
- 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)";
- }
- }
-
- return err;
+ return error;
}
static JdwpError VM_Version(JdwpState*, Request*, ExpandBuf* pReply)
@@ -684,13 +639,14 @@
* Example: Eclipse sometimes uses java/lang/Class.forName(String s) on
* values in the "variables" display.
*/
-static JdwpError CT_InvokeMethod(JdwpState* state, Request* request, ExpandBuf* pReply)
+static JdwpError CT_InvokeMethod(JdwpState* state, Request* request,
+ ExpandBuf* pReply ATTRIBUTE_UNUSED)
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
RefTypeId class_id = request->ReadRefTypeId();
ObjectId thread_id = request->ReadThreadId();
MethodId method_id = request->ReadMethodId();
- return RequestInvoke(state, request, pReply, thread_id, 0, class_id, method_id, false);
+ return RequestInvoke(state, request, thread_id, 0, class_id, method_id, false);
}
/*
@@ -700,7 +656,8 @@
* Example: in IntelliJ, create a watch on "new String(myByteArray)" to
* see the contents of a byte[] as a string.
*/
-static JdwpError CT_NewInstance(JdwpState* state, Request* request, ExpandBuf* pReply)
+static JdwpError CT_NewInstance(JdwpState* state, Request* request,
+ ExpandBuf* pReply ATTRIBUTE_UNUSED)
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
RefTypeId class_id = request->ReadRefTypeId();
ObjectId thread_id = request->ReadThreadId();
@@ -711,7 +668,7 @@
if (status != ERR_NONE) {
return status;
}
- return RequestInvoke(state, request, pReply, thread_id, object_id, class_id, method_id, true);
+ return RequestInvoke(state, request, thread_id, object_id, class_id, method_id, true);
}
/*
@@ -863,14 +820,15 @@
* object), it will try to invoke the object's toString() function. This
* feature becomes crucial when examining ArrayLists with Eclipse.
*/
-static JdwpError OR_InvokeMethod(JdwpState* state, Request* request, ExpandBuf* pReply)
+static JdwpError OR_InvokeMethod(JdwpState* state, Request* request,
+ ExpandBuf* pReply ATTRIBUTE_UNUSED)
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
ObjectId object_id = request->ReadObjectId();
ObjectId thread_id = request->ReadThreadId();
RefTypeId class_id = request->ReadRefTypeId();
MethodId method_id = request->ReadMethodId();
- return RequestInvoke(state, request, pReply, thread_id, object_id, class_id, method_id, false);
+ return RequestInvoke(state, request, thread_id, object_id, class_id, method_id, false);
}
static JdwpError OR_DisableCollection(JdwpState*, Request* request, ExpandBuf*)
@@ -1602,13 +1560,27 @@
return result;
}
+// Returns true if the given command_set and command identify an "invoke" command.
+static bool IsInvokeCommand(uint8_t command_set, uint8_t command) {
+ if (command_set == kJDWPClassTypeCmdSet) {
+ return command == kJDWPClassTypeInvokeMethodCmd || command == kJDWPClassTypeNewInstanceCmd;
+ } else if (command_set == kJDWPObjectReferenceCmdSet) {
+ return command == kJDWPObjectReferenceInvokeCmd;
+ } else {
+ return false;
+ }
+}
+
/*
- * Process a request from the debugger.
+ * Process a request from the debugger. The skip_reply flag is set to true to indicate to the
+ * caller the reply must not be sent to the debugger. This is used for invoke commands where the
+ * reply is sent by the event thread after completing the invoke.
*
* On entry, the JDWP thread is in VMWAIT.
*/
-size_t JdwpState::ProcessRequest(Request* request, ExpandBuf* pReply) {
+size_t JdwpState::ProcessRequest(Request* request, ExpandBuf* pReply, bool* skip_reply) {
JdwpError result = ERR_NONE;
+ *skip_reply = false;
if (request->GetCommandSet() != kJDWPDdmCmdSet) {
/*
@@ -1661,24 +1633,31 @@
result = ERR_NOT_IMPLEMENTED;
}
- /*
- * Set up the reply header.
- *
- * If we encountered an error, only send the header back.
- */
- uint8_t* replyBuf = expandBufGetBuffer(pReply);
- size_t replyLength = (result == ERR_NONE) ? expandBufGetLength(pReply) : kJDWPHeaderLen;
- Set4BE(replyBuf + 0, replyLength);
- Set4BE(replyBuf + 4, request->GetId());
- Set1(replyBuf + 8, kJDWPFlagReply);
- Set2BE(replyBuf + 9, result);
+ size_t replyLength = 0U;
+ if (result == ERR_NONE && IsInvokeCommand(request->GetCommandSet(), request->GetCommand())) {
+ // We successfully request an invoke in the event thread. It will send the reply once the
+ // invoke completes so we must not send it now.
+ *skip_reply = true;
+ } else {
+ /*
+ * Set up the reply header.
+ *
+ * If we encountered an error, only send the header back.
+ */
+ uint8_t* replyBuf = expandBufGetBuffer(pReply);
+ replyLength = (result == ERR_NONE) ? expandBufGetLength(pReply) : kJDWPHeaderLen;
+ Set4BE(replyBuf + kJDWPHeaderSizeOffset, replyLength);
+ Set4BE(replyBuf + kJDWPHeaderIdOffset, request->GetId());
+ Set1(replyBuf + kJDWPHeaderFlagsOffset, kJDWPFlagReply);
+ Set2BE(replyBuf + kJDWPHeaderErrorCodeOffset, result);
- CHECK_GT(expandBufGetLength(pReply), 0U) << GetCommandName(request) << " " << request->GetId();
+ CHECK_GT(expandBufGetLength(pReply), 0U) << GetCommandName(request) << " " << request->GetId();
- size_t respLen = expandBufGetLength(pReply) - kJDWPHeaderLen;
- VLOG(jdwp) << "REPLY: " << GetCommandName(request) << " " << result << " (length=" << respLen << ")";
- if (false) {
- VLOG(jdwp) << HexDump(expandBufGetBuffer(pReply) + kJDWPHeaderLen, respLen, false, "");
+ size_t respLen = expandBufGetLength(pReply) - kJDWPHeaderLen;
+ VLOG(jdwp) << "REPLY: " << GetCommandName(request) << " " << result << " (length=" << respLen << ")";
+ if (false) {
+ VLOG(jdwp) << HexDump(expandBufGetBuffer(pReply) + kJDWPHeaderLen, respLen, false, "");
+ }
}
VLOG(jdwp) << "----------";