Commit da7e6481 authored by tonyg's avatar tonyg Committed by Commit bot

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-Commit-Position: refs/heads/master@{#299379}
parent 8fab32f6
...@@ -64,17 +64,14 @@ def _WaitUntil(predicate, max_attempts=5): ...@@ -64,17 +64,14 @@ def _WaitUntil(predicate, max_attempts=5):
return False return False
def _CheckPortStatus(port, expected_status): def _CheckPortAvailable(port):
"""Returns True if port has expected_status. """Returns True if |port| is available."""
return _WaitUntil(lambda: ports.IsHostPortAvailable(port))
Args:
port: the port number.
expected_status: boolean of expected status.
Returns: def _CheckPortNotAvailable(port):
Returns True if the status is expected. Otherwise returns False. """Returns True if |port| is not available."""
""" return _WaitUntil(lambda: not ports.IsHostPortAvailable(port))
return _WaitUntil(lambda: ports.IsHostPortUsed(port) == expected_status)
def _CheckDevicePortStatus(device, port): def _CheckDevicePortStatus(device, port):
...@@ -167,7 +164,7 @@ class TestServerThread(threading.Thread): ...@@ -167,7 +164,7 @@ class TestServerThread(threading.Thread):
port_json = json.loads(port_json) port_json = json.loads(port_json)
if port_json.has_key('port') and isinstance(port_json['port'], int): if port_json.has_key('port') and isinstance(port_json['port'], int):
self.host_port = port_json['port'] 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.') logging.error('Failed to get port information from the server data.')
return False return False
...@@ -236,7 +233,7 @@ class TestServerThread(threading.Thread): ...@@ -236,7 +233,7 @@ class TestServerThread(threading.Thread):
if self.pipe_out: if self.pipe_out:
self.is_ready = self._WaitToStartAndGetPortFromTestServer() self.is_ready = self._WaitToStartAndGetPortFromTestServer()
else: else:
self.is_ready = _CheckPortStatus(self.host_port, True) self.is_ready = _CheckPortNotAvailable(self.host_port)
if self.is_ready: if self.is_ready:
Forwarder.Map([(0, self.host_port)], self.device, self.tool) Forwarder.Map([(0, self.host_port)], self.device, self.tool)
# Check whether the forwarder is ready on the device. # Check whether the forwarder is ready on the device.
...@@ -346,7 +343,7 @@ class SpawningServerRequestHandler(BaseHTTPServer.BaseHTTPRequestHandler): ...@@ -346,7 +343,7 @@ class SpawningServerRequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
logging.info('Handling request to kill a test server on port: %d.', port) logging.info('Handling request to kill a test server on port: %d.', port)
self.server.test_server_instance.Stop() self.server.test_server_instance.Stop()
# Make sure the status of test server is correct before sending response. # Make sure the status of test server is correct before sending response.
if _CheckPortStatus(port, False): if _CheckPortAvailable(port):
self._SendResponse(200, 'OK', {}, 'killed') self._SendResponse(200, 'OK', {}, 'killed')
logging.info('Test server on port %d is killed', port) logging.info('Test server on port %d is killed', port)
else: else:
......
...@@ -9,11 +9,9 @@ import fcntl ...@@ -9,11 +9,9 @@ import fcntl
import httplib import httplib
import logging import logging
import os import os
import re
import socket import socket
import traceback import traceback
from pylib import cmd_helper
from pylib import constants from pylib import constants
...@@ -57,7 +55,7 @@ def AllocateTestServerPort(): ...@@ -57,7 +55,7 @@ def AllocateTestServerPort():
with open(constants.TEST_SERVER_PORT_FILE, 'r+') as fp: with open(constants.TEST_SERVER_PORT_FILE, 'r+') as fp:
port = int(fp.read()) port = int(fp.read())
ports_tried.append(port) ports_tried.append(port)
while IsHostPortUsed(port): while not IsHostPortAvailable(port):
port += 1 port += 1
ports_tried.append(port) ports_tried.append(port)
if (port > constants.TEST_SERVER_PORT_LAST or if (port > constants.TEST_SERVER_PORT_LAST or
...@@ -67,7 +65,7 @@ def AllocateTestServerPort(): ...@@ -67,7 +65,7 @@ def AllocateTestServerPort():
fp.seek(0, os.SEEK_SET) fp.seek(0, os.SEEK_SET)
fp.write('%d' % (port + 1)) fp.write('%d' % (port + 1))
except Exception as e: except Exception as e:
logging.info(e) logging.error(e)
finally: finally:
if fp_lock: if fp_lock:
fcntl.flock(fp_lock, fcntl.LOCK_UN) fcntl.flock(fp_lock, fcntl.LOCK_UN)
...@@ -80,25 +78,23 @@ def AllocateTestServerPort(): ...@@ -80,25 +78,23 @@ def AllocateTestServerPort():
return port return port
def IsHostPortUsed(host_port): def IsHostPortAvailable(host_port):
"""Checks whether the specified host port is used or not. """Checks whether the specified host port is available.
Uses -n -P to inhibit the conversion of host/port numbers to host/port names.
Args: Args:
host_port: Port on host we want to check. host_port: Port on host to check.
Returns: 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 s = socket.socket()
# TODO(jnd): Find a better way to filter the port. Note that connecting to the try:
# socket and closing it would leave it in the TIME_WAIT state. Setting s.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
# SO_LINGER on it and then closing it makes the Python HTTP server crash. s.bind(('', host_port))
re_port = re.compile(port_info, re.MULTILINE) s.close()
if re_port.search(cmd_helper.GetCmdOutput(['lsof', '-nPi:%d' % host_port])):
return True return True
return False except socket.error:
return False
def IsDevicePortUsed(device, device_port, state=''): def IsDevicePortUsed(device, device_port, state=''):
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment