Use writev() for DDMS packets.
The code was using the JDWP "expanding buffer" stuff, which is fine for
4-byte status messages but will be terrible for buffers of tracing and
profiling data. Now we construct the header in a separate buffer.
diff --git a/vm/jdwp/JdwpAdb.c b/vm/jdwp/JdwpAdb.c
index a74f9e1..cbb5e9d 100644
--- a/vm/jdwp/JdwpAdb.c
+++ b/vm/jdwp/JdwpAdb.c
@@ -434,8 +434,6 @@
cmd = cmdSet = 0; // shut up gcc
- /*dumpPacket(netState->inputBuffer);*/
-
length = read4BE(&buf);
id = read4BE(&buf);
flags = read1(&buf);
@@ -673,7 +671,6 @@
JdwpNetState* netState = state->netState;
int cc;
- /* dumpPacket(expandBufGetBuffer(pReq)); */
if (netState->clientSock < 0) {
/* can happen with some DDMS events */
LOGV("NOT sending request -- no debugger is attached\n");
@@ -697,6 +694,53 @@
return true;
}
+/*
+ * Send a request that was split into two buffers.
+ *
+ * The entire packet must be sent with a single writev() call to avoid
+ * threading issues.
+ *
+ * Returns "true" if it was sent successfully.
+ */
+static bool sendBufferedRequest(JdwpState* state, const void* header,
+ size_t headerLen, const void* body, size_t bodyLen)
+{
+ JdwpNetState* netState = state->netState;
+
+ assert(headerLen > 0);
+
+ if (netState->clientSock < 0) {
+ /* can happen with some DDMS events */
+ LOGV("NOT sending request -- no debugger is attached\n");
+ return false;
+ }
+
+ struct iovec iov[2];
+ int iovcnt = 1;
+ iov[0].iov_base = (void*) header;
+ iov[0].iov_len = headerLen;
+ if (body != NULL) {
+ iovcnt++;
+ iov[1].iov_base = (void*) body;
+ iov[1].iov_len = bodyLen;
+ }
+
+ /*
+ * TODO: we currently assume the writev() will complete in one
+ * go, which may not be safe for a network socket. We may need
+ * to mutex this against handlePacket().
+ */
+ ssize_t actual;
+ actual = writev(netState->clientSock, iov, iovcnt);
+ if ((size_t)actual != headerLen + bodyLen) {
+ LOGE("Failed sending b-req to debugger: %s (%d of %zu)\n",
+ strerror(errno), (int) actual, headerLen+bodyLen);
+ return false;
+ }
+
+ return true;
+}
+
/*
* Our functions.
@@ -711,7 +755,8 @@
isConnected,
awaitingHandshake,
processIncoming,
- sendRequest
+ sendRequest,
+ sendBufferedRequest
};
/*
diff --git a/vm/jdwp/JdwpEvent.c b/vm/jdwp/JdwpEvent.c
index 3233463..bf6054d 100644
--- a/vm/jdwp/JdwpEvent.c
+++ b/vm/jdwp/JdwpEvent.c
@@ -1260,38 +1260,17 @@
*/
void dvmJdwpDdmSendChunk(JdwpState* state, int type, int len, const u1* buf)
{
- ExpandBuf* pReq;
- u1* outBuf;
+ u1 header[kJDWPHeaderLen + 8];
- /*
- * Write the chunk header and data into the ExpandBuf.
- */
- pReq = expandBufAlloc();
- expandBufAddSpace(pReq, kJDWPHeaderLen);
- expandBufAdd4BE(pReq, type);
- expandBufAdd4BE(pReq, len);
- if (len > 0) {
- outBuf = expandBufAddSpace(pReq, len);
- memcpy(outBuf, buf, len);
- }
+ /* form the header (JDWP plus DDMS) */
+ set4BE(header, sizeof(header) + len);
+ set4BE(header+4, dvmJdwpNextRequestSerial(state));
+ set1(header+8, 0); /* flags */
+ set1(header+9, kJDWPDdmCmdSet);
+ set1(header+10, kJDWPDdmCmd);
+ set4BE(header+11, type);
+ set4BE(header+15, len);
- /*
- * Go back and write the JDWP header.
- */
- outBuf = expandBufGetBuffer(pReq);
-
- set4BE(outBuf, expandBufGetLength(pReq));
- set4BE(outBuf+4, dvmJdwpNextRequestSerial(state));
- set1(outBuf+8, 0); /* flags */
- set1(outBuf+9, kJDWPDdmCmdSet);
- set1(outBuf+10, kJDWPDdmCmd);
-
- /*
- * Send it up.
- */
- //LOGD("Sending chunk (type=0x%08x len=%d)\n", type, len);
- dvmJdwpSendRequest(state, pReq);
-
- expandBufFree(pReq);
+ dvmJdwpSendBufferedRequest(state, header, sizeof(header), buf, len);
}
diff --git a/vm/jdwp/JdwpPriv.h b/vm/jdwp/JdwpPriv.h
index 087b560..f5d80f8 100644
--- a/vm/jdwp/JdwpPriv.h
+++ b/vm/jdwp/JdwpPriv.h
@@ -59,6 +59,8 @@
bool (*awaitingHandshake)(struct JdwpState* state);
bool (*processIncoming)(struct JdwpState* state);
bool (*sendRequest)(struct JdwpState* state, ExpandBuf* pReq);
+ bool (*sendBufferedRequest)(struct JdwpState* state, const void* header,
+ size_t headerLen, const void* body, size_t bodyLen);
} JdwpTransport;
const JdwpTransport* dvmJdwpSocketTransport();
@@ -167,5 +169,11 @@
INLINE bool dvmJdwpSendRequest(JdwpState* state, ExpandBuf* pReq) {
return (*state->transport->sendRequest)(state, pReq);
}
+INLINE bool dvmJdwpSendBufferedRequest(JdwpState* state, const void* header,
+ size_t headerLen, const void* body, size_t bodyLen)
+{
+ return (*state->transport->sendBufferedRequest)(state, header, headerLen,
+ body, bodyLen);
+}
#endif /*_DALVIK_JDWP_JDWPPRIV*/
diff --git a/vm/jdwp/JdwpSocket.c b/vm/jdwp/JdwpSocket.c
index 7b1ccfc..a6cbb02 100644
--- a/vm/jdwp/JdwpSocket.c
+++ b/vm/jdwp/JdwpSocket.c
@@ -825,7 +825,7 @@
JdwpNetState* netState = state->netState;
int cc;
- dumpPacket(expandBufGetBuffer(pReq));
+ /*dumpPacket(expandBufGetBuffer(pReq));*/
if (netState->clientSock < 0) {
/* can happen with some DDMS events */
LOGV("NOT sending request -- no debugger is attached\n");
@@ -849,9 +849,60 @@
return true;
}
+/*
+ * Send a request that was split into two buffers.
+ *
+ * The entire packet must be sent with a single writev() call to avoid
+ * threading issues.
+ *
+ * Returns "true" if it was sent successfully.
+ */
+static bool sendBufferedRequest(JdwpState* state, const void* header,
+ size_t headerLen, const void* body, size_t bodyLen)
+{
+ JdwpNetState* netState = state->netState;
+
+ assert(headerLen > 0);
+
+ if (netState->clientSock < 0) {
+ /* can happen with some DDMS events */
+ LOGV("NOT sending request -- no debugger is attached\n");
+ return false;
+ }
+
+ struct iovec iov[2];
+ int iovcnt = 1;
+ iov[0].iov_base = (void*) header;
+ iov[0].iov_len = headerLen;
+ if (body != NULL) {
+ iovcnt++;
+ iov[1].iov_base = (void*) body;
+ iov[1].iov_len = bodyLen;
+ }
+
+ /*
+ * TODO: we currently assume the writev() will complete in one
+ * go, which may not be safe for a network socket. We may need
+ * to mutex this against handlePacket().
+ */
+ ssize_t actual;
+ actual = writev(netState->clientSock, iov, iovcnt);
+ if ((size_t)actual != headerLen + bodyLen) {
+ LOGE("Failed sending b-req to debugger: %s (%d of %zu)\n",
+ strerror(errno), (int) actual, headerLen+bodyLen);
+ return false;
+ }
+
+ return true;
+}
+
/*
* Our functions.
+ *
+ * We can't generally share the implementations with other transports,
+ * even if they're also socket-based, because our JdwpNetState will be
+ * different from theirs.
*/
static const JdwpTransport socketTransport = {
prepareSocket,
@@ -863,7 +914,8 @@
isConnected,
awaitingHandshake,
processIncoming,
- sendRequest
+ sendRequest,
+ sendBufferedRequest,
};
/*
