Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 0a018306 authored by Josh Gao's avatar Josh Gao Committed by Gerrit Code Review
Browse files

Merge changes I0ed00441,I12d46493,Ib484f701 

* changes:
  Revert "adb: fix deadlock between transport_unref and usb_close."
  adb: add lock to remove_socket.
  adb: reformat comments.
parents 1ca98523 e48ecce6

adb/client/usb_libusb.cpp

+6 −1
Original line number Diff line number Diff line
@@ -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

+58 −59
Original line number Diff line number Diff line
@@ -19,18 +19,20 @@ 


#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

     * asockets that this asocket lives in

     */

    asocket* next;

    asocket* prev;
@@ -40,7 +42,7 @@ struct asocket { 
    unsigned id;



    /* flag: set when the socket's peer has closed

        ** but packets are still queued for delivery

     * but packets are still queued for delivery

     */

    int closing;
@@ -49,49 +51,46 @@ struct asocket { 
    bool has_write_error;



    /* flag: quit adbd when both ends close the

        ** local service socket

     * local service socket

     */

    int exit_on_close;



        /* the asocket we are connected to

        */



    // the asocket we are connected to

    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.

     * 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

        */

    // 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.

     * 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

     * 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.

     * 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.

     * we are not allowed to make any further calls on the

     * peer once our close method is called.

     */

    void (*close)(asocket* s);

adb/sockets.cpp

+1 −1
Original line number Diff line number Diff line
@@ -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

+5 −5
Original line number Diff line number Diff line
@@ -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

+2 −2
Original line number Diff line number Diff line
@@ -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;