Fix DDMS-JDWP race

DDMS would race against the JDWP-Handshake in some circumstances
causing clients to become confused as DDMS packets were recieved
before the handshake reply.

Test: atest CtsJdwpTunnelHostTestCases
Test: ./tools/dt_fds_forward.py
Bug: 178655046
Change-Id: Iabeb68829455ee4d2682f0a14591c8b7b0f4fc5f

5 files changed, 25 insertions, 2 deletions

diff --git a/adbconnection/adbconnection.cc b/adbconnection/adbconnection.cc
index ced2252d090..cca4485cdde 100644
--- a/adbconnection/adbconnection.cc
+++ b/adbconnection/adbconnection.cc
@@ -24,6 +24,7 @@
 #include "android-base/endian.h"
 #include "android-base/stringprintf.h"
 #include "base/file_utils.h"
+#include "base/globals.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/mutex.h"
@@ -61,6 +62,7 @@ using dt_fd_forward::kListenStartMessage;
 using dt_fd_forward::kListenEndMessage;
 using dt_fd_forward::kAcceptMessage;
 using dt_fd_forward::kCloseMessage;
+using dt_fd_forward::kHandshakeCompleteMessage;
 
 // Messages sent to the transport
 using dt_fd_forward::kPerformHandshakeMessage;
@@ -344,7 +346,7 @@ void AdbConnectionState::SendDdmPacket(uint32_t id,
                                        art::ArrayRef<const uint8_t> data) {
   // Get the write_event early to fail fast.
   ScopedEventFdLock lk(adb_write_event_fd_);
-  if (adb_connection_socket_ == -1) {
+  if (adb_connection_socket_ == -1 || !performed_handshake_) {
     VLOG(jdwp) << "Not sending ddms data of type "
                << StringPrintf("%c%c%c%c",
                                static_cast<char>(type >> 24),
@@ -580,6 +582,10 @@ void AdbConnectionState::RunPollLoop(art::Thread* self) {
           }
         } else if (memcmp(kListenEndMessage, buf, sizeof(kListenEndMessage)) == 0) {
           agent_listening_ = false;
+        } else if (memcmp(kHandshakeCompleteMessage, buf, sizeof(kHandshakeCompleteMessage)) == 0) {
+          if (agent_has_socket_) {
+            performed_handshake_ = true;
+          }
         } else if (memcmp(kCloseMessage, buf, sizeof(kCloseMessage)) == 0) {
           CloseFds();
           agent_has_socket_ = false;
diff --git a/adbconnection/adbconnection.h b/adbconnection/adbconnection.h
index 8f5727441d3..6500b791645 100644
--- a/adbconnection/adbconnection.h
+++ b/adbconnection/adbconnection.h
@@ -163,7 +163,7 @@ class AdbConnectionState {
 
   std::atomic<bool> sent_agent_fds_;
 
-  bool performed_handshake_;
+  std::atomic<bool> performed_handshake_;
 
   bool notified_ddm_active_;
 
diff --git a/dt_fd_forward/dt_fd_forward.cc b/dt_fd_forward/dt_fd_forward.cc
index d5b6de5eadc..2a6bfda0ad4 100644
--- a/dt_fd_forward/dt_fd_forward.cc
+++ b/dt_fd_forward/dt_fd_forward.cc
@@ -52,6 +52,8 @@
 
 #include <base/strlcpy.h>
 
+#include "fd_transport.h"
+
 namespace dt_fd_forward {
 
 // Helper that puts line-number in error message.
@@ -287,6 +289,11 @@ static void SendAcceptMessage(int fd) {
   TEMP_FAILURE_RETRY(send(fd, kAcceptMessage, sizeof(kAcceptMessage), MSG_EOR));
 }
 
+static void SendHandshakeCompleteMessage(int fd) {
+  TEMP_FAILURE_RETRY(
+      send(fd, kHandshakeCompleteMessage, sizeof(kHandshakeCompleteMessage), MSG_EOR));
+}
+
 IOResult FdForwardTransport::ReceiveFdsFromSocket(bool* do_handshake) {
   union {
     cmsghdr cm;
@@ -402,6 +409,8 @@ jdwpTransportError FdForwardTransport::Accept() {
         continue;
       }
     }
+    // Tell everyone we have finished the handshake.
+    SendHandshakeCompleteMessage(close_notify_fd_);
     break;
   }
   CHECK(ChangeState(TransportState::kOpening, TransportState::kOpen));
diff --git a/dt_fd_forward/export/fd_transport.h b/dt_fd_forward/export/fd_transport.h
index 144ac5c6ecc..40dbe424702 100644
--- a/dt_fd_forward/export/fd_transport.h
+++ b/dt_fd_forward/export/fd_transport.h
@@ -65,6 +65,11 @@ static constexpr char kListenEndMessage[] = "dt_fd_forward:END-LISTEN";
 // fds are closed.
 static constexpr char kAcceptMessage[] = "dt_fd_forward:ACCEPTED";
 
+// This message is sent over the fd associated with the transport when we have
+// completed the handshake. If the handshake was already performed this is sent
+// immediately.
+static constexpr char kHandshakeCompleteMessage[] = "dt_fd_forward:HANDSHAKE-COMPLETE";
+
 // This message is sent over the fd associated with the transport when we are closing the fds. This
 // can be used by the proxy to send additional data on a dup'd fd. The write_lock_fd_ will be held
 // until the other two fds are closed and then it will be released and closed.
diff --git a/tools/dt_fds_forward.py b/tools/dt_fds_forward.py
index 1f9c41fc26f..2c6a84dfcf2 100755
--- a/tools/dt_fds_forward.py
+++ b/tools/dt_fds_forward.py
@@ -38,6 +38,7 @@ NEED_HANDSHAKE_MESSAGE = b"HANDSHAKE:REQD\x00"
 LISTEN_START_MESSAGE   = b"dt_fd_forward:START-LISTEN\x00"
 LISTEN_END_MESSAGE     = b"dt_fd_forward:END-LISTEN\x00"
 ACCEPTED_MESSAGE       = b"dt_fd_forward:ACCEPTED\x00"
+HANDSHAKEN_MESSAGE     = b"dt_fd_forward:HANDSHAKE-COMPLETE\x00"
 CLOSE_MESSAGE          = b"dt_fd_forward:CLOSING\x00"
 
 libc = ctypes.cdll.LoadLibrary("libc.so.6")
@@ -105,6 +106,8 @@ def HandleSockets(host, port, local_sock, finish_event):
           listening = False
         elif buf == ACCEPTED_MESSAGE:
           print("Fds were accepted.")
+        elif buf == HANDSHAKEN_MESSAGE:
+          print("Handshake completed.")
         elif buf == CLOSE_MESSAGE:
           # TODO Dup the fds and send a fake DDMS message like the actual plugin would.
           print("Fds were closed")