commit 2eaa41a6dfb97ad1015e456692318af8de13af5c
author Josh Gao <jmgao@google.com> Tue Feb 23 18:05:57 2016 -0800
committer Josh Gao <jmgao@google.com> Thu Feb 25 11:01:59 2016 -0800
tree 9388d0cb00973257fd1777a37ea53a4570ac48ac
parent b6117c4397a711d0e70cade978de97faffb6d4b7

adb: fix leak of framework_fd.

Move the fdevent for the framework authentication connection out of
atransport into its own static variable in adb_auth_client, since its
lifetime is completely unrelated to that of the USB connection.

Bug: http://b/27297963
Change-Id: Ie6180d0b59d133120c5755e239e76ab33ed3cc1d
(cherry picked from commit 9f4861155447ce813d06f39ea7b739837b1a4e2d)

adb/adb_auth_client.cpp

diff --git a/adb/adb_auth_client.cpp b/adb/adb_auth_client.cpp
index c4ffc85..8ef9948 100644
--- a/adb/adb_auth_client.cpp
+++ b/adb/adb_auth_client.cpp
@@ -44,6 +44,7 @@
 };
 
 static fdevent listener_fde;
+static fdevent framework_fde;
 static int framework_fd = -1;
 
 static void usb_disconnected(void* unused, atransport* t);
@@ -161,29 +162,30 @@
     return ret;
 }
 
-static void usb_disconnected(void* unused, atransport* t)
-{
+static void usb_disconnected(void* unused, atransport* t) {
     D("USB disconnect");
     usb_transport = NULL;
     needs_retry = false;
 }
 
-static void adb_auth_event(int fd, unsigned events, void *data)
-{
+static void framework_disconnected() {
+    D("Framework disconnect");
+    fdevent_remove(&framework_fde);
+    framework_fd = -1;
+}
+
+static void adb_auth_event(int fd, unsigned events, void*) {
     char response[2];
     int ret;
 
     if (events & FDE_READ) {
         ret = unix_read(fd, response, sizeof(response));
         if (ret <= 0) {
-            D("Framework disconnect");
-            if (usb_transport)
-                fdevent_remove(&usb_transport->auth_fde);
-            framework_fd = -1;
-        }
-        else if (ret == 2 && response[0] == 'O' && response[1] == 'K') {
-            if (usb_transport)
+            framework_disconnected();
+        } else if (ret == 2 && response[0] == 'O' && response[1] == 'K') {
+            if (usb_transport) {
                 adb_auth_verified(usb_transport);
+            }
         }
     }
 }
@@ -221,13 +223,9 @@
         D("Failed to write PK, errno=%d", errno);
         return;
     }
-
-    fdevent_install(&t->auth_fde, framework_fd, adb_auth_event, t);
-    fdevent_add(&t->auth_fde, FDE_READ);
 }
 
-static void adb_auth_listener(int fd, unsigned events, void *data)
-{
+static void adb_auth_listener(int fd, unsigned events, void* data) {
     sockaddr_storage addr;
     socklen_t alen;
     int s;
@@ -240,7 +238,14 @@
         return;
     }
 
+    if (framework_fd >= 0) {
+        PLOG(WARNING) << "adb received framework auth socket connection again";
+        framework_disconnected();
+    }
+
     framework_fd = s;
+    fdevent_install(&framework_fde, framework_fd, adb_auth_event, nullptr);
+    fdevent_add(&framework_fde, FDE_READ);
 
     if (needs_retry) {
         needs_retry = false;

adb/transport.h

diff --git a/adb/transport.h b/adb/transport.h
index 76d6afa..4c0c008 100644
--- a/adb/transport.h
+++ b/adb/transport.h
@@ -50,7 +50,6 @@
     // it's better to do this piece by piece.
 
     atransport() {
-        auth_fde = {};
         transport_fde = {};
         protocol_version = A_VERSION;
         max_payload = MAX_PAYLOAD;
@@ -87,7 +86,6 @@
 
     void* key = nullptr;
     unsigned char token[TOKEN_SIZE] = {};
-    fdevent auth_fde;
     size_t failed_auth_attempts = 0;
 
     const std::string connection_state_name() const;

adb/transport_test.cpp

diff --git a/adb/transport_test.cpp b/adb/transport_test.cpp
index 97fc069..1bdea2a 100644
--- a/adb/transport_test.cpp
+++ b/adb/transport_test.cpp
@@ -53,7 +53,6 @@
 
         EXPECT_EQ(key, rhs.key);
         EXPECT_EQ(0, memcmp(token, rhs.token, TOKEN_SIZE));
-        EXPECT_EQ(0, memcmp(&auth_fde, &rhs.auth_fde, sizeof(fdevent)));
         EXPECT_EQ(failed_auth_attempts, rhs.failed_auth_attempts);
 
         EXPECT_EQ(features(), rhs.features());