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 (file)
--- a/vm/jdwp/JdwpAdb.c
+++ b/vm/jdwp/JdwpAdb.c
@@ -434,8 +434,6 @@ static bool handlePacket(JdwpState* state)
 
     cmd = cmdSet = 0;       // shut up gcc
 
-    /*dumpPacket(netState->inputBuffer);*/
-
     length = read4BE(&buf);
     id = read4BE(&buf);
     flags = read1(&buf);
@@ -673,7 +671,6 @@ static bool sendRequest(JdwpState* state, ExpandBuf* pReq)
     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 @@ static bool sendRequest(JdwpState* state, ExpandBuf* pReq)
     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 @@ static const JdwpTransport socketTransport = {
     isConnected,
     awaitingHandshake,
     processIncoming,
-    sendRequest
+    sendRequest,
+    sendBufferedRequest
 };
 
 /*

diff --git a/vm/jdwp/JdwpEvent.c b/vm/jdwp/JdwpEvent.c
index 3233463..bf6054d 100644 (file)
--- a/vm/jdwp/JdwpEvent.c
+++ b/vm/jdwp/JdwpEvent.c
@@ -1260,38 +1260,17 @@ bool dvmJdwpPostFieldAccess(JdwpState* state, int STUFF, ObjectId thisPtr,
  */
 void dvmJdwpDdmSendChunk(JdwpState* state, int type, int len, const u1* buf)
 {
-    ExpandBuf* pReq;
-    u1* outBuf;
-
-    /*
-     * 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);
-    }
-
-    /*
-     * 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);
+    u1 header[kJDWPHeaderLen + 8];
+
+    /* 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);
+
+    dvmJdwpSendBufferedRequest(state, header, sizeof(header), buf, len);
 }
 

diff --git a/vm/jdwp/JdwpPriv.h b/vm/jdwp/JdwpPriv.h
index 087b560..f5d80f8 100644 (file)
--- a/vm/jdwp/JdwpPriv.h
+++ b/vm/jdwp/JdwpPriv.h
@@ -59,6 +59,8 @@ typedef struct JdwpTransport {
     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 dvmJdwpProcessIncoming(JdwpState* state) {
 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 (file)
--- a/vm/jdwp/JdwpSocket.c
+++ b/vm/jdwp/JdwpSocket.c
@@ -825,7 +825,7 @@ static bool sendRequest(JdwpState* state, ExpandBuf* pReq)
     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 @@ static bool sendRequest(JdwpState* state, ExpandBuf* pReq)
     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 @@ static const JdwpTransport socketTransport = {
     isConnected,
     awaitingHandshake,
     processIncoming,
-    sendRequest
+    sendRequest,
+    sendBufferedRequest,
 };
 
 /*