Merge changes I0ed00441,I12d46493,Ib484f701 am: 0a01830612

am: 1425641300

Change-Id: I6fbd5287542ee758c6a15fff544b12bac8205eb4

adb/client/usb_libusb.cpp

@@ -412,8 +412,13 @@ static void device_disconnected(libusb_device* device) {
     if (it != usb_handles.end()) {
         if (!it->second->device_handle) {
             // If the handle is null, we were never able to open the device.
-            unregister_usb_transport(it->second.get());
+
+            // Temporarily release the usb handles mutex to avoid deadlock.
+            std::unique_ptr<usb_handle> handle = std::move(it->second);
             usb_handles.erase(it);
+            lock.unlock();
+            unregister_usb_transport(handle.get());
+            lock.lock();
         } else {
             // Closure of the transport will erase the usb_handle.
         }

adb/socket.h

@@ -19,84 +19,83 @@
 
 #include <stddef.h>
 
+#include <memory>
+
 #include "fdevent.h"
 
 struct apacket;
 class atransport;
 
 /* An asocket represents one half of a connection between a local and
-** remote entity.  A local asocket is bound to a file descriptor.  A
-** remote asocket is bound to the protocol engine.
-*/
+ * remote entity.  A local asocket is bound to a file descriptor.  A
+ * remote asocket is bound to the protocol engine.
+ */
 struct asocket {
-        /* chain pointers for the local/remote list of
-        ** asockets that this asocket lives in
-        */
-    asocket *next;
-    asocket *prev;
+    /* chain pointers for the local/remote list of
+     * asockets that this asocket lives in
+     */
+    asocket* next;
+    asocket* prev;
 
-        /* the unique identifier for this asocket
-        */
+    /* the unique identifier for this asocket
+     */
     unsigned id;
 
-        /* flag: set when the socket's peer has closed
-        ** but packets are still queued for delivery
-        */
-    int    closing;
+    /* flag: set when the socket's peer has closed
+     * but packets are still queued for delivery
+     */
+    int closing;
 
     // flag: set when the socket failed to write, so the socket will not wait to
     // write packets and close directly.
     bool has_write_error;
 
-        /* flag: quit adbd when both ends close the
-        ** local service socket
-        */
-    int    exit_on_close;
+    /* flag: quit adbd when both ends close the
+     * local service socket
+     */
+    int exit_on_close;
 
-        /* the asocket we are connected to
-        */
+    // the asocket we are connected to
+    asocket* peer;
 
-    asocket *peer;
-
-        /* For local asockets, the fde is used to bind
-        ** us to our fd event system.  For remote asockets
-        ** these fields are not used.
-        */
+    /* For local asockets, the fde is used to bind
+     * us to our fd event system.  For remote asockets
+     * these fields are not used.
+     */
     fdevent fde;
     int fd;
 
-        /* queue of apackets waiting to be written
-        */
-    apacket *pkt_first;
-    apacket *pkt_last;
+    // queue of apackets waiting to be written
+    apacket* pkt_first;
+    apacket* pkt_last;
 
-        /* enqueue is called by our peer when it has data
-        ** for us.  It should return 0 if we can accept more
-        ** data or 1 if not.  If we return 1, we must call
-        ** peer->ready() when we once again are ready to
-        ** receive data.
-        */
-    int (*enqueue)(asocket *s, apacket *pkt);
+    /* enqueue is called by our peer when it has data
+     * for us.  It should return 0 if we can accept more
+     * data or 1 if not.  If we return 1, we must call
+     * peer->ready() when we once again are ready to
+     * receive data.
+     */
+    int (*enqueue)(asocket* s, apacket* pkt);
 
-        /* ready is called by the peer when it is ready for
-        ** us to send data via enqueue again
-        */
-    void (*ready)(asocket *s);
+    /* ready is called by the peer when it is ready for
+     * us to send data via enqueue again
+     */
+    void (*ready)(asocket* s);
 
-        /* shutdown is called by the peer before it goes away.
-        ** the socket should not do any further calls on its peer.
-        ** Always followed by a call to close. Optional, i.e. can be NULL.
-        */
-    void (*shutdown)(asocket *s);
+    /* shutdown is called by the peer before it goes away.
+     * the socket should not do any further calls on its peer.
+     * Always followed by a call to close. Optional, i.e. can be NULL.
+     */
+    void (*shutdown)(asocket* s);
 
-        /* close is called by the peer when it has gone away.
-        ** we are not allowed to make any further calls on the
-        ** peer once our close method is called.
-        */
-    void (*close)(asocket *s);
+    /* close is called by the peer when it has gone away.
+     * we are not allowed to make any further calls on the
+     * peer once our close method is called.
+     */
+    void (*close)(asocket* s);
 
-        /* A socket is bound to atransport */
-    atransport *transport;
+    /* A socket is bound to atransport */
+    atransport* transport;
 
     size_t get_max_payload() const;
 };

adb/sockets.cpp

@@ -96,7 +96,7 @@ void install_local_socket(asocket* s) {
 }
 
 void remove_socket(asocket* s) {
-    // socket_list_lock should already be held
+    std::lock_guard<std::recursive_mutex> lock(local_socket_list_lock);
     if (s->prev && s->next) {
         s->prev->next = s->next;
         s->next->prev = s->prev;

adb/transport.cpp

@@ -615,15 +615,15 @@ static void remove_transport(atransport* transport) {
 static void transport_unref(atransport* t) {
     CHECK(t != nullptr);
 
-    size_t old_refcount = t->ref_count--;
-    CHECK_GT(old_refcount, 0u);
-
-    if (old_refcount == 1u) {
+    std::lock_guard<std::recursive_mutex> lock(transport_lock);
+    CHECK_GT(t->ref_count, 0u);
+    t->ref_count--;
+    if (t->ref_count == 0) {
         D("transport: %s unref (kicking and closing)", t->serial);
         t->close(t);
         remove_transport(t);
     } else {
-        D("transport: %s unref (count=%zu)", t->serial, old_refcount - 1);
+        D("transport: %s unref (count=%zu)", t->serial, t->ref_count);
     }
 }
 

adb/transport.h

@@ -64,7 +64,7 @@ class atransport {
     // it's better to do this piece by piece.
 
     atransport(ConnectionState state = kCsOffline)
-        : id(NextTransportId()), ref_count(0), connection_state_(state) {
+        : id(NextTransportId()), connection_state_(state) {
         transport_fde = {};
         protocol_version = A_VERSION;
         max_payload = MAX_PAYLOAD;
@@ -88,7 +88,7 @@ class atransport {
     int fd = -1;
     int transport_socket = -1;
     fdevent transport_fde;
-    std::atomic<size_t> ref_count;
+    size_t ref_count = 0;
     uint32_t sync_token = 0;
     bool online = false;
     TransportType type = kTransportAny;