summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorTreehugger Robot2018-07-31 23:19:16 -0500
committerGerrit Code Review2018-07-31 23:19:16 -0500
commit24f9fc7feed8735aea1588940a63c636e07e1ee8 (patch)
treeb874e08eae2a8c72911b13ef19f6db8afb0456cf
parent8b8d7d67f21cf7331677a2911665e7d3518207b7 (diff)
parentdef91c0abff84d6b51db507965cfb5f543538a80 (diff)
downloadplatform-system-core-24f9fc7feed8735aea1588940a63c636e07e1ee8.tar.gz
platform-system-core-24f9fc7feed8735aea1588940a63c636e07e1ee8.tar.xz
platform-system-core-24f9fc7feed8735aea1588940a63c636e07e1ee8.zip
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.
-rw-r--r--adb/adb.cpp84
-rw-r--r--adb/adb.h5
-rw-r--r--adb/client/commandline.cpp2
-rw-r--r--adb/daemon/services.cpp2
-rw-r--r--adb/transport.cpp69
5 files changed, 105 insertions, 57 deletions
diff --git a/adb/adb.cpp b/adb/adb.cpp
index 19300f660..38c6f62c9 100644
--- a/adb/adb.cpp
+++ b/adb/adb.cpp
@@ -920,13 +920,45 @@ int launch_server(const std::string& socket_spec) {
920} 920}
921#endif /* ADB_HOST */ 921#endif /* ADB_HOST */
922 922
923bool handle_forward_request(const char* service, atransport* transport, int reply_fd) {
924 return handle_forward_request(service, [transport](std::string*) { return transport; },
925 reply_fd);
926}
927
923// Try to handle a network forwarding request. 928// Try to handle a network forwarding request.
924// This returns 1 on success, 0 on failure, and -1 to indicate this is not 929bool handle_forward_request(const char* service,
925// a forwarding-related request. 930 std::function<atransport*(std::string* error)> transport_acquirer,
926int handle_forward_request(const char* service, atransport* transport, int reply_fd) { 931 int reply_fd) {
932 if (!strcmp(service, "list-forward")) {
933 // Create the list of forward redirections.
934 std::string listeners = format_listeners();
935#if ADB_HOST
936 SendOkay(reply_fd);
937#endif
938 SendProtocolString(reply_fd, listeners);
939 return true;
940 }
941
942 if (!strcmp(service, "killforward-all")) {
943 remove_all_listeners();
944#if ADB_HOST
945 /* On the host: 1st OKAY is connect, 2nd OKAY is status */
946 SendOkay(reply_fd);
947#endif
948 SendOkay(reply_fd);
949 return true;
950 }
951
927 if (!strncmp(service, "forward:", 8) || !strncmp(service, "killforward:", 12)) { 952 if (!strncmp(service, "forward:", 8) || !strncmp(service, "killforward:", 12)) {
928 // killforward:local 953 // killforward:local
929 // forward:(norebind:)?local;remote 954 // forward:(norebind:)?local;remote
955 std::string error;
956 atransport* transport = transport_acquirer(&error);
957 if (!transport) {
958 SendFail(reply_fd, error);
959 return true;
960 }
961
930 bool kill_forward = false; 962 bool kill_forward = false;
931 bool no_rebind = false; 963 bool no_rebind = false;
932 if (android::base::StartsWith(service, "killforward:")) { 964 if (android::base::StartsWith(service, "killforward:")) {
@@ -946,17 +978,16 @@ int handle_forward_request(const char* service, atransport* transport, int reply
946 // Check killforward: parameter format: '<local>' 978 // Check killforward: parameter format: '<local>'
947 if (pieces.size() != 1 || pieces[0].empty()) { 979 if (pieces.size() != 1 || pieces[0].empty()) {
948 SendFail(reply_fd, android::base::StringPrintf("bad killforward: %s", service)); 980 SendFail(reply_fd, android::base::StringPrintf("bad killforward: %s", service));
949 return 1; 981 return true;
950 } 982 }
951 } else { 983 } else {
952 // Check forward: parameter format: '<local>;<remote>' 984 // Check forward: parameter format: '<local>;<remote>'
953 if (pieces.size() != 2 || pieces[0].empty() || pieces[1].empty() || pieces[1][0] == '*') { 985 if (pieces.size() != 2 || pieces[0].empty() || pieces[1].empty() || pieces[1][0] == '*') {
954 SendFail(reply_fd, android::base::StringPrintf("bad forward: %s", service)); 986 SendFail(reply_fd, android::base::StringPrintf("bad forward: %s", service));
955 return 1; 987 return true;
956 } 988 }
957 } 989 }
958 990
959 std::string error;
960 InstallStatus r; 991 InstallStatus r;
961 int resolved_tcp_port = 0; 992 int resolved_tcp_port = 0;
962 if (kill_forward) { 993 if (kill_forward) {
@@ -977,7 +1008,7 @@ int handle_forward_request(const char* service, atransport* transport, int reply
977 SendProtocolString(reply_fd, android::base::StringPrintf("%d", resolved_tcp_port)); 1008 SendProtocolString(reply_fd, android::base::StringPrintf("%d", resolved_tcp_port));
978 } 1009 }
979 1010
980 return 1; 1011 return true;
981 } 1012 }
982 1013
983 std::string message; 1014 std::string message;
@@ -996,9 +1027,10 @@ int handle_forward_request(const char* service, atransport* transport, int reply
996 break; 1027 break;
997 } 1028 }
998 SendFail(reply_fd, message); 1029 SendFail(reply_fd, message);
999 return 1; 1030 return true;
1000 } 1031 }
1001 return 0; 1032
1033 return false;
1002} 1034}
1003 1035
1004#if ADB_HOST 1036#if ADB_HOST
@@ -1186,35 +1218,15 @@ int handle_host_request(const char* service, TransportType type, const char* ser
1186 return SendOkay(reply_fd, response); 1218 return SendOkay(reply_fd, response);
1187 } 1219 }
1188 1220
1189 if (!strcmp(service, "list-forward")) { 1221 if (handle_forward_request(service,
1190 // Create the list of forward redirections. 1222 [=](std::string* error) {
1191 std::string listeners = format_listeners(); 1223 return acquire_one_transport(type, serial, transport_id, nullptr,
1192#if ADB_HOST 1224 error);
1193 SendOkay(reply_fd); 1225 },
1194#endif 1226 reply_fd)) {
1195 return SendProtocolString(reply_fd, listeners); 1227 return 0;
1196 }
1197
1198 if (!strcmp(service, "killforward-all")) {
1199 remove_all_listeners();
1200#if ADB_HOST
1201 /* On the host: 1st OKAY is connect, 2nd OKAY is status */
1202 SendOkay(reply_fd);
1203#endif
1204 SendOkay(reply_fd);
1205 return 1;
1206 }
1207
1208 std::string error;
1209 atransport* t = acquire_one_transport(type, serial, transport_id, nullptr, &error);
1210 if (!t) {
1211 SendFail(reply_fd, error);
1212 return 1;
1213 } 1228 }
1214 1229
1215 int ret = handle_forward_request(service, t, reply_fd);
1216 if (ret >= 0)
1217 return ret - 1;
1218 return -1; 1230 return -1;
1219} 1231}
1220 1232
diff --git a/adb/adb.h b/adb/adb.h
index 13ca4d7e0..e6af780c4 100644
--- a/adb/adb.h
+++ b/adb/adb.h
@@ -158,7 +158,10 @@ asocket* create_jdwp_tracker_service_socket();
158unique_fd create_jdwp_connection_fd(int jdwp_pid); 158unique_fd create_jdwp_connection_fd(int jdwp_pid);
159#endif 159#endif
160 160
161int handle_forward_request(const char* service, atransport* transport, int reply_fd); 161bool handle_forward_request(const char* service, atransport* transport, int reply_fd);
162bool handle_forward_request(const char* service,
163 std::function<atransport*(std::string* error)> transport_acquirer,
164 int reply_fd);
162 165
163/* packet allocator */ 166/* packet allocator */
164apacket* get_apacket(void); 167apacket* get_apacket(void);
diff --git a/adb/client/commandline.cpp b/adb/client/commandline.cpp
index a7a94e775..3fb14f351 100644
--- a/adb/client/commandline.cpp
+++ b/adb/client/commandline.cpp
@@ -1614,9 +1614,9 @@ int adb_commandline(int argc, const char** argv) {
1614 return bugreport.DoIt(argc, argv); 1614 return bugreport.DoIt(argc, argv);
1615 } else if (!strcmp(argv[0], "forward") || !strcmp(argv[0], "reverse")) { 1615 } else if (!strcmp(argv[0], "forward") || !strcmp(argv[0], "reverse")) {
1616 bool reverse = !strcmp(argv[0], "reverse"); 1616 bool reverse = !strcmp(argv[0], "reverse");
1617 ++argv;
1618 --argc; 1617 --argc;
1619 if (argc < 1) return syntax_error("%s requires an argument", argv[0]); 1618 if (argc < 1) return syntax_error("%s requires an argument", argv[0]);
1619 ++argv;
1620 1620
1621 // Determine the <host-prefix> for this command. 1621 // Determine the <host-prefix> for this command.
1622 std::string host_prefix; 1622 std::string host_prefix;
diff --git a/adb/daemon/services.cpp b/adb/daemon/services.cpp
index 25024b05a..1f59d6446 100644
--- a/adb/daemon/services.cpp
+++ b/adb/daemon/services.cpp
@@ -157,7 +157,7 @@ unique_fd reverse_service(const char* command, atransport* transport) {
157 return unique_fd{}; 157 return unique_fd{};
158 } 158 }
159 VLOG(SERVICES) << "service socketpair: " << s[0] << ", " << s[1]; 159 VLOG(SERVICES) << "service socketpair: " << s[0] << ", " << s[1];
160 if (handle_forward_request(command, transport, s[1]) < 0) { 160 if (!handle_forward_request(command, transport, s[1])) {
161 SendFail(s[1], "not a reverse forwarding command"); 161 SendFail(s[1], "not a reverse forwarding command");
162 } 162 }
163 adb_close(s[1]); 163 adb_close(s[1]);
diff --git a/adb/transport.cpp b/adb/transport.cpp
index 793c283fc..922200826 100644
--- a/adb/transport.cpp
+++ b/adb/transport.cpp
@@ -50,6 +50,7 @@
50#include "adb_trace.h" 50#include "adb_trace.h"
51#include "adb_utils.h" 51#include "adb_utils.h"
52#include "fdevent.h" 52#include "fdevent.h"
53#include "sysdeps/chrono.h"
53 54
54static void register_transport(atransport* transport); 55static void register_transport(atransport* transport);
55static void remove_transport(atransport* transport); 56static void remove_transport(atransport* transport);
@@ -80,6 +81,7 @@ class SCOPED_CAPABILITY ScopedAssumeLocked {
80 ~ScopedAssumeLocked() RELEASE() {} 81 ~ScopedAssumeLocked() RELEASE() {}
81}; 82};
82 83
84#if ADB_HOST
83// Tracks and handles atransport*s that are attempting reconnection. 85// Tracks and handles atransport*s that are attempting reconnection.
84class ReconnectHandler { 86class ReconnectHandler {
85 public: 87 public:
@@ -102,12 +104,18 @@ class ReconnectHandler {
102 // Tracks a reconnection attempt. 104 // Tracks a reconnection attempt.
103 struct ReconnectAttempt { 105 struct ReconnectAttempt {
104 atransport* transport; 106 atransport* transport;
105 std::chrono::system_clock::time_point deadline; 107 std::chrono::steady_clock::time_point reconnect_time;
106 size_t attempts_left; 108 size_t attempts_left;
109
110 bool operator<(const ReconnectAttempt& rhs) const {
111 // std::priority_queue returns the largest element first, so we want attempts that have
112 // less time remaining (i.e. smaller time_points) to compare greater.
113 return reconnect_time > rhs.reconnect_time;
114 }
107 }; 115 };
108 116
109 // Only retry for up to one minute. 117 // Only retry for up to one minute.
110 static constexpr const std::chrono::seconds kDefaultTimeout = std::chrono::seconds(10); 118 static constexpr const std::chrono::seconds kDefaultTimeout = 10s;
111 static constexpr const size_t kMaxAttempts = 6; 119 static constexpr const size_t kMaxAttempts = 6;
112 120
113 // Protects all members. 121 // Protects all members.
@@ -115,7 +123,7 @@ class ReconnectHandler {
115 bool running_ GUARDED_BY(reconnect_mutex_) = true; 123 bool running_ GUARDED_BY(reconnect_mutex_) = true;
116 std::thread handler_thread_; 124 std::thread handler_thread_;
117 std::condition_variable reconnect_cv_; 125 std::condition_variable reconnect_cv_;
118 std::queue<ReconnectAttempt> reconnect_queue_ GUARDED_BY(reconnect_mutex_); 126 std::priority_queue<ReconnectAttempt> reconnect_queue_ GUARDED_BY(reconnect_mutex_);
119 127
120 DISALLOW_COPY_AND_ASSIGN(ReconnectHandler); 128 DISALLOW_COPY_AND_ASSIGN(ReconnectHandler);
121}; 129};
@@ -137,7 +145,7 @@ void ReconnectHandler::Stop() {
137 // Drain the queue to free all resources. 145 // Drain the queue to free all resources.
138 std::lock_guard<std::mutex> lock(reconnect_mutex_); 146 std::lock_guard<std::mutex> lock(reconnect_mutex_);
139 while (!reconnect_queue_.empty()) { 147 while (!reconnect_queue_.empty()) {
140 ReconnectAttempt attempt = reconnect_queue_.front(); 148 ReconnectAttempt attempt = reconnect_queue_.top();
141 reconnect_queue_.pop(); 149 reconnect_queue_.pop();
142 remove_transport(attempt.transport); 150 remove_transport(attempt.transport);
143 } 151 }
@@ -148,9 +156,10 @@ void ReconnectHandler::TrackTransport(atransport* transport) {
148 { 156 {
149 std::lock_guard<std::mutex> lock(reconnect_mutex_); 157 std::lock_guard<std::mutex> lock(reconnect_mutex_);
150 if (!running_) return; 158 if (!running_) return;
151 reconnect_queue_.emplace(ReconnectAttempt{ 159 // Arbitrary sleep to give adbd time to get ready, if we disconnected because it exited.
152 transport, std::chrono::system_clock::now() + ReconnectHandler::kDefaultTimeout, 160 auto reconnect_time = std::chrono::steady_clock::now() + 250ms;
153 ReconnectHandler::kMaxAttempts}); 161 reconnect_queue_.emplace(
162 ReconnectAttempt{transport, reconnect_time, ReconnectHandler::kMaxAttempts});
154 } 163 }
155 reconnect_cv_.notify_one(); 164 reconnect_cv_.notify_one();
156} 165}
@@ -162,15 +171,27 @@ void ReconnectHandler::Run() {
162 std::unique_lock<std::mutex> lock(reconnect_mutex_); 171 std::unique_lock<std::mutex> lock(reconnect_mutex_);
163 ScopedAssumeLocked assume_lock(reconnect_mutex_); 172 ScopedAssumeLocked assume_lock(reconnect_mutex_);
164 173
165 auto deadline = std::chrono::time_point<std::chrono::system_clock>::max(); 174 if (!reconnect_queue_.empty()) {
166 if (!reconnect_queue_.empty()) deadline = reconnect_queue_.front().deadline; 175 // FIXME: libstdc++ (used on Windows) implements condition_variable with
167 reconnect_cv_.wait_until(lock, deadline, [&]() REQUIRES(reconnect_mutex_) { 176 // system_clock as its clock, so we're probably hosed if the clock changes,
168 return !running_ || 177 // even if we use steady_clock throughout. This problem goes away once we
169 (!reconnect_queue_.empty() && reconnect_queue_.front().deadline < deadline); 178 // switch to libc++.
170 }); 179 reconnect_cv_.wait_until(lock, reconnect_queue_.top().reconnect_time);
180 } else {
181 reconnect_cv_.wait(lock);
182 }
171 183
172 if (!running_) return; 184 if (!running_) return;
173 attempt = reconnect_queue_.front(); 185 if (reconnect_queue_.empty()) continue;
186
187 // Go back to sleep in case |reconnect_cv_| woke up spuriously and we still
188 // have more time to wait for the current attempt.
189 auto now = std::chrono::steady_clock::now();
190 if (reconnect_queue_.top().reconnect_time > now) {
191 continue;
192 }
193
194 attempt = reconnect_queue_.top();
174 reconnect_queue_.pop(); 195 reconnect_queue_.pop();
175 if (attempt.transport->kicked()) { 196 if (attempt.transport->kicked()) {
176 D("transport %s was kicked. giving up on it.", attempt.transport->serial.c_str()); 197 D("transport %s was kicked. giving up on it.", attempt.transport->serial.c_str());
@@ -191,9 +212,9 @@ void ReconnectHandler::Run() {
191 212
192 std::lock_guard<std::mutex> lock(reconnect_mutex_); 213 std::lock_guard<std::mutex> lock(reconnect_mutex_);
193 reconnect_queue_.emplace(ReconnectAttempt{ 214 reconnect_queue_.emplace(ReconnectAttempt{
194 attempt.transport, 215 attempt.transport,
195 std::chrono::system_clock::now() + ReconnectHandler::kDefaultTimeout, 216 std::chrono::steady_clock::now() + ReconnectHandler::kDefaultTimeout,
196 attempt.attempts_left - 1}); 217 attempt.attempts_left - 1});
197 continue; 218 continue;
198 } 219 }
199 220
@@ -204,6 +225,8 @@ void ReconnectHandler::Run() {
204 225
205static auto& reconnect_handler = *new ReconnectHandler(); 226static auto& reconnect_handler = *new ReconnectHandler();
206 227
228#endif
229
207} // namespace 230} // namespace
208 231
209TransportId NextTransportId() { 232TransportId NextTransportId() {
@@ -677,9 +700,11 @@ static void transport_registration_func(int _fd, unsigned ev, void*) {
677 update_transports(); 700 update_transports();
678} 701}
679 702
703#if ADB_HOST
680void init_reconnect_handler(void) { 704void init_reconnect_handler(void) {
681 reconnect_handler.Start(); 705 reconnect_handler.Start();
682} 706}
707#endif
683 708
684void init_transport_registration(void) { 709void init_transport_registration(void) {
685 int s[2]; 710 int s[2];
@@ -698,7 +723,9 @@ void init_transport_registration(void) {
698} 723}
699 724
700void kick_all_transports() { 725void kick_all_transports() {
726#if ADB_HOST
701 reconnect_handler.Stop(); 727 reconnect_handler.Stop();
728#endif
702 // To avoid only writing part of a packet to a transport after exit, kick all transports. 729 // To avoid only writing part of a packet to a transport after exit, kick all transports.
703 std::lock_guard<std::recursive_mutex> lock(transport_lock); 730 std::lock_guard<std::recursive_mutex> lock(transport_lock);
704 for (auto t : transport_list) { 731 for (auto t : transport_list) {
@@ -736,13 +763,19 @@ static void transport_unref(atransport* t) {
736 t->ref_count--; 763 t->ref_count--;
737 if (t->ref_count == 0) { 764 if (t->ref_count == 0) {
738 t->connection()->Stop(); 765 t->connection()->Stop();
766#if ADB_HOST
739 if (t->IsTcpDevice() && !t->kicked()) { 767 if (t->IsTcpDevice() && !t->kicked()) {
740 D("transport: %s unref (attempting reconnection) %d", t->serial.c_str(), t->kicked()); 768 D("transport: %s unref (attempting reconnection)", t->serial.c_str());
741 reconnect_handler.TrackTransport(t); 769 reconnect_handler.TrackTransport(t);
742 } else { 770 } else {
743 D("transport: %s unref (kicking and closing)", t->serial.c_str()); 771 D("transport: %s unref (kicking and closing)", t->serial.c_str());
744 remove_transport(t); 772 remove_transport(t);
745 } 773 }
774#else
775 D("transport: %s unref (kicking and closing)", t->serial.c_str());
776 remove_transport(t);
777#endif
778
746 } else { 779 } else {
747 D("transport: %s unref (count=%zu)", t->serial.c_str(), t->ref_count); 780 D("transport: %s unref (count=%zu)", t->serial.c_str(), t->ref_count);
748 } 781 }