Added mutex for jdwp socket writes to prevent interleaving.

This commit includes a following change: Clean up jdwp adb/socket
and fix initialization issues.

Change-Id: I79461b92d690e42d8d1d82d31fbcacb4d5650a74
diff --git a/vm/jdwp/JdwpAdb.cpp b/vm/jdwp/JdwpAdb.cpp
index ccda1f4..f0156bd 100644
--- a/vm/jdwp/JdwpAdb.cpp
+++ b/vm/jdwp/JdwpAdb.cpp
@@ -47,9 +47,8 @@
 #define kJdwpControlName    "\0jdwp-control"
 #define kJdwpControlNameLen (sizeof(kJdwpControlName)-1)
 
-struct JdwpNetState {
+struct JdwpNetState : public JdwpNetStateBase {
     int                 controlSock;
-    int                 clientSock;
     bool                awaitingHandshake;
     bool                shuttingDown;
     int                 wakeFds[2];
@@ -62,6 +61,23 @@
         struct sockaddr_un  controlAddrUn;
         struct sockaddr     controlAddrPlain;
     } controlAddr;
+
+    JdwpNetState()
+    {
+        controlSock = -1;
+        awaitingHandshake = false;
+        shuttingDown = false;
+        wakeFds[0] = -1;
+        wakeFds[1] = -1;
+
+        inputCount = 0;
+
+        controlAddr.controlAddrUn.sun_family = AF_UNIX;
+        controlAddrLen = sizeof(controlAddr.controlAddrUn.sun_family) +
+                kJdwpControlNameLen;
+        memcpy(controlAddr.controlAddrUn.sun_path, kJdwpControlName,
+                kJdwpControlNameLen);
+    }
 };
 
 static void
@@ -87,32 +103,9 @@
         netState->wakeFds[1] = -1;
     }
 
-    free(netState);
+    delete netState;
 }
 
-
-static JdwpNetState* adbStateAlloc()
-{
-    JdwpNetState* netState = (JdwpNetState*) calloc(sizeof(*netState),1);
-
-    netState->controlSock = -1;
-    netState->clientSock  = -1;
-
-    netState->controlAddr.controlAddrUn.sun_family = AF_UNIX;
-    netState->controlAddrLen =
-            sizeof(netState->controlAddr.controlAddrUn.sun_family) +
-            kJdwpControlNameLen;
-
-    memcpy(netState->controlAddr.controlAddrUn.sun_path,
-           kJdwpControlName, kJdwpControlNameLen);
-
-    netState->wakeFds[0] = -1;
-    netState->wakeFds[1] = -1;
-
-    return netState;
-}
-
-
 /*
  * Do initial prep work, e.g. binding to ports and opening files.  This
  * runs in the main thread, before the JDWP thread starts, so it shouldn't
@@ -124,7 +117,7 @@
 
     LOGV("ADB transport startup");
 
-    state->netState = netState = adbStateAlloc();
+    state->netState = netState = new JdwpNetState;
     if (netState == NULL)
         return false;
 
@@ -469,16 +462,9 @@
         hdr.cmd = cmd;
         dvmJdwpProcessRequest(state, &hdr, buf, dataLen, pReply);
         if (expandBufGetLength(pReply) > 0) {
-            int cc;
+            ssize_t cc = netState->writePacket(pReply);
 
-            /*
-             * TODO: we currently assume the write() will complete in one
-             * go, which may not be safe for a network socket.  We may need
-             * to mutex this against sendRequest().
-             */
-            cc = write(netState->clientSock, expandBufGetBuffer(pReply),
-                    expandBufGetLength(pReply));
-            if (cc != (int) expandBufGetLength(pReply)) {
+            if (cc != (ssize_t) expandBufGetLength(pReply)) {
                 LOGE("Failed sending reply to debugger: %s", strerror(errno));
                 expandBufFree(pReply);
                 return false;
@@ -680,7 +666,6 @@
 static bool sendRequest(JdwpState* state, ExpandBuf* pReq)
 {
     JdwpNetState* netState = state->netState;
-    int cc;
 
     if (netState->clientSock < 0) {
         /* can happen with some DDMS events */
@@ -688,17 +673,13 @@
         return false;
     }
 
-    /*
-     * TODO: we currently assume the write() will complete in one
-     * go, which may not be safe for a network socket.  We may need
-     * to mutex this against handlePacket().
-     */
     errno = 0;
-    cc = write(netState->clientSock, expandBufGetBuffer(pReq),
-            expandBufGetLength(pReq));
-    if (cc != (int) expandBufGetLength(pReq)) {
+
+    ssize_t cc = netState->writePacket(pReq);
+
+    if (cc != (ssize_t) expandBufGetLength(pReq)) {
         LOGE("Failed sending req to debugger: %s (%d of %d)",
-            strerror(errno), cc, (int) expandBufGetLength(pReq));
+            strerror(errno), (int) cc, (int) expandBufGetLength(pReq));
         return false;
     }
 
@@ -729,13 +710,8 @@
     for (i = 0; i < iovcnt; i++)
         expected += iov[i].iov_len;
 
-    /*
-     * 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);
+    ssize_t actual = netState->writeBufferedPacket(iov, iovcnt);
+
     if ((size_t)actual != expected) {
         LOGE("Failed sending b-req to debugger: %s (%d of %zu)",
             strerror(errno), (int) actual, expected);
diff --git a/vm/jdwp/JdwpMain.cpp b/vm/jdwp/JdwpMain.cpp
index 710a937..fb1e23d 100644
--- a/vm/jdwp/JdwpMain.cpp
+++ b/vm/jdwp/JdwpMain.cpp
@@ -30,6 +30,40 @@
 
 static void* jdwpThreadStart(void* arg);
 
+/*
+ * JdwpNetStateBase class implementation
+ */
+JdwpNetStateBase::JdwpNetStateBase()
+{
+    clientSock = -1;
+    dvmDbgInitMutex(&socketLock);
+}
+
+/*
+ * Write a packet. Grabs a mutex to assure atomicity.
+ */
+ssize_t JdwpNetStateBase::writePacket(ExpandBuf* pReply)
+{
+    dvmDbgLockMutex(&socketLock);
+    ssize_t cc = write(clientSock, expandBufGetBuffer(pReply),
+            expandBufGetLength(pReply));
+    dvmDbgUnlockMutex(&socketLock);
+
+    return cc;
+}
+
+/*
+ * Write a buffered packet. Grabs a mutex to assure atomicity.
+ */
+ssize_t JdwpNetStateBase::writeBufferedPacket(const struct iovec* iov,
+    int iovcnt)
+{
+    dvmDbgLockMutex(&socketLock);
+    ssize_t actual = writev(clientSock, iov, iovcnt);
+    dvmDbgUnlockMutex(&socketLock);
+
+    return actual;
+}
 
 /*
  * Initialize JDWP.
diff --git a/vm/jdwp/JdwpPriv.h b/vm/jdwp/JdwpPriv.h
index 11c3438..67a953f 100644
--- a/vm/jdwp/JdwpPriv.h
+++ b/vm/jdwp/JdwpPriv.h
@@ -118,6 +118,21 @@
     bool            ddmActive;
 };
 
+/*
+ * Base class for JdwpNetState
+ */
+class JdwpNetStateBase {
+public:
+    int             clientSock;     /* active connection to debugger */
+
+    JdwpNetStateBase();
+    ssize_t writePacket(ExpandBuf* pReply);
+    ssize_t writeBufferedPacket(const struct iovec* iov, int iovcnt);
+
+private:
+    pthread_mutex_t socketLock;     /* socket synchronization */
+};
+
 
 /* reset all session-specific data */
 void dvmJdwpResetState(JdwpState* state);
diff --git a/vm/jdwp/JdwpSocket.cpp b/vm/jdwp/JdwpSocket.cpp
index 23da1b7..cb4ac34 100644
--- a/vm/jdwp/JdwpSocket.cpp
+++ b/vm/jdwp/JdwpSocket.cpp
@@ -50,10 +50,9 @@
  *
  * We only talk to one debugger at a time.
  */
-struct JdwpNetState {
+struct JdwpNetState : public JdwpNetStateBase {
     short   listenPort;
     int     listenSock;         /* listen for connection from debugger */
-    int     clientSock;         /* active connection to debugger */
     int     wakePipe[2];        /* break out of select */
 
     struct in_addr remoteAddr;
@@ -64,6 +63,18 @@
     /* pending data from the network; would be more efficient as circular buf */
     unsigned char  inputBuffer[kInputBufferSize];
     int     inputCount;
+
+    JdwpNetState()
+    {
+        listenPort  = 0;
+        listenSock  = -1;
+        wakePipe[0] = -1;
+        wakePipe[1] = -1;
+
+        awaitingHandshake = false;
+
+        inputCount = 0;
+    }
 };
 
 static JdwpNetState* netStartup(short port);
@@ -129,15 +140,8 @@
  */
 static JdwpNetState* netStartup(short port)
 {
-    JdwpNetState* netState;
     int one = 1;
-
-    netState = (JdwpNetState*) malloc(sizeof(*netState));
-    memset(netState, 0, sizeof(*netState));
-    netState->listenSock = -1;
-    netState->clientSock = -1;
-    netState->wakePipe[0] = -1;
-    netState->wakePipe[1] = -1;
+    JdwpNetState* netState = new JdwpNetState;
 
     if (port < 0)
         return netState;
@@ -251,7 +255,7 @@
         netState->wakePipe[1] = -1;
     }
 
-    free(netState);
+    delete netState;
 }
 static void netFreeExtern(JdwpState* state)
 {
@@ -614,16 +618,9 @@
         hdr.cmd = cmd;
         dvmJdwpProcessRequest(state, &hdr, buf, dataLen, pReply);
         if (expandBufGetLength(pReply) > 0) {
-            int cc;
+            ssize_t cc = netState->writePacket(pReply);
 
-            /*
-             * TODO: we currently assume the write() will complete in one
-             * go, which may not be safe for a network socket.  We may need
-             * to mutex this against sendRequest().
-             */
-            cc = write(netState->clientSock, expandBufGetBuffer(pReply),
-                    expandBufGetLength(pReply));
-            if (cc != (int) expandBufGetLength(pReply)) {
+            if (cc != (ssize_t) expandBufGetLength(pReply)) {
                 LOGE("Failed sending reply to debugger: %s", strerror(errno));
                 expandBufFree(pReply);
                 return false;
@@ -827,7 +824,6 @@
 static bool sendRequest(JdwpState* state, ExpandBuf* pReq)
 {
     JdwpNetState* netState = state->netState;
-    int cc;
 
     /*dumpPacket(expandBufGetBuffer(pReq));*/
     if (netState->clientSock < 0) {
@@ -836,17 +832,12 @@
         return false;
     }
 
-    /*
-     * TODO: we currently assume the write() will complete in one
-     * go, which may not be safe for a network socket.  We may need
-     * to mutex this against handlePacket().
-     */
     errno = 0;
-    cc = write(netState->clientSock, expandBufGetBuffer(pReq),
-            expandBufGetLength(pReq));
-    if (cc != (int) expandBufGetLength(pReq)) {
+    ssize_t cc = netState->writePacket(pReq);
+
+    if (cc != (ssize_t) expandBufGetLength(pReq)) {
         LOGE("Failed sending req to debugger: %s (%d of %d)",
-            strerror(errno), cc, (int) expandBufGetLength(pReq));
+            strerror(errno), (int) cc, (int) expandBufGetLength(pReq));
         return false;
     }
 
@@ -877,13 +868,8 @@
     for (i = 0; i < iovcnt; i++)
         expected += iov[i].iov_len;
 
-    /*
-     * 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);
+    ssize_t actual = netState->writeBufferedPacket(iov, iovcnt);
+
     if ((size_t)actual != expected) {
         LOGE("Failed sending b-req to debugger: %s (%d of %zu)",
             strerror(errno), (int) actual, expected);