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

Commit 39103b95 authored by Josh Gao's avatar Josh Gao
Browse files

adb: fix deadlock in kick_all_transports. 

Previously, kick_all_transports would deadlock if there were any
inaccessible transports, because the transport kick function would call
unregister_usb_transport, which attempts to take the already-held
transport lock.

Fix this by switching the transport lock over to a recursive mutex.

Bug: 65419665
Test: manual
Change-Id: If61296ff4745e1699f3e216811c1383582627604
(cherry picked from commit 1db71afe)
parent 5dec8e22

adb/transport.cpp

+15 −15
Original line number Diff line number Diff line
@@ -49,7 +49,7 @@ static void transport_unref(atransport *t); 
static auto& transport_list = *new std::list<atransport*>();

static auto& pending_list = *new std::list<atransport*>();



static std::mutex& transport_lock = *new std::mutex();

static auto& transport_lock = *new std::recursive_mutex();



const char* const kFeatureShell2 = "shell_v2";

const char* const kFeatureCmd = "cmd";
@@ -298,7 +298,7 @@ static void write_transport_thread(void* _t) { 
}



void kick_transport(atransport* t) {

    std::lock_guard<std::mutex> lock(transport_lock);

    std::lock_guard<std::recursive_mutex> lock(transport_lock);

    // As kick_transport() can be called from threads without guarantee that t is valid,

    // check if the transport is in transport_list first.

    if (std::find(transport_list.begin(), transport_list.end(), t) != transport_list.end()) {
@@ -330,7 +330,7 @@ static void device_tracker_remove(device_tracker* tracker) { 
    device_tracker** pnode = &device_tracker_list;

    device_tracker* node = *pnode;



    std::lock_guard<std::mutex> lock(transport_lock);

    std::lock_guard<std::recursive_mutex> lock(transport_lock);

    while (node) {

        if (node == tracker) {

            *pnode = node->next;
@@ -403,7 +403,7 @@ asocket* create_device_tracker(void) { 


// Check if all of the USB transports are connected.

bool iterate_transports(std::function<bool(const atransport*)> fn) {

    std::lock_guard<std::mutex> lock(transport_lock);

    std::lock_guard<std::recursive_mutex> lock(transport_lock);

    for (const auto& t : transport_list) {

        if (!fn(t)) {

            return false;
@@ -507,7 +507,7 @@ static void transport_registration_func(int _fd, unsigned ev, void* data) { 
        adb_close(t->fd);



        {

            std::lock_guard<std::mutex> lock(transport_lock);

            std::lock_guard<std::recursive_mutex> lock(transport_lock);

            transport_list.remove(t);

        }
@@ -546,7 +546,7 @@ static void transport_registration_func(int _fd, unsigned ev, void* data) { 
    }



    {

        std::lock_guard<std::mutex> lock(transport_lock);

        std::lock_guard<std::recursive_mutex> lock(transport_lock);

        pending_list.remove(t);

        transport_list.push_front(t);

    }
@@ -573,7 +573,7 @@ void init_transport_registration(void) { 


void kick_all_transports() {

    // To avoid only writing part of a packet to a transport after exit, kick all transports.

    std::lock_guard<std::mutex> lock(transport_lock);

    std::lock_guard<std::recursive_mutex> lock(transport_lock);

    for (auto t : transport_list) {

        t->Kick();

    }
@@ -652,7 +652,7 @@ atransport* acquire_one_transport(TransportType type, const char* serial, bool* 
        *error_out = "no devices found";

    }



    std::unique_lock<std::mutex> lock(transport_lock);

    std::unique_lock<std::recursive_mutex> lock(transport_lock);

    for (const auto& t : transport_list) {

        if (t->GetConnectionState() == kCsNoPerm) {

#if ADB_HOST
@@ -927,7 +927,7 @@ static void append_transport(const atransport* t, std::string* result, bool long 
std::string list_transports(bool long_listing) {

    std::string result;



    std::lock_guard<std::mutex> lock(transport_lock);

    std::lock_guard<std::recursive_mutex> lock(transport_lock);

    for (const auto& t : transport_list) {

        append_transport(t, &result, long_listing);

    }
@@ -935,7 +935,7 @@ std::string list_transports(bool long_listing) { 
}



void close_usb_devices(std::function<bool(const atransport*)> predicate) {

    std::lock_guard<std::mutex> lock(transport_lock);

    std::lock_guard<std::recursive_mutex> lock(transport_lock);

    for (auto& t : transport_list) {

        if (predicate(t)) {

            t->Kick();
@@ -964,7 +964,7 @@ int register_socket_transport(int s, const char* serial, int port, int local) { 
        return -1;

    }



    std::unique_lock<std::mutex> lock(transport_lock);

    std::unique_lock<std::recursive_mutex> lock(transport_lock);

    for (const auto& transport : pending_list) {

        if (transport->serial && strcmp(serial, transport->serial) == 0) {

            VLOG(TRANSPORT) << "socket transport " << transport->serial
@@ -996,7 +996,7 @@ int register_socket_transport(int s, const char* serial, int port, int local) { 
atransport* find_transport(const char* serial) {

    atransport* result = nullptr;



    std::lock_guard<std::mutex> lock(transport_lock);

    std::lock_guard<std::recursive_mutex> lock(transport_lock);

    for (auto& t : transport_list) {

        if (t->serial && strcmp(serial, t->serial) == 0) {

            result = t;
@@ -1008,7 +1008,7 @@ atransport* find_transport(const char* serial) { 
}



void kick_all_tcp_devices() {

    std::lock_guard<std::mutex> lock(transport_lock);

    std::lock_guard<std::recursive_mutex> lock(transport_lock);

    for (auto& t : transport_list) {

        if (t->IsTcpDevice()) {

            // Kicking breaks the read_transport thread of this transport out of any read, then
@@ -1037,7 +1037,7 @@ void register_usb_transport(usb_handle* usb, const char* serial, const char* dev 
    }



    {

        std::lock_guard<std::mutex> lock(transport_lock);

        std::lock_guard<std::recursive_mutex> lock(transport_lock);

        pending_list.push_front(t);

    }
@@ -1046,7 +1046,7 @@ void register_usb_transport(usb_handle* usb, const char* serial, const char* dev 


// This should only be used for transports with connection_state == kCsNoPerm.

void unregister_usb_transport(usb_handle* usb) {

    std::lock_guard<std::mutex> lock(transport_lock);

    std::lock_guard<std::recursive_mutex> lock(transport_lock);

    transport_list.remove_if(

        [usb](atransport* t) { return t->usb == usb && t->GetConnectionState() == kCsNoPerm; });

}