diff --git a/adb/client/usb_libusb.cpp b/adb/client/usb_libusb.cpp index e7f44c685..7025f283c 100644 --- a/adb/client/usb_libusb.cpp +++ b/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 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. } diff --git a/adb/socket.h b/adb/socket.h index 4acdf4a6d..64d05a92a 100644 --- a/adb/socket.h +++ b/adb/socket.h @@ -19,84 +19,83 @@ #include +#include + #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; }; diff --git a/adb/sockets.cpp b/adb/sockets.cpp index f28a3df50..c53fbb4ff 100644 --- a/adb/sockets.cpp +++ b/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 lock(local_socket_list_lock); if (s->prev && s->next) { s->prev->next = s->next; s->next->prev = s->prev; diff --git a/adb/transport.cpp b/adb/transport.cpp index b2e03a057..1b597fdd8 100644 --- a/adb/transport.cpp +++ b/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 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); } } diff --git a/adb/transport.h b/adb/transport.h index 00fad5646..374bfc30d 100644 --- a/adb/transport.h +++ b/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 ref_count; + size_t ref_count = 0; uint32_t sync_token = 0; bool online = false; TransportType type = kTransportAny;