From 3bd6c7c9137fadf2225fba428a914f14362dd275 Mon Sep 17 00:00:00 2001 From: tonyg Date: Mon, 13 Oct 2014 15:33:10 -0700 Subject: [PATCH] Make host port availability detection more robust. Previously, it only detected ports that were bound to *, localhost or 127.0.0.1. This detects ports bound to any interface. I noticed this while investigating 294878, and it could certainly be a source of flake, but I don't think it's the main one. BUG=294878 Review URL: https://codereview.chromium.org/634803002 Cr-Original-Commit-Position: refs/heads/master@{#299379} Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src Cr-Mirrored-Commit: da7e64810f0305512cd20f0301c2ac1b21d8bc70 --- android/pylib/chrome_test_server_spawner.py | 21 +++++++-------- android/pylib/ports.py | 30 +++++++++------------ 2 files changed, 22 insertions(+), 29 deletions(-) diff --git a/android/pylib/chrome_test_server_spawner.py b/android/pylib/chrome_test_server_spawner.py index e1fe7b18b..052c2fde7 100644 --- a/android/pylib/chrome_test_server_spawner.py +++ b/android/pylib/chrome_test_server_spawner.py @@ -64,17 +64,14 @@ def _WaitUntil(predicate, max_attempts=5): return False -def _CheckPortStatus(port, expected_status): - """Returns True if port has expected_status. +def _CheckPortAvailable(port): + """Returns True if |port| is available.""" + return _WaitUntil(lambda: ports.IsHostPortAvailable(port)) - Args: - port: the port number. - expected_status: boolean of expected status. - Returns: - Returns True if the status is expected. Otherwise returns False. - """ - return _WaitUntil(lambda: ports.IsHostPortUsed(port) == expected_status) +def _CheckPortNotAvailable(port): + """Returns True if |port| is not available.""" + return _WaitUntil(lambda: not ports.IsHostPortAvailable(port)) def _CheckDevicePortStatus(device, port): @@ -167,7 +164,7 @@ class TestServerThread(threading.Thread): port_json = json.loads(port_json) if port_json.has_key('port') and isinstance(port_json['port'], int): self.host_port = port_json['port'] - return _CheckPortStatus(self.host_port, True) + return _CheckPortNotAvailable(self.host_port) logging.error('Failed to get port information from the server data.') return False @@ -236,7 +233,7 @@ class TestServerThread(threading.Thread): if self.pipe_out: self.is_ready = self._WaitToStartAndGetPortFromTestServer() else: - self.is_ready = _CheckPortStatus(self.host_port, True) + self.is_ready = _CheckPortNotAvailable(self.host_port) if self.is_ready: Forwarder.Map([(0, self.host_port)], self.device, self.tool) # Check whether the forwarder is ready on the device. @@ -346,7 +343,7 @@ class SpawningServerRequestHandler(BaseHTTPServer.BaseHTTPRequestHandler): logging.info('Handling request to kill a test server on port: %d.', port) self.server.test_server_instance.Stop() # Make sure the status of test server is correct before sending response. - if _CheckPortStatus(port, False): + if _CheckPortAvailable(port): self._SendResponse(200, 'OK', {}, 'killed') logging.info('Test server on port %d is killed', port) else: diff --git a/android/pylib/ports.py b/android/pylib/ports.py index 34efb5253..578152cb0 100644 --- a/android/pylib/ports.py +++ b/android/pylib/ports.py @@ -9,11 +9,9 @@ import fcntl import httplib import logging import os -import re import socket import traceback -from pylib import cmd_helper from pylib import constants @@ -57,7 +55,7 @@ def AllocateTestServerPort(): with open(constants.TEST_SERVER_PORT_FILE, 'r+') as fp: port = int(fp.read()) ports_tried.append(port) - while IsHostPortUsed(port): + while not IsHostPortAvailable(port): port += 1 ports_tried.append(port) if (port > constants.TEST_SERVER_PORT_LAST or @@ -67,7 +65,7 @@ def AllocateTestServerPort(): fp.seek(0, os.SEEK_SET) fp.write('%d' % (port + 1)) except Exception as e: - logging.info(e) + logging.error(e) finally: if fp_lock: fcntl.flock(fp_lock, fcntl.LOCK_UN) @@ -80,25 +78,23 @@ def AllocateTestServerPort(): return port -def IsHostPortUsed(host_port): - """Checks whether the specified host port is used or not. - - Uses -n -P to inhibit the conversion of host/port numbers to host/port names. +def IsHostPortAvailable(host_port): + """Checks whether the specified host port is available. Args: - host_port: Port on host we want to check. + host_port: Port on host to check. Returns: - True if the port on host is already used, otherwise returns False. + True if the port on host is available, otherwise returns False. """ - port_info = '(\*)|(127\.0\.0\.1)|(localhost):%d' % host_port - # TODO(jnd): Find a better way to filter the port. Note that connecting to the - # socket and closing it would leave it in the TIME_WAIT state. Setting - # SO_LINGER on it and then closing it makes the Python HTTP server crash. - re_port = re.compile(port_info, re.MULTILINE) - if re_port.search(cmd_helper.GetCmdOutput(['lsof', '-nPi:%d' % host_port])): + s = socket.socket() + try: + s.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) + s.bind(('', host_port)) + s.close() return True - return False + except socket.error: + return False def IsDevicePortUsed(device, device_port, state=''):