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/test_adb.py b/adb/test_adb.py
index 5bda22f..0f1b034 100644
--- a/adb/test_adb.py
+++ b/adb/test_adb.py
@@ -21,8 +21,11 @@
"""
from __future__ import print_function
+import contextlib
import os
import random
+import socket
+import struct
import subprocess
import threading
import unittest
@@ -140,6 +143,71 @@
subprocess.check_output(['adb', '-P', str(port), 'kill-server'],
stderr=subprocess.STDOUT)
+ # Use SO_LINGER to cause TCP RST segment to be sent on socket close.
+ def _reset_socket_on_close(self, sock):
+ # The linger structure is two shorts on Windows, but two ints on Unix.
+ linger_format = 'hh' if os.name == 'nt' else 'ii'
+ l_onoff = 1
+ l_linger = 0
+
+ sock.setsockopt(socket.SOL_SOCKET, socket.SO_LINGER,
+ struct.pack(linger_format, l_onoff, l_linger))
+ # Verify that we set the linger structure properly by retrieving it.
+ linger = sock.getsockopt(socket.SOL_SOCKET, socket.SO_LINGER, 16)
+ self.assertEqual((l_onoff, l_linger),
+ struct.unpack_from(linger_format, linger))
+
+ def test_emu_kill(self):
+ """Ensure that adb emu kill works.
+
+ Bug: https://code.google.com/p/android/issues/detail?id=21021
+ """
+ port = 12345
+
+ with contextlib.closing(
+ socket.socket(socket.AF_INET, socket.SOCK_STREAM)) as listener:
+ # Use SO_REUSEADDR so subsequent runs of the test can grab the port
+ # even if it is in TIME_WAIT.
+ listener.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
+ listener.bind(('127.0.0.1', port))
+ listener.listen(4)
+
+ # Now that listening has started, start adb emu kill, telling it to
+ # connect to our mock emulator.
+ p = subprocess.Popen(
+ ['adb', '-s', 'emulator-' + str(port), 'emu', 'kill'],
+ stderr=subprocess.STDOUT)
+
+ accepted_connection, addr = listener.accept()
+ with contextlib.closing(accepted_connection) as conn:
+ # If WSAECONNABORTED (10053) is raised by any socket calls,
+ # then adb probably isn't reading the data that we sent it.
+ conn.sendall('Android Console: type \'help\' for a list ' +
+ 'of commands\r\n')
+ conn.sendall('OK\r\n')
+
+ with contextlib.closing(conn.makefile()) as f:
+ self.assertEqual('kill\n', f.readline())
+ self.assertEqual('quit\n', f.readline())
+
+ conn.sendall('OK: killing emulator, bye bye\r\n')
+
+ # Use SO_LINGER to send TCP RST segment to test whether adb
+ # ignores WSAECONNRESET on Windows. This happens with the
+ # real emulator because it just calls exit() without closing
+ # the socket or calling shutdown(SD_SEND). At process
+ # termination, Windows sends a TCP RST segment for every
+ # open socket that shutdown(SD_SEND) wasn't used on.
+ self._reset_socket_on_close(conn)
+
+ # Wait for adb to finish, so we can check return code.
+ p.communicate()
+
+ # If this fails, adb probably isn't ignoring WSAECONNRESET when
+ # reading the response from the adb emu kill command (on Windows).
+ self.assertEqual(0, p.returncode)
+
+
def main():
random.seed(0)
if len(adb.get_devices()) > 0: