From ce5ce87a66addb5f6a1170a0e0958e63dd30d78c Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Tue, 11 Dec 2018 13:11:52 -0800 Subject: [PATCH 1/3] adb: remove incorrect use of RTTI. We were dynamic_casting to UsbConnection to check for USB connections, but the actual type was a BlockingConnectionAdapter wrapping a UsbConnection, with the result that unplugging an inaccessible (due to permissions) device on Linux wouldn't make the device go away. Test: manual Change-Id: Icb4acea5fd3c3baa9691698686213e122e898e4a --- adb/Android.bp | 1 - adb/transport.cpp | 6 +----- adb/transport.h | 7 +++++++ adb/transport_usb.cpp | 1 + 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/adb/Android.bp b/adb/Android.bp index ae8e386e4..fd68bc343 100644 --- a/adb/Android.bp +++ b/adb/Android.bp @@ -24,7 +24,6 @@ cc_defaults { "-Wno-missing-field-initializers", "-Wvla", ], - rtti: true, use_version_lib: true, diff --git a/adb/transport.cpp b/adb/transport.cpp index 87416549b..f59a13583 100644 --- a/adb/transport.cpp +++ b/adb/transport.cpp @@ -1305,11 +1305,7 @@ void register_usb_transport(usb_handle* usb, const char* serial, const char* dev void unregister_usb_transport(usb_handle* usb) { std::lock_guard lock(transport_lock); transport_list.remove_if([usb](atransport* t) { - auto connection = t->connection(); - if (auto usb_connection = dynamic_cast(connection.get())) { - return usb_connection->handle_ == usb && t->GetConnectionState() == kCsNoPerm; - } - return false; + return t->GetUsbHandle() == usb && t->GetConnectionState() == kCsNoPerm; }); } #endif diff --git a/adb/transport.h b/adb/transport.h index d593700dc..790004fdd 100644 --- a/adb/transport.h +++ b/adb/transport.h @@ -37,6 +37,7 @@ #include "adb.h" #include "adb_unique_fd.h" +#include "usb.h" typedef std::unordered_set FeatureSet; @@ -242,6 +243,9 @@ class atransport { return connection_; } + void SetUsbHandle(usb_handle* h) { usb_handle_ = h; } + usb_handle* GetUsbHandle() { return usb_handle_; } + const TransportId id; size_t ref_count = 0; bool online = false; @@ -333,6 +337,9 @@ class atransport { // The underlying connection object. std::shared_ptr connection_ GUARDED_BY(mutex_); + // USB handle for the connection, if available. + usb_handle* usb_handle_ = nullptr; + // A callback that will be invoked when the atransport needs to reconnect. ReconnectCallback reconnect_; diff --git a/adb/transport_usb.cpp b/adb/transport_usb.cpp index c471bf985..2e5918ab6 100644 --- a/adb/transport_usb.cpp +++ b/adb/transport_usb.cpp @@ -180,6 +180,7 @@ void init_usb_transport(atransport* t, usb_handle* h) { auto connection = std::make_unique(h); t->SetConnection(std::make_unique(std::move(connection))); t->type = kTransportUsb; + t->SetUsbHandle(h); } int is_adb_interface(int usb_class, int usb_subclass, int usb_protocol) { From e89a55dd4115ccbd46d281b37c0d56f8a25964ec Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Fri, 16 Nov 2018 14:47:59 -0800 Subject: [PATCH 2/3] adb: make `adb raw` bidirectional. Test: adb raw shell: Change-Id: I973f42c55c71ffd125e58f76d29100a2d5b0c308 --- adb/client/commandline.cpp | 38 +++++++++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/adb/client/commandline.cpp b/adb/client/commandline.cpp index 95e66ae84..c8e834e67 100644 --- a/adb/client/commandline.cpp +++ b/adb/client/commandline.cpp @@ -1275,6 +1275,42 @@ static int adb_connect_command(const std::string& command) { return 0; } +static int adb_connect_command_bidirectional(const std::string& command) { + std::string error; + int fd = adb_connect(command, &error); + if (fd < 0) { + fprintf(stderr, "error: %s\n", error.c_str()); + return 1; + } + + static constexpr auto forward = [](int src, int sink, bool exit_on_end) { + char buf[4096]; + while (true) { + int rc = adb_read(src, buf, sizeof(buf)); + if (rc == 0) { + if (exit_on_end) { + exit(0); + } else { + adb_shutdown(sink, SHUT_WR); + } + return; + } else if (rc < 0) { + perror_exit("read failed"); + } + if (!WriteFdExactly(sink, buf, rc)) { + perror_exit("write failed"); + } + } + }; + + std::thread read(forward, fd, STDOUT_FILENO, true); + std::thread write(forward, STDIN_FILENO, fd, false); + read.join(); + write.join(); + adb_close(fd); + return 0; +} + static int adb_query_command(const std::string& command) { std::string result; std::string error; @@ -1766,7 +1802,7 @@ int adb_commandline(int argc, const char** argv) { if (argc != 2) { error_exit("usage: adb raw SERVICE"); } - return adb_connect_command(argv[1]); + return adb_connect_command_bidirectional(argv[1]); } /* "adb /?" is a common idiom under Windows */ From 6eb788298bbb07b4d10c08f8fdd123ebd601c7f1 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Fri, 16 Nov 2018 15:40:16 -0800 Subject: [PATCH 3/3] adbd: add source/sink services. Add some services that skip the service fd to see how much of a benefit it'll be to eliminate it. Test: adb raw source:$((300 * 1024 * 1024)) | pv > /dev/null Test: dd if=/dev/zero bs=1M count=100 | pv | adb raw sink:$((100 * 1024 * 1024)) Change-Id: I042f25f85b16ae9869cb1f1e306d8671b024ed97 --- adb/Android.bp | 1 + adb/adb.h | 4 ++ adb/daemon/services.cpp | 100 ++++++++++++++++++++++++++++++++++++++++ adb/sockets.cpp | 7 +-- 4 files changed, 107 insertions(+), 5 deletions(-) diff --git a/adb/Android.bp b/adb/Android.bp index fd68bc343..6234af104 100644 --- a/adb/Android.bp +++ b/adb/Android.bp @@ -24,6 +24,7 @@ cc_defaults { "-Wno-missing-field-initializers", "-Wvla", ], + cpp_std: "experimental", use_version_lib: true, diff --git a/adb/adb.h b/adb/adb.h index e2911e81e..cdd63465c 100644 --- a/adb/adb.h +++ b/adb/adb.h @@ -148,6 +148,10 @@ unique_fd daemon_service_to_fd(const char* name, atransport* transport); asocket* host_service_to_socket(const char* name, const char* serial, TransportId transport_id); #endif +#if !ADB_HOST +asocket* daemon_service_to_socket(std::string_view name); +#endif + #if !ADB_HOST int init_jdwp(void); asocket* create_jdwp_service_socket(); diff --git a/adb/daemon/services.cpp b/adb/daemon/services.cpp index b300facff..3182ddd3a 100644 --- a/adb/daemon/services.cpp +++ b/adb/daemon/services.cpp @@ -33,6 +33,7 @@ #include #include +#include #include #include #include @@ -223,6 +224,105 @@ static void spin_service(unique_fd fd) { WriteFdExactly(fd.get(), "spinning\n"); } +struct ServiceSocket : public asocket { + ServiceSocket() { + install_local_socket(this); + this->enqueue = [](asocket* self, apacket::payload_type data) { + return static_cast(self)->Enqueue(std::move(data)); + }; + this->ready = [](asocket* self) { return static_cast(self)->Ready(); }; + this->close = [](asocket* self) { return static_cast(self)->Close(); }; + } + virtual ~ServiceSocket() = default; + + virtual int Enqueue(apacket::payload_type data) { return -1; } + virtual void Ready() {} + virtual void Close() { + if (peer) { + peer->peer = nullptr; + if (peer->shutdown) { + peer->shutdown(peer); + } + peer->close(peer); + } + + remove_socket(this); + delete this; + } +}; + +struct SinkSocket : public ServiceSocket { + explicit SinkSocket(size_t byte_count) { + LOG(INFO) << "Creating new SinkSocket with capacity " << byte_count; + bytes_left_ = byte_count; + } + + virtual ~SinkSocket() { LOG(INFO) << "SinkSocket destroyed"; } + + virtual int Enqueue(apacket::payload_type data) override final { + if (bytes_left_ <= data.size()) { + // Done reading. + Close(); + return -1; + } + + bytes_left_ -= data.size(); + return 0; + } + + size_t bytes_left_; +}; + +struct SourceSocket : public ServiceSocket { + explicit SourceSocket(size_t byte_count) { + LOG(INFO) << "Creating new SourceSocket with capacity " << byte_count; + bytes_left_ = byte_count; + } + + virtual ~SourceSocket() { LOG(INFO) << "SourceSocket destroyed"; } + + void Ready() { + size_t len = std::min(bytes_left_, get_max_payload()); + if (len == 0) { + Close(); + return; + } + + Block block(len); + memset(block.data(), 0, block.size()); + peer->enqueue(peer, std::move(block)); + bytes_left_ -= len; + } + + int Enqueue(apacket::payload_type data) { return -1; } + + size_t bytes_left_; +}; + +asocket* daemon_service_to_socket(std::string_view name) { + if (name == "jdwp") { + return create_jdwp_service_socket(); + } else if (name == "track-jdwp") { + return create_jdwp_tracker_service_socket(); + } else if (name.starts_with("sink:")) { + name.remove_prefix(strlen("sink:")); + uint64_t byte_count = 0; + if (!android::base::ParseUint(name.data(), &byte_count)) { + return nullptr; + } + return new SinkSocket(byte_count); + } else if (name.starts_with("source:")) { + name.remove_prefix(strlen("source:")); + uint64_t byte_count = 0; + if (!android::base::ParseUint(name.data(), &byte_count)) { + return nullptr; + } + return new SourceSocket(byte_count); + } + + return nullptr; +} + unique_fd daemon_service_to_fd(const char* name, atransport* transport) { if (!strncmp("dev:", name, 4)) { return unique_fd{unix_open(name + 4, O_RDWR | O_CLOEXEC)}; diff --git a/adb/sockets.cpp b/adb/sockets.cpp index 8b07f7480..1bd57c1b2 100644 --- a/adb/sockets.cpp +++ b/adb/sockets.cpp @@ -348,11 +348,8 @@ asocket* create_local_socket(int fd) { asocket* create_local_service_socket(const char* name, atransport* transport) { #if !ADB_HOST - if (!strcmp(name, "jdwp")) { - return create_jdwp_service_socket(); - } - if (!strcmp(name, "track-jdwp")) { - return create_jdwp_tracker_service_socket(); + if (asocket* s = daemon_service_to_socket(name); s) { + return s; } #endif int fd = service_to_fd(name, transport);