adb: fix adb client running out of sockets on Windows
Background
==========
On Windows, if you run "adb shell exit" in a loop in two windows,
eventually the adb client will be unable to connect to the adb server. I
think connect() is returning WSAEADDRINUSE: "Only one usage of each
socket address (protocol/network address/port) is normally permitted.
(10048)". The Windows System Event Log may also show Event 4227, Tcpip.
Netstat output is filled with:
# for the adb server
TCP 127.0.0.1:5037 127.0.0.1:65523 TIME_WAIT
# for the adb client
TCP 127.0.0.1:65523 127.0.0.1:5037 TIME_WAIT
The error probably means that the client is running out of free
address:port pairs.
The first netstat line is unavoidable, but the second line exists
because the adb client is not waiting for orderly/graceful shutdown of
the socket, and that is apparently required on Windows to get rid of the
second line. For more info, see
https://github.com/CompareAndSwap/SocketCloseTest .
This is exacerbated by the fact that "adb shell exit" makes 4 socket
connections to the adb server: 1) host:version, 2) host:features, 3)
host:version (again), 4) shell:exit. Also exacerbating is the fact that
the adb protocol is length-prefixed so the client typically does not
have to 'read() until zero' which effectively waits for orderly/graceful
shutdown.
The Fix
=======
Introduce a function, ReadOrderlyShutdown(), that should be called in
the adb client to wait for the server to close its socket, before
closing the client socket.
I reviewed all code where the adb client makes a connection to the adb
server and added ReadOrderlyShutdown() when it made sense. I wasn't able
to add it to the following:
* interactive_shell: this doesn't matter because this is interactive and
thus can't be run fast enough to use up ports.
* adb sideload: I couldn't get enough test coverage and I don't think
this is being called frequently enough to be a problem.
* send_shell_command, backup, adb_connect_command, adb shell, adb
exec-out, install_multiple_app, adb_send_emulator_command: These
already wait for server socket shutdown since they already call
recv() until zero.
* restore, adb exec-in: protocol design can't have the server close
first.
* adb start-server: no fd is actually returned
* create_local_service_socket, local_connect_arbitrary_ports,
connect_device: probably called rarely enough not to be a problem.
Also in this change
===================
* Clarify comments in when adb_shutdown() is called before exit().
* add some missing adb_close() in adb sideload.
* Fixup error handling and comments in adb_send_emulator_command().
* Make SyncConnection::SendQuit return a success boolean.
* Add unittest for adb emu kill command. This gets code coverage over
this very careful piece of code.
Change-Id: Iad0b1336f5b74186af2cd35f7ea827d0fa77a17c
Signed-off-by: Spencer Low <CompareAndSwap@gmail.com>
diff --git a/adb/console.cpp b/adb/console.cpp
index ba5a72b..5a9c6ab 100644
--- a/adb/console.cpp
+++ b/adb/console.cpp
@@ -25,6 +25,7 @@
#include "adb.h"
#include "adb_client.h"
+#include "adb_io.h"
// Return the console port of the currently connected emulator (if any) or -1 if
// there is no emulator, and -2 if there is more than one.
@@ -87,14 +88,20 @@
return 1;
}
+ std::string commands;
+
for (int i = 1; i < argc; i++) {
- adb_write(fd, argv[i], strlen(argv[i]));
- adb_write(fd, i == argc - 1 ? "\n" : " ", 1);
+ commands.append(argv[i]);
+ commands.append(i == argc - 1 ? "\n" : " ");
}
- const char disconnect_command[] = "quit\n";
- if (adb_write(fd, disconnect_command, sizeof(disconnect_command) - 1) == -1) {
- LOG(FATAL) << "Could not finalize emulator command";
+ commands.append("quit\n");
+
+ if (!WriteFdExactly(fd, commands)) {
+ fprintf(stderr, "error: cannot write to emulator: %s\n",
+ strerror(errno));
+ adb_close(fd);
+ return 1;
}
// Drain output that the emulator console has sent us to prevent a problem
@@ -106,11 +113,14 @@
do {
char buf[BUFSIZ];
result = adb_read(fd, buf, sizeof(buf));
- // Keep reading until zero bytes (EOF) or an error. If 'adb emu kill'
- // is executed, the emulator calls exit() which causes adb to get
- // ECONNRESET. Any other emu command is followed by the quit command
- // that we sent above, and that causes the emulator to close the socket
- // which should cause zero bytes (EOF) to be returned.
+ // Keep reading until zero bytes (orderly/graceful shutdown) or an
+ // error. If 'adb emu kill' is executed, the emulator calls exit() with
+ // the socket open (and shutdown(SD_SEND) was not called), which causes
+ // Windows to send a TCP RST segment which causes adb to get ECONNRESET.
+ // Any other emu command is followed by the quit command that we
+ // appended above, and that causes the emulator to close the socket
+ // which should cause zero bytes (orderly/graceful shutdown) to be
+ // returned.
} while (result > 0);
adb_close(fd);