adb: wait for devices to come up instead of sleeping for 3s.
Replace a hard-coded 3 second sleep with logic to wait until we've
scanned USB devices once and they've all come online.
Before:
adb shell true 0.00s user 0.00s system 0% cpu 3.047 total
After:
adb shell true 0.00s user 0.00s system 9% cpu 0.041 total
Bug: http://b/37869663
Test: `time adb shell true` after adb kill-server
Change-Id: I251d42afb885908ed9d03167287594ea16650d3f
diff --git a/adb/adb.cpp b/adb/adb.cpp
index 39e71e5..0181daa 100644
--- a/adb/adb.cpp
+++ b/adb/adb.cpp
@@ -31,6 +31,8 @@
#include <time.h>
#include <chrono>
+#include <condition_variable>
+#include <mutex>
#include <string>
#include <thread>
#include <vector>
@@ -48,6 +50,7 @@
#include "adb_io.h"
#include "adb_listeners.h"
#include "adb_utils.h"
+#include "sysdeps/chrono.h"
#include "transport.h"
#if !ADB_HOST
@@ -313,19 +316,15 @@
if (type == "bootloader") {
D("setting connection_state to kCsBootloader");
t->SetConnectionState(kCsBootloader);
- update_transports();
} else if (type == "device") {
D("setting connection_state to kCsDevice");
t->SetConnectionState(kCsDevice);
- update_transports();
} else if (type == "recovery") {
D("setting connection_state to kCsRecovery");
t->SetConnectionState(kCsRecovery);
- update_transports();
} else if (type == "sideload") {
D("setting connection_state to kCsSideload");
t->SetConnectionState(kCsSideload);
- update_transports();
} else {
D("setting connection_state to kCsHost");
t->SetConnectionState(kCsHost);
@@ -353,6 +352,8 @@
send_auth_request(t);
}
#endif
+
+ update_transports();
}
void handle_packet(apacket *p, atransport *t)
@@ -1229,4 +1230,50 @@
return ret - 1;
return -1;
}
+
+static auto& init_mutex = *new std::mutex();
+static auto& init_cv = *new std::condition_variable();
+static bool device_scan_complete = false;
+static bool transports_ready = false;
+
+void update_transport_status() {
+ bool result = iterate_transports([](const atransport* t) {
+ if (t->type == kTransportUsb && t->online != 1) {
+ return false;
+ }
+ return true;
+ });
+
+ D("update_transport_status: transports_ready = %s", result ? "true" : "false");
+
+ bool ready;
+
+ {
+ std::lock_guard<std::mutex> lock(init_mutex);
+ transports_ready = result;
+ ready = transports_ready && device_scan_complete;
+ }
+
+ if (ready) {
+ D("update_transport_status: notifying");
+ init_cv.notify_all();
+ }
+}
+
+void adb_notify_device_scan_complete() {
+ D("device scan complete");
+
+ {
+ std::lock_guard<std::mutex> lock(init_mutex);
+ device_scan_complete = true;
+ }
+
+ update_transport_status();
+}
+
+void adb_wait_for_device_initialization() {
+ std::unique_lock<std::mutex> lock(init_mutex);
+ init_cv.wait_for(lock, 3s, []() { return device_scan_complete && transports_ready; });
+}
+
#endif // ADB_HOST
diff --git a/adb/adb.h b/adb/adb.h
index e3675d8..d6b2b81 100644
--- a/adb/adb.h
+++ b/adb/adb.h
@@ -228,4 +228,18 @@
void parse_banner(const std::string&, atransport* t);
+// On startup, the adb server needs to wait until all of the connected devices are ready.
+// To do this, we need to know when the scan has identified all of the potential new transports, and
+// when each transport becomes ready.
+// TODO: Do this for mDNS as well, instead of just USB?
+
+// We've found all of the transports we potentially care about.
+void adb_notify_device_scan_complete();
+
+// One or more transports have changed status, check to see if we're ready.
+void update_transport_status();
+
+// Wait until device scan has completed and every transport is ready, or a timeout elapses.
+void adb_wait_for_device_initialization();
+
#endif
diff --git a/adb/adb_client.cpp b/adb/adb_client.cpp
index b656887..4f3ff25 100644
--- a/adb/adb_client.cpp
+++ b/adb/adb_client.cpp
@@ -28,12 +28,15 @@
#include <sys/stat.h>
#include <sys/types.h>
+#include <condition_variable>
+#include <mutex>
#include <string>
#include <thread>
#include <vector>
#include <android-base/stringprintf.h>
#include <android-base/strings.h>
+#include <android-base/thread_annotations.h>
#include <cutils/sockets.h>
#include "adb_io.h"
@@ -177,9 +180,8 @@
} else {
fprintf(stderr, "* daemon started successfully\n");
}
- // Give the server some time to start properly and detect devices.
- std::this_thread::sleep_for(3s);
- // fall through to _adb_connect
+ // The server will wait until it detects all of its connected devices before acking.
+ // Fall through to _adb_connect.
} else {
// If a server is already running, check its version matches.
int version = ADB_SERVER_VERSION - 1;
diff --git a/adb/client/main.cpp b/adb/client/main.cpp
index 606203c..fe5099c 100644
--- a/adb/client/main.cpp
+++ b/adb/client/main.cpp
@@ -156,33 +156,38 @@
}
#endif
- // Inform our parent that we are up and running.
+ // Wait for the USB scan to complete before notifying the parent that we're up.
+ // We need to perform this in a thread, because we would otherwise block the event loop.
+ std::thread notify_thread([ack_reply_fd]() {
+ adb_wait_for_device_initialization();
- // Any error output written to stderr now goes to adb.log. We could
- // keep around a copy of the stderr fd and use that to write any errors
- // encountered by the following code, but that is probably overkill.
+ // Any error output written to stderr now goes to adb.log. We could
+ // keep around a copy of the stderr fd and use that to write any errors
+ // encountered by the following code, but that is probably overkill.
#if defined(_WIN32)
- const HANDLE ack_reply_handle = cast_int_to_handle(ack_reply_fd);
- const CHAR ack[] = "OK\n";
- const DWORD bytes_to_write = arraysize(ack) - 1;
- DWORD written = 0;
- if (!WriteFile(ack_reply_handle, ack, bytes_to_write, &written, NULL)) {
- fatal("adb: cannot write ACK to handle 0x%p: %s", ack_reply_handle,
- android::base::SystemErrorCodeToString(GetLastError()).c_str());
- }
- if (written != bytes_to_write) {
- fatal("adb: cannot write %lu bytes of ACK: only wrote %lu bytes",
- bytes_to_write, written);
- }
- CloseHandle(ack_reply_handle);
+ const HANDLE ack_reply_handle = cast_int_to_handle(ack_reply_fd);
+ const CHAR ack[] = "OK\n";
+ const DWORD bytes_to_write = arraysize(ack) - 1;
+ DWORD written = 0;
+ if (!WriteFile(ack_reply_handle, ack, bytes_to_write, &written, NULL)) {
+ fatal("adb: cannot write ACK to handle 0x%p: %s", ack_reply_handle,
+ android::base::SystemErrorCodeToString(GetLastError()).c_str());
+ }
+ if (written != bytes_to_write) {
+ fatal("adb: cannot write %lu bytes of ACK: only wrote %lu bytes", bytes_to_write,
+ written);
+ }
+ CloseHandle(ack_reply_handle);
#else
- // TODO(danalbert): Can't use SendOkay because we're sending "OK\n", not
- // "OKAY".
- if (!android::base::WriteStringToFd("OK\n", ack_reply_fd)) {
- fatal_errno("error writing ACK to fd %d", ack_reply_fd);
- }
- unix_close(ack_reply_fd);
+ // TODO(danalbert): Can't use SendOkay because we're sending "OK\n", not
+ // "OKAY".
+ if (!android::base::WriteStringToFd("OK\n", ack_reply_fd)) {
+ fatal_errno("error writing ACK to fd %d", ack_reply_fd);
+ }
+ unix_close(ack_reply_fd);
#endif
+ });
+ notify_thread.detach();
}
D("Event loop starting");
diff --git a/adb/client/usb_libusb.cpp b/adb/client/usb_libusb.cpp
index d39884a..20610ee 100644
--- a/adb/client/usb_libusb.cpp
+++ b/adb/client/usb_libusb.cpp
@@ -352,6 +352,8 @@
}
libusb_free_device_list(list, 1);
+ adb_notify_device_scan_complete();
+
std::this_thread::sleep_for(500ms);
}
}
diff --git a/adb/transport.cpp b/adb/transport.cpp
index cc8c162..20de642 100644
--- a/adb/transport.cpp
+++ b/adb/transport.cpp
@@ -400,8 +400,27 @@
return &tracker->socket;
}
+// 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);
+ for (const auto& t : transport_list) {
+ if (!fn(t)) {
+ return false;
+ }
+ }
+ for (const auto& t : pending_list) {
+ if (!fn(t)) {
+ return false;
+ }
+ }
+ return true;
+}
+
// Call this function each time the transport list has changed.
void update_transports() {
+ update_transport_status();
+
+ // Notify `adb track-devices` clients.
std::string transports = list_transports(false);
device_tracker* tracker = device_tracker_list;
diff --git a/adb/transport.h b/adb/transport.h
index 8c15d66..e129355 100644
--- a/adb/transport.h
+++ b/adb/transport.h
@@ -198,6 +198,10 @@
void kick_transport(atransport* t);
void update_transports(void);
+// Iterates across all of the current and pending transports.
+// Stops iteration and returns false if fn returns false, otherwise returns true.
+bool iterate_transports(std::function<bool(const atransport*)> fn);
+
void init_transport_registration(void);
void init_mdns_transport_discovery(void);
std::string list_transports(bool long_listing);