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

Commit b5e11415 authored by Yabin Cui's avatar Yabin Cui
Browse files

adb: fix two device offline problems. 

When device goes offline, user usually has to manually replug the
usb device. This patch tries to solve two offline situations, all
because when adb on host is killed, the adbd on device is not notified.

1. When adb server is killed while pushing a large file to device,
the device is still reading the unfinished large message. So the
device thinks of the CNXN message as part of the previous unfinished
message, so it doesn't reply and the device is in offline state.

The solution is to add a write_msg_lock in atransport struct. And it
kicks the transport only after sending a whole message. By kicking
all transports before exit, we ensure that we don't write part of
a message to any device. So next time we start adb server, the device
should be waiting for a new message.

2. When adb server is killed while pulling a large file from device,
the device is still trying to send the unfinished large message. So
adb on host usually reads data with EOVERFLOW error. This is because
adb on host is reading less than one packet sent from device.

The solution is to use buffered read on host. The max packet size
of bulk transactions in USB 3.0 is 1024 bytes. By preparing an at least
1024 bytes buffer when reading, EOVERFLOW no longer occurs. And teach
adb host to ignore wrong messages.

To be safe, this patch doesn't change any logic on device.

Bug: http://b/32952319
Test: run python -m unittest -q test_device.DeviceOfflineTest
Test: on linux/mac/windows with bullhead, ryu.
Change-Id: Ib149d30028a62a6f03857b8a95ab5a1d6e9b9c4e
parent 82bd278d

adb/adb.cpp

+36 −27
Original line number Diff line number Diff line
@@ -253,6 +253,19 @@ void send_connect(atransport* t) { 
    send_packet(cp, t);

}



#if ADB_HOST



void SendConnectOnHost(atransport* t) {

    // Send an empty message before A_CNXN message. This is because the data toggle of the ep_out on

    // host and ep_in on device may not be the same.

    apacket* p = get_apacket();

    CHECK(p);

    send_packet(p, t);

    send_connect(t);

}



#endif



// qual_overwrite is used to overwrite a qualifier string.  dst is a

// pointer to a char pointer.  It is assumed that if *dst is non-NULL, it

// was malloc'ed and needs to freed.  *dst will be set to a dup of src.
@@ -299,29 +312,29 @@ void parse_banner(const std::string& banner, atransport* t) { 
    const std::string& type = pieces[0];

    if (type == "bootloader") {

        D("setting connection_state to kCsBootloader");

        t->connection_state = kCsBootloader;

        t->SetConnectionState(kCsBootloader);

        update_transports();

    } else if (type == "device") {

        D("setting connection_state to kCsDevice");

        t->connection_state = kCsDevice;

        t->SetConnectionState(kCsDevice);

        update_transports();

    } else if (type == "recovery") {

        D("setting connection_state to kCsRecovery");

        t->connection_state = kCsRecovery;

        t->SetConnectionState(kCsRecovery);

        update_transports();

    } else if (type == "sideload") {

        D("setting connection_state to kCsSideload");

        t->connection_state = kCsSideload;

        t->SetConnectionState(kCsSideload);

        update_transports();

    } else {

        D("setting connection_state to kCsHost");

        t->connection_state = kCsHost;

        t->SetConnectionState(kCsHost);

    }

}



static void handle_new_connection(atransport* t, apacket* p) {

    if (t->connection_state != kCsOffline) {

        t->connection_state = kCsOffline;

    if (t->GetConnectionState() != kCsOffline) {

        t->SetConnectionState(kCsOffline);

        handle_offline(t);

    }
@@ -355,10 +368,10 @@ void handle_packet(apacket *p, atransport *t) 
        if (p->msg.arg0){

            send_packet(p, t);

#if ADB_HOST

            send_connect(t);

            SendConnectOnHost(t);

#endif

        } else {

            t->connection_state = kCsOffline;

            t->SetConnectionState(kCsOffline);

            handle_offline(t);

            send_packet(p, t);

        }
@@ -372,7 +385,9 @@ void handle_packet(apacket *p, atransport *t) 
        switch (p->msg.arg0) {

#if ADB_HOST

            case ADB_AUTH_TOKEN:

                t->connection_state = kCsUnauthorized;

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

                    t->SetConnectionState(kCsUnauthorized);

                }

                send_auth_response(p->data, p->msg.data_length, t);

                break;

#else
@@ -391,7 +406,7 @@ void handle_packet(apacket *p, atransport *t) 
                break;

#endif

            default:

                t->connection_state = kCsOffline;

                t->SetConnectionState(kCsOffline);

                handle_offline(t);

                break;

        }
@@ -1034,7 +1049,6 @@ static int SendOkay(int fd, const std::string& s) { 
    SendProtocolString(fd, s);

    return 0;

}

#endif



int handle_host_request(const char* service, TransportType type,

                        const char* serial, int reply_fd, asocket* s) {
@@ -1053,7 +1067,6 @@ int handle_host_request(const char* service, TransportType type, 
        android::base::quick_exit(0);

    }



#if ADB_HOST

    // "transport:" is used for switching transport with a specified serial number

    // "transport-usb:" is used for switching transport to the only USB transport

    // "transport-local:" is used for switching transport to the only local transport
@@ -1098,16 +1111,10 @@ int handle_host_request(const char* service, TransportType type, 
    if (!strcmp(service, "reconnect-offline")) {

        std::string response;

        close_usb_devices([&response](const atransport* transport) {

            switch (transport->connection_state) {

            switch (transport->GetConnectionState()) {

                case kCsOffline:

                case kCsUnauthorized:

                    response += "reconnecting ";

                    if (transport->serial) {

                        response += transport->serial;

                    } else {

                        response += "<unknown>";

                    }

                    response += "\n";

                    response += "reconnecting " + transport->serial_name() + "\n";

                    return true;

                default:

                    return false;
@@ -1131,7 +1138,6 @@ int handle_host_request(const char* service, TransportType type, 
        return 0;

    }



#if ADB_HOST

    if (!strcmp(service, "host-features")) {

        FeatureSet features = supported_features();

        // Abuse features to report libusb status.
@@ -1141,7 +1147,6 @@ int handle_host_request(const char* service, TransportType type, 
        SendOkay(reply_fd, FeatureSetToString(features));

        return 0;

    }

#endif



    // remove TCP transport

    if (!strncmp(service, "disconnect:", 11)) {
@@ -1211,15 +1216,19 @@ int handle_host_request(const char* service, TransportType type, 
    }



    if (!strcmp(service, "reconnect")) {

        if (s->transport != nullptr) {

            kick_transport(s->transport);

        std::string response;

        atransport* t = acquire_one_transport(type, serial, nullptr, &response, true);

        if (t != nullptr) {

            kick_transport(t);

            response =

                "reconnecting " + t->serial_name() + " [" + t->connection_state_name() + "]\n";

        }

        return SendOkay(reply_fd, "done");

        return SendOkay(reply_fd, response);

    }

#endif // ADB_HOST



    int ret = handle_forward_request(service, type, serial, reply_fd);

    if (ret >= 0)

      return ret - 1;

    return -1;

}

#endif  // ADB_HOST

adb/adb.h

+4 −1
Original line number Diff line number Diff line
@@ -139,7 +139,7 @@ int adb_server_main(int is_daemon, const std::string& socket_spec, int ack_reply 
int get_available_local_transport_index();

#endif

int  init_socket_transport(atransport *t, int s, int port, int local);

void init_usb_transport(atransport *t, usb_handle *usb, ConnectionState state);

void init_usb_transport(atransport* t, usb_handle* usb);



std::string getEmulatorSerialString(int console_port);

#if ADB_HOST
@@ -222,6 +222,9 @@ void handle_online(atransport *t); 
void handle_offline(atransport *t);



void send_connect(atransport *t);

#if ADB_HOST

void SendConnectOnHost(atransport* t);

#endif



void parse_banner(const std::string&, atransport* t);

adb/adb_client.cpp

+4 −7
Original line number Diff line number Diff line
@@ -136,8 +136,7 @@ int _adb_connect(const std::string& service, std::string* error) { 
        return -2;

    }



    if ((memcmp(&service[0],"host",4) != 0 || service == "host:reconnect") &&

        switch_socket_transport(fd, error)) {

    if (memcmp(&service[0], "host", 4) != 0 && switch_socket_transport(fd, error)) {

        return -1;

    }
@@ -147,12 +146,10 @@ int _adb_connect(const std::string& service, std::string* error) { 
        return -1;

    }



    if (service != "reconnect") {

    if (!adb_status(fd, error)) {

        adb_close(fd);

        return -1;

    }

    }



    D("_adb_connect: return fd %d", fd);

    return fd;

adb/adb_trace.cpp

+2 −2
Original line number Diff line number Diff line
@@ -155,7 +155,7 @@ void adb_trace_init(char** argv) { 
    }

#endif



#if !defined(_WIN32)

#if ADB_HOST && !defined(_WIN32)

    // adb historically ignored $ANDROID_LOG_TAGS but passed it through to logcat.

    // If set, move it out of the way so that libbase logging doesn't try to parse it.

    std::string log_tags;
@@ -168,7 +168,7 @@ void adb_trace_init(char** argv) { 


    android::base::InitLogging(argv, &AdbLogger);



#if !defined(_WIN32)

#if ADB_HOST && !defined(_WIN32)

    // Put $ANDROID_LOG_TAGS back so we can pass it to logcat.

    if (!log_tags.empty()) setenv("ANDROID_LOG_TAGS", log_tags.c_str(), 1);

#endif

adb/adb_trace.h

+3 −0
Original line number Diff line number Diff line
@@ -58,6 +58,9 @@ extern int adb_trace_mask; 
void adb_trace_init(char**);

void adb_trace_enable(AdbTrace trace_tag);



// Include <atomic> before stdatomic.h (introduced in cutils/trace.h) to avoid compile error.

#include <atomic>



#define ATRACE_TAG ATRACE_TAG_ADB

#include <cutils/trace.h>

#include <utils/Trace.h>