From 7e197ef83307ffcf2f55d1b8a603d0d3bd2e1d4d Mon Sep 17 00:00:00 2001
From: Josh Gao <jmgao@google.com>
Date: Wed, 7 Jun 2017 18:18:36 -0700
Subject: [PATCH] adb: fix deadlock between transport_unref and usb_close.

Fix a deadlock that happened when a reader/writer thread released a
transport while the hotplug thread attempted  to handle a device
disconnection. Decrementing a transport refcount to zero would hold the
global transport mutex and attempt to take the usb handles mutex, while
the hotplug thread would hold the usb handles mutex and try to call
unregister_usb_transport, which would attempt to take the global
transport mutex.

Resolve this by making transport_unref not take the global transport
mutex.

Bug: http://b/62423753
Test: python test_device.py
Change-Id: Ib48b80a2091a254527f3a7d945b6a11fae61f937
---
 adb/transport.cpp | 10 +++++-----
 adb/transport.h   |  4 ++--
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/adb/transport.cpp b/adb/transport.cpp
index 308ee8db7..2bbbefdd4 100644
--- a/adb/transport.cpp
+++ b/adb/transport.cpp
@@ -603,15 +603,15 @@ static void remove_transport(atransport* transport) {
 static void transport_unref(atransport* t) {
     CHECK(t != nullptr);
 
-    std::lock_guard<std::mutex> lock(transport_lock);
-    CHECK_GT(t->ref_count, 0u);
-    t->ref_count--;
-    if (t->ref_count == 0) {
+    size_t old_refcount = t->ref_count--;
+    CHECK_GT(old_refcount, 0u);
+
+    if (old_refcount == 1u) {
         D("transport: %s unref (kicking and closing)", t->serial);
         t->close(t);
         remove_transport(t);
     } else {
-        D("transport: %s unref (count=%zu)", t->serial, t->ref_count);
+        D("transport: %s unref (count=%zu)", t->serial, old_refcount - 1);
     }
 }
 
diff --git a/adb/transport.h b/adb/transport.h
index 57fc9889a..4a89ed993 100644
--- a/adb/transport.h
+++ b/adb/transport.h
@@ -61,7 +61,7 @@ public:
     // class in one go is a very large change. Given how bad our testing is,
     // it's better to do this piece by piece.
 
-    atransport(ConnectionState state = kCsOffline) : connection_state_(state) {
+    atransport(ConnectionState state = kCsOffline) : ref_count(0), connection_state_(state) {
         transport_fde = {};
         protocol_version = A_VERSION;
         max_payload = MAX_PAYLOAD;
@@ -88,7 +88,7 @@ public:
     int fd = -1;
     int transport_socket = -1;
     fdevent transport_fde;
-    size_t ref_count = 0;
+    std::atomic<size_t> ref_count;
     uint32_t sync_token = 0;
     bool online = false;
     TransportType type = kTransportAny;