Changeset 85659 in webkit


Ignore:
Timestamp:
May 3, 2011 2:18:43 PM (13 years ago)
Author:
dpranke@chromium.org
Message:

2011-05-03 Dirk Pranke <​dpranke@chromium.org>

Reviewed by Ojan Vafai.

new-run-webkit-tests: fix http server startup/shutdown

Previous versions of the code had three problems that made startup
and shutdown flaky. The first is that it would throw exceptions
if it couldn't delete stale log files, which was overly
paranoid. The second is that some of the exceptions weren't
defined properly. The third, and most important, is that it was
using urllib to check if ports were available, which was leaving
sockets in a half-closed state, and keeping ports from being
reused. By switching to raw sockets, we are able to now reliably
restart.

This change also switches the code to using Executives to stop
processes, which will let us delete a bunch of code in the
port/* implementations and fix a weird layering problem in a
subsequent patch.

​https://bugs.webkit.org/show_bug.cgi?id=59977

  • Scripts/webkitpy/layout_tests/port/apache_http_server.py:
  • Scripts/webkitpy/layout_tests/port/http_server.py:
  • Scripts/webkitpy/layout_tests/port/http_server_base.py:
  • Scripts/webkitpy/layout_tests/port/websocket_server.py:
Location:
trunk/Tools
Files:
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/Tools/ChangeLog

    r85658 r85659  
     12011-05-03  Dirk Pranke  <dpranke@chromium.org>
     2
     3        Reviewed by Ojan Vafai.
     4
     5        new-run-webkit-tests: fix http server startup/shutdown
     6
     7        Previous versions of the code had three problems that made startup
     8        and shutdown flaky. The first is that it would throw exceptions
     9        if it couldn't delete stale log files, which was overly
     10        paranoid. The second is that some of the exceptions weren't
     11        defined properly. The third, and most important, is that it was
     12        using urllib to check if ports were available, which was leaving
     13        sockets in a half-closed state, and keeping ports from being
     14        reused. By switching to raw sockets, we are able to now reliably
     15        restart.
     16
     17        This change also switches the code to using Executives to stop
     18        processes, which will let us delete a bunch of code in the
     19        port/* implementations and fix a weird layering problem in a
     20        subsequent patch.
     21
     22        https://bugs.webkit.org/show_bug.cgi?id=59977
     23
     24        * Scripts/webkitpy/layout_tests/port/apache_http_server.py:
     25        * Scripts/webkitpy/layout_tests/port/http_server.py:
     26        * Scripts/webkitpy/layout_tests/port/http_server_base.py:
     27        * Scripts/webkitpy/layout_tests/port/websocket_server.py:
     28
    1292011-05-03  Dirk Pranke  <dpranke@chromium.org>
    230

  • trunk/Tools/Scripts/webkitpy/layout_tests/port/apache_http_server.py

    r79633 r85659  
    198198        # Note: Not thread safe: http://bugs.python.org/issue2320
    199199        _log.debug('Starting http server, cmd="%s"' % str(self._start_cmd))
    200         self._httpd_proc = subprocess.Popen(self._start_cmd,
    201                                             stderr=subprocess.PIPE,
    202             shell=True)
    203         err = self._httpd_proc.stderr.read()
     200        self._process = subprocess.Popen(self._start_cmd, stderr=subprocess.PIPE, shell=True)
     201        err = self._process.stderr.read()
    204202        if len(err):
    205203            _log.debug(err)
    … …  
    209207    def start(self):
    210208        """Starts the apache http server."""
    211         # Stop any currently running servers.
    212         self.stop()
     209        if self._process:
     210            raise http_server_base.ServerError('httpd already running')
     211
     212        self.check_that_all_ports_are_available()
    213213
    214214        _log.debug("Starting apache http server")
    … …  
    222222            _log.debug("Server successfully started")
    223223        else:
    224             raise Exception('Failed to start http server')
     224            raise http_server_base.ServerError('Failed to start http server')
    225225
    226226    def stop(self):
    … …  
    228228        _log.debug("Shutting down any running http servers")
    229229        httpd_pid = None
     230        if self._process:
     231            httpd_pid = self._process.pid
     232        elif os.path.exists(self._pid_file):
     233            httpd_pid = int(open(self._pid_file).readline())
     234
     235        if not httpd_pid:
     236            return
     237
     238        self._port_obj._executive.kill_process(httpd_pid)
     239
     240        if self._process:
     241            self._process.wait()
     242            self._process = None
    230243        if os.path.exists(self._pid_file):
    231             httpd_pid = int(open(self._pid_file).readline())
    232         # FIXME: We shouldn't be calling a protected method of _port_obj!
    233         self._port_obj._shut_down_http_server(httpd_pid)
     244            os.remove(self._pid_file)

  • trunk/Tools/Scripts/webkitpy/layout_tests/port/http_server.py

    r85447 r85659  
    5959        http_server_base.HttpServerBase.__init__(self, port_obj)
    6060        self._output_dir = output_dir
    61         self._process = None
    6261        self._port = port
    6362        self._root = root
    … …  
    10099                 'sslcert': self._pem_file}])
    101100
    102     def is_running(self):
    103         return self._process != None
    104 
    105101    def start(self):
    106102        if self.is_running():
    107             raise 'Lighttpd already running'
     103            raise http_server_base.ServerError('Lighttpd already running')
    108104
    109105        base_conf_file = self._port_obj.path_from_webkit_base('Tools',
    … …  
    117113
    118114        # Remove old log files. We only need to keep the last ones.
    119         self.remove_log_files(self._output_dir, "access.log-")
    120         self.remove_log_files(self._output_dir, "error.log-")
     115        # Don't worry too much if we can't remove the old ones. Sometimes
     116        # they are locked by other processes but this clears eventually.
     117        try:
     118            self.remove_log_files(self._output_dir, "access.log-")
     119            self.remove_log_files(self._output_dir, "error.log-")
     120        except OSError, e:
     121            _log.warning('Failed to remove old http server log files')
     122            pass
    121123
    122124        # Write out the config
    … …  
    209211
    210212        env = self._port_obj.setup_environ_for_server()
     213
     214        self.mappings = mappings
     215        self.check_that_all_ports_are_available()
     216
    211217        _log.debug('Starting http server, cmd="%s"' % str(start_cmd))
    212218        # FIXME: Should use Executive.run_command
    213219        self._process = subprocess.Popen(start_cmd, env=env, stdin=subprocess.PIPE)
    214 
    215         # Wait for server to start.
    216         self.mappings = mappings
    217         server_started = self.wait_for_action(
    218             self.is_server_running_on_all_ports)
    219 
    220         # Our process terminated already
     220        server_started = self.wait_for_action(self.is_server_running_on_all_ports)
    221221        if not server_started or self._process.returncode != None:
    222             raise Exception('Failed to start httpd.')
     222            raise http_server_base.ServerError('Failed to start httpd.')
    223223
    224224        _log.debug("Server successfully started")
    225225
    226     # TODO(deanm): Find a nicer way to shutdown cleanly.  Our log files are
    227     # probably not being flushed, etc... why doesn't our python have os.kill ?
    228 
    229     def stop(self, force=False):
    230         if not force and not self.is_running():
     226    # FIXME: It would be nice if we could shut the server down cleanly.
     227    def stop(self):
     228        if not self.is_running():
    231229            return
    232230
    233         httpd_pid = None
    234         if self._process:
    235             httpd_pid = self._process.pid
    236         self._port_obj._shut_down_http_server(httpd_pid)
    237 
    238         if self._process:
    239             # wait() is not threadsafe and can throw OSError due to:
    240             # http://bugs.python.org/issue1731717
    241             self._process.wait()
    242             self._process = None
     231        httpd_pid = self._process.pid
     232        self._executive.kill_process(httpd_pid)
     233
     234        # wait() is not threadsafe and can throw OSError due to:
     235        # http://bugs.python.org/issue1731717
     236        self._process.wait()
     237        self._process = None

  • trunk/Tools/Scripts/webkitpy/layout_tests/port/http_server_base.py

    r76547 r85659  
    3030"""Base class with common routines between the Apache and Lighttpd servers."""
    3131
     32import errno
    3233import logging
    33 import os
     34import socket
    3435import time
    35 import urllib
    3636
    3737from webkitpy.common.system import filesystem
    … …  
    4040
    4141
     42class ServerError(Exception):
     43    pass
     44
     45
    4246class HttpServerBase(object):
    4347
    4448    def __init__(self, port_obj):
    4549        self._port_obj = port_obj
     50        self._executive = port_obj._executive
     51        self._filesystem = port_obj._filesystem
     52        self._process = None
    4653
    4754    def wait_for_action(self, action):
    … …  
    5764        return False
    5865
     66    def is_running(self):
     67        return self._process and self._process.returncode is None
     68
    5969    def is_server_running_on_all_ports(self):
    6070        """Returns whether the server is running on all the desired ports."""
     71        if not self._executive.check_running_pid(self._process.pid):
     72            _log.debug("Server isn't running at all")
     73            raise ServerError("Server exited")
     74
    6175        for mapping in self.mappings:
    62             if 'sslcert' in mapping:
    63                 http_suffix = 's'
    64             else:
    65                 http_suffix = ''
    66 
    67             url = 'http%s://127.0.0.1:%d/' % (http_suffix, mapping['port'])
    68 
     76            s = socket.socket()
     77            port = mapping['port']
    6978            try:
    70                 response = urllib.urlopen(url, proxies={})
    71                 _log.debug("Server running at %s" % url)
     79                s.connect(('localhost', port))
     80                _log.debug("Server running on %d" % port)
    7281            except IOError, e:
    73                 _log.debug("Server NOT running at %s: %s" % (url, e))
     82                if e.errno not in (errno.ECONNREFUSED, errno.ECONNRESET):
     83                    raise
     84                _log.debug("Server NOT running on %d: %s" % (port, e))
    7485                return False
    75 
     86            finally:
     87                s.close()
    7688        return True
    7789
     90    def check_that_all_ports_are_available(self):
     91        for mapping in self.mappings:
     92            s = socket.socket()
     93            port = mapping['port']
     94            try:
     95                s.bind(('localhost', port))
     96            except IOError, e:
     97                if e.errno in (errno.EALREADY, errno.EADDRINUSE):
     98                    raise ServerError('Port %d is already in use.' % port)
     99                else:
     100                    raise ServerError('Unexpected error: %s.' % str(e))
     101            finally:
     102                s.close()
     103
    78104    def remove_log_files(self, folder, starts_with):
    79         files = os.listdir(folder)
     105        files = self._filesystem.listdir(folder)
    80106        for file in files:
    81107            if file.startswith(starts_with):
    82                 full_path = os.path.join(folder, file)
    83                 filesystem.FileSystem().remove(full_path)
     108                full_path = self._filesystem.join(folder, file)
     109                self._filesystem.remove(full_path)

  • trunk/Tools/Scripts/webkitpy/layout_tests/port/websocket_server.py

    r85080 r85659  
    4141import tempfile
    4242import time
    43 import urllib
    44 
    45 import factory
     43
    4644import http_server
     45import http_server_base
    4746
    4847from webkitpy.common.system.executive import Executive
    … …  
    5655_DEFAULT_WS_PORT = 8880
    5756_DEFAULT_WSS_PORT = 9323
    58 
    59 
    60 def url_is_alive(url):
    61     """Checks to see if we get an http response from |url|.
    62     We poll the url 20 times with a 0.5 second delay.  If we don't
    63     get a reply in that time, we give up and assume the httpd
    64     didn't start properly.
    65 
    66     Args:
    67       url: The URL to check.
    68     Return:
    69       True if the url is alive.
    70     """
    71     sleep_time = 0.5
    72     wait_time = 10
    73     while wait_time > 0:
    74         try:
    75             response = urllib.urlopen(url, proxies={})
    76             # Server is up and responding.
    77             return True
    78         except IOError:
    79             pass
    80         # Wait for sleep_time before trying again.
    81         wait_time -= sleep_time
    82         time.sleep(sleep_time)
    83 
    84     return False
    85 
    86 
    87 class PyWebSocketNotStarted(Exception):
    88     pass
    89 
    90 
    91 class PyWebSocketNotFound(Exception):
    92     pass
    9357
    9458
    … …  
    11983        self._pidfile = pidfile
    12084        self._wsout = None
     85        self.mappings = []
    12186
    12287        # Webkit tests
    … …  
    139104            return
    140105        if self.is_running():
    141             raise PyWebSocketNotStarted('%s is already running.' %
    142                                         self._server_name)
     106            raise http_server_base.ServerError('%s is already running.' % self._server_name)
    143107
    144108        time_str = time.strftime('%d%b%Y-%H%M%S')
    … …  
    150114
    151115        # Remove old log files. We only need to keep the last ones.
    152         self.remove_log_files(self._output_dir, log_prefix)
     116        # Don't worry too much if this fails.
     117        try:
     118            self.remove_log_files(self._output_dir, log_prefix)
     119        except OSError, e:
     120            _log.warning('Failed to remove old websocket server log files')
     121            pass
    153122
    154123        error_log = os.path.join(self._output_dir, log_file_name + "-err.txt")
    … …  
    192161                             env.get('PYTHONPATH', ''))
    193162
     163        self.mappings = [{'port': self._port}]
     164        self.check_that_all_ports_are_available()
     165
    194166        _log.debug('Starting %s server on %d.' % (
    195167                   self._server_name, self._port))
    … …  
    203175                                         env=env)
    204176
    205         if self._use_tls:
    206             url = 'https'
    207         else:
    208             url = 'http'
    209         url = url + '://127.0.0.1:%d/' % self._port
    210         if not url_is_alive(url):
    211             if self._process.returncode == None:
    212                 # FIXME: We should use a non-static Executive for easier
    213                 # testing.
    214                 Executive().kill_process(self._process.pid)
    215             with codecs.open(output_log, "r", "utf-8") as fp:
    216                 for line in fp:
    217                     _log.error(line)
    218             raise PyWebSocketNotStarted(
    219                 'Failed to start %s server on port %s.' %
    220                     (self._server_name, self._port))
    221 
    222         # Our process terminated already
    223         if self._process.returncode != None:
    224             raise PyWebSocketNotStarted(
    225                 'Failed to start %s server.' % self._server_name)
     177        server_started = self.wait_for_action(self.is_server_running_on_all_ports)
     178        if not server_started or self._process.returncode != None:
     179            raise http_server_base.ServerError('Failed to start websocket server on port %d.' % self._port)
    226180        if self._pidfile:
    227181            with codecs.open(self._pidfile, "w", "ascii") as file:
    … …  
    240194
    241195        if not pid:
    242             raise PyWebSocketNotFound(
    243                 'Failed to find %s server pid.' % self._server_name)
     196            raise http_server_base.ServerError('Failed to find %s server pid.' % self._server_name)
    244197
    245198        _log.debug('Shutting down %s server %d.' % (self._server_name, pid))
    246         # FIXME: We should use a non-static Executive for easier testing.
    247         Executive().kill_process(pid)
     199        self._port_obj._executive.kill_process(pid)
    248200
    249201        if self._process:
Note: See TracChangeset for help on using the changeset viewer.