Merge changes Ib55d304d,I6fa078ea,I18e9213d,Ife58f0aa,Iccc55557
* changes:
adb: disable ReconnectHandler in adbd.
adb: fix error message for `adb {forward,reverse}`
adb: more immediately try to reconnect connections.
adb: don't pass time_point::max to condition_variable::wait_until.
adb: move list-forward, kill-forward back into handle_forward_request.
diff --git a/adb/adb.cpp b/adb/adb.cpp
index 19300f6..38c6f62 100644
--- a/adb/adb.cpp
+++ b/adb/adb.cpp
@@ -920,13 +920,45 @@
}
#endif /* ADB_HOST */
+bool handle_forward_request(const char* service, atransport* transport, int reply_fd) {
+ return handle_forward_request(service, [transport](std::string*) { return transport; },
+ reply_fd);
+}
+
// Try to handle a network forwarding request.
-// This returns 1 on success, 0 on failure, and -1 to indicate this is not
-// a forwarding-related request.
-int handle_forward_request(const char* service, atransport* transport, int reply_fd) {
+bool handle_forward_request(const char* service,
+ std::function<atransport*(std::string* error)> transport_acquirer,
+ int reply_fd) {
+ if (!strcmp(service, "list-forward")) {
+ // Create the list of forward redirections.
+ std::string listeners = format_listeners();
+#if ADB_HOST
+ SendOkay(reply_fd);
+#endif
+ SendProtocolString(reply_fd, listeners);
+ return true;
+ }
+
+ if (!strcmp(service, "killforward-all")) {
+ remove_all_listeners();
+#if ADB_HOST
+ /* On the host: 1st OKAY is connect, 2nd OKAY is status */
+ SendOkay(reply_fd);
+#endif
+ SendOkay(reply_fd);
+ return true;
+ }
+
if (!strncmp(service, "forward:", 8) || !strncmp(service, "killforward:", 12)) {
// killforward:local
// forward:(norebind:)?local;remote
+ std::string error;
+ atransport* transport = transport_acquirer(&error);
+ if (!transport) {
+ SendFail(reply_fd, error);
+ return true;
+ }
+
bool kill_forward = false;
bool no_rebind = false;
if (android::base::StartsWith(service, "killforward:")) {
@@ -946,17 +978,16 @@
// Check killforward: parameter format: '<local>'
if (pieces.size() != 1 || pieces[0].empty()) {
SendFail(reply_fd, android::base::StringPrintf("bad killforward: %s", service));
- return 1;
+ return true;
}
} else {
// Check forward: parameter format: '<local>;<remote>'
if (pieces.size() != 2 || pieces[0].empty() || pieces[1].empty() || pieces[1][0] == '*') {
SendFail(reply_fd, android::base::StringPrintf("bad forward: %s", service));
- return 1;
+ return true;
}
}
- std::string error;
InstallStatus r;
int resolved_tcp_port = 0;
if (kill_forward) {
@@ -977,7 +1008,7 @@
SendProtocolString(reply_fd, android::base::StringPrintf("%d", resolved_tcp_port));
}
- return 1;
+ return true;
}
std::string message;
@@ -996,9 +1027,10 @@
break;
}
SendFail(reply_fd, message);
- return 1;
+ return true;
}
- return 0;
+
+ return false;
}
#if ADB_HOST
@@ -1186,35 +1218,15 @@
return SendOkay(reply_fd, response);
}
- if (!strcmp(service, "list-forward")) {
- // Create the list of forward redirections.
- std::string listeners = format_listeners();
-#if ADB_HOST
- SendOkay(reply_fd);
-#endif
- return SendProtocolString(reply_fd, listeners);
+ if (handle_forward_request(service,
+ [=](std::string* error) {
+ return acquire_one_transport(type, serial, transport_id, nullptr,
+ error);
+ },
+ reply_fd)) {
+ return 0;
}
- if (!strcmp(service, "killforward-all")) {
- remove_all_listeners();
-#if ADB_HOST
- /* On the host: 1st OKAY is connect, 2nd OKAY is status */
- SendOkay(reply_fd);
-#endif
- SendOkay(reply_fd);
- return 1;
- }
-
- std::string error;
- atransport* t = acquire_one_transport(type, serial, transport_id, nullptr, &error);
- if (!t) {
- SendFail(reply_fd, error);
- return 1;
- }
-
- int ret = handle_forward_request(service, t, reply_fd);
- if (ret >= 0)
- return ret - 1;
return -1;
}
diff --git a/adb/adb.h b/adb/adb.h
index 13ca4d7..e6af780 100644
--- a/adb/adb.h
+++ b/adb/adb.h
@@ -158,7 +158,10 @@
unique_fd create_jdwp_connection_fd(int jdwp_pid);
#endif
-int handle_forward_request(const char* service, atransport* transport, int reply_fd);
+bool handle_forward_request(const char* service, atransport* transport, int reply_fd);
+bool handle_forward_request(const char* service,
+ std::function<atransport*(std::string* error)> transport_acquirer,
+ int reply_fd);
/* packet allocator */
apacket* get_apacket(void);
diff --git a/adb/client/commandline.cpp b/adb/client/commandline.cpp
index a7a94e7..3fb14f3 100644
--- a/adb/client/commandline.cpp
+++ b/adb/client/commandline.cpp
@@ -1614,9 +1614,9 @@
return bugreport.DoIt(argc, argv);
} else if (!strcmp(argv[0], "forward") || !strcmp(argv[0], "reverse")) {
bool reverse = !strcmp(argv[0], "reverse");
- ++argv;
--argc;
if (argc < 1) return syntax_error("%s requires an argument", argv[0]);
+ ++argv;
// Determine the <host-prefix> for this command.
std::string host_prefix;
diff --git a/adb/daemon/services.cpp b/adb/daemon/services.cpp
index 25024b0..1f59d64 100644
--- a/adb/daemon/services.cpp
+++ b/adb/daemon/services.cpp
@@ -157,7 +157,7 @@
return unique_fd{};
}
VLOG(SERVICES) << "service socketpair: " << s[0] << ", " << s[1];
- if (handle_forward_request(command, transport, s[1]) < 0) {
+ if (!handle_forward_request(command, transport, s[1])) {
SendFail(s[1], "not a reverse forwarding command");
}
adb_close(s[1]);
diff --git a/adb/transport.cpp b/adb/transport.cpp
index 793c283..9222008 100644
--- a/adb/transport.cpp
+++ b/adb/transport.cpp
@@ -50,6 +50,7 @@
#include "adb_trace.h"
#include "adb_utils.h"
#include "fdevent.h"
+#include "sysdeps/chrono.h"
static void register_transport(atransport* transport);
static void remove_transport(atransport* transport);
@@ -80,6 +81,7 @@
~ScopedAssumeLocked() RELEASE() {}
};
+#if ADB_HOST
// Tracks and handles atransport*s that are attempting reconnection.
class ReconnectHandler {
public:
@@ -102,12 +104,18 @@
// Tracks a reconnection attempt.
struct ReconnectAttempt {
atransport* transport;
- std::chrono::system_clock::time_point deadline;
+ std::chrono::steady_clock::time_point reconnect_time;
size_t attempts_left;
+
+ bool operator<(const ReconnectAttempt& rhs) const {
+ // std::priority_queue returns the largest element first, so we want attempts that have
+ // less time remaining (i.e. smaller time_points) to compare greater.
+ return reconnect_time > rhs.reconnect_time;
+ }
};
// Only retry for up to one minute.
- static constexpr const std::chrono::seconds kDefaultTimeout = std::chrono::seconds(10);
+ static constexpr const std::chrono::seconds kDefaultTimeout = 10s;
static constexpr const size_t kMaxAttempts = 6;
// Protects all members.
@@ -115,7 +123,7 @@
bool running_ GUARDED_BY(reconnect_mutex_) = true;
std::thread handler_thread_;
std::condition_variable reconnect_cv_;
- std::queue<ReconnectAttempt> reconnect_queue_ GUARDED_BY(reconnect_mutex_);
+ std::priority_queue<ReconnectAttempt> reconnect_queue_ GUARDED_BY(reconnect_mutex_);
DISALLOW_COPY_AND_ASSIGN(ReconnectHandler);
};
@@ -137,7 +145,7 @@
// Drain the queue to free all resources.
std::lock_guard<std::mutex> lock(reconnect_mutex_);
while (!reconnect_queue_.empty()) {
- ReconnectAttempt attempt = reconnect_queue_.front();
+ ReconnectAttempt attempt = reconnect_queue_.top();
reconnect_queue_.pop();
remove_transport(attempt.transport);
}
@@ -148,9 +156,10 @@
{
std::lock_guard<std::mutex> lock(reconnect_mutex_);
if (!running_) return;
- reconnect_queue_.emplace(ReconnectAttempt{
- transport, std::chrono::system_clock::now() + ReconnectHandler::kDefaultTimeout,
- ReconnectHandler::kMaxAttempts});
+ // Arbitrary sleep to give adbd time to get ready, if we disconnected because it exited.
+ auto reconnect_time = std::chrono::steady_clock::now() + 250ms;
+ reconnect_queue_.emplace(
+ ReconnectAttempt{transport, reconnect_time, ReconnectHandler::kMaxAttempts});
}
reconnect_cv_.notify_one();
}
@@ -162,15 +171,27 @@
std::unique_lock<std::mutex> lock(reconnect_mutex_);
ScopedAssumeLocked assume_lock(reconnect_mutex_);
- auto deadline = std::chrono::time_point<std::chrono::system_clock>::max();
- if (!reconnect_queue_.empty()) deadline = reconnect_queue_.front().deadline;
- reconnect_cv_.wait_until(lock, deadline, [&]() REQUIRES(reconnect_mutex_) {
- return !running_ ||
- (!reconnect_queue_.empty() && reconnect_queue_.front().deadline < deadline);
- });
+ if (!reconnect_queue_.empty()) {
+ // FIXME: libstdc++ (used on Windows) implements condition_variable with
+ // system_clock as its clock, so we're probably hosed if the clock changes,
+ // even if we use steady_clock throughout. This problem goes away once we
+ // switch to libc++.
+ reconnect_cv_.wait_until(lock, reconnect_queue_.top().reconnect_time);
+ } else {
+ reconnect_cv_.wait(lock);
+ }
if (!running_) return;
- attempt = reconnect_queue_.front();
+ if (reconnect_queue_.empty()) continue;
+
+ // Go back to sleep in case |reconnect_cv_| woke up spuriously and we still
+ // have more time to wait for the current attempt.
+ auto now = std::chrono::steady_clock::now();
+ if (reconnect_queue_.top().reconnect_time > now) {
+ continue;
+ }
+
+ attempt = reconnect_queue_.top();
reconnect_queue_.pop();
if (attempt.transport->kicked()) {
D("transport %s was kicked. giving up on it.", attempt.transport->serial.c_str());
@@ -191,9 +212,9 @@
std::lock_guard<std::mutex> lock(reconnect_mutex_);
reconnect_queue_.emplace(ReconnectAttempt{
- attempt.transport,
- std::chrono::system_clock::now() + ReconnectHandler::kDefaultTimeout,
- attempt.attempts_left - 1});
+ attempt.transport,
+ std::chrono::steady_clock::now() + ReconnectHandler::kDefaultTimeout,
+ attempt.attempts_left - 1});
continue;
}
@@ -204,6 +225,8 @@
static auto& reconnect_handler = *new ReconnectHandler();
+#endif
+
} // namespace
TransportId NextTransportId() {
@@ -677,9 +700,11 @@
update_transports();
}
+#if ADB_HOST
void init_reconnect_handler(void) {
reconnect_handler.Start();
}
+#endif
void init_transport_registration(void) {
int s[2];
@@ -698,7 +723,9 @@
}
void kick_all_transports() {
+#if ADB_HOST
reconnect_handler.Stop();
+#endif
// To avoid only writing part of a packet to a transport after exit, kick all transports.
std::lock_guard<std::recursive_mutex> lock(transport_lock);
for (auto t : transport_list) {
@@ -736,13 +763,19 @@
t->ref_count--;
if (t->ref_count == 0) {
t->connection()->Stop();
+#if ADB_HOST
if (t->IsTcpDevice() && !t->kicked()) {
- D("transport: %s unref (attempting reconnection) %d", t->serial.c_str(), t->kicked());
+ D("transport: %s unref (attempting reconnection)", t->serial.c_str());
reconnect_handler.TrackTransport(t);
} else {
D("transport: %s unref (kicking and closing)", t->serial.c_str());
remove_transport(t);
}
+#else
+ D("transport: %s unref (kicking and closing)", t->serial.c_str());
+ remove_transport(t);
+#endif
+
} else {
D("transport: %s unref (count=%zu)", t->serial.c_str(), t->ref_count);
}