Changeset 58397 in webkit


Ignore:
Timestamp:
Apr 28, 2010 3:09:09 AM (14 years ago)
Author:
ukai@chromium.org
Message:

2010-04-28 Eric Seidel <​eric@webkit.org>

Reviewed by Jeremy Orlow.

webkitpy: ScriptError('Failed to run "[u\'taskkill.exe\', u\'/f\', u\'/im\', u\'httpd.exe\']" exit_code: 128',)
​https://bugs.webkit.org/show_bug.cgi?id=38248

The previous code did not check the return code of taskkill.
When I moved that callsite from using subprocess.call to
Executive.run_command having a non-zero return code became an error.

In this change I've centralized our killall handling in executive,
and added tests for it to make sure it works.

Currently kill_process and kill_all swallow exceptions in the cases
where the process(es) to be killed do(es) not exist.

  • Scripts/webkitpy/common/system/executive.py:
  • Scripts/webkitpy/common/system/executive_unittest.py:
  • Scripts/webkitpy/layout_tests/port/chromium_linux.py:
  • Scripts/webkitpy/layout_tests/port/chromium_mac.py:
  • Scripts/webkitpy/layout_tests/port/chromium_win.py:
  • Scripts/webkitpy/layout_tests/port/gtk.py:
  • Scripts/webkitpy/layout_tests/port/mac.py:
  • Scripts/webkitpy/layout_tests/port/qt.py:
  • Scripts/webkitpy/layout_tests/port/win.py:
Location:
trunk/WebKitTools
Files:
10 edited

Legend:

Unmodified
Added
Removed

  • trunk/WebKitTools/ChangeLog

    r58395 r58397  
     12010-04-28  Eric Seidel  <eric@webkit.org>
     2
     3        Reviewed by Jeremy Orlow.
     4
     5        webkitpy: ScriptError('Failed to run "[u\'taskkill.exe\', u\'/f\', u\'/im\', u\'httpd.exe\']" exit_code: 128',)
     6        https://bugs.webkit.org/show_bug.cgi?id=38248
     7
     8        The previous code did not check the return code of taskkill.
     9        When I moved that callsite from using subprocess.call to
     10        Executive.run_command having a non-zero return code became an error.
     11
     12        In this change I've centralized our killall handling in executive,
     13        and added tests for it to make sure it works.
     14
     15        Currently kill_process and kill_all swallow exceptions in the cases
     16        where the process(es) to be killed do(es) not exist.
     17
     18        * Scripts/webkitpy/common/system/executive.py:
     19        * Scripts/webkitpy/common/system/executive_unittest.py:
     20        * Scripts/webkitpy/layout_tests/port/chromium_linux.py:
     21        * Scripts/webkitpy/layout_tests/port/chromium_mac.py:
     22        * Scripts/webkitpy/layout_tests/port/chromium_win.py:
     23        * Scripts/webkitpy/layout_tests/port/gtk.py:
     24        * Scripts/webkitpy/layout_tests/port/mac.py:
     25        * Scripts/webkitpy/layout_tests/port/qt.py:
     26        * Scripts/webkitpy/layout_tests/port/win.py:
     27
    1282010-04-28  Eric Seidel  <eric@webkit.org>
    229

  • trunk/WebKitTools/Scripts/webkitpy/common/system/executive.py

    r58314 r58397  
    162162
    163163    def kill_process(self, pid):
     164        """Attempts to kill the given pid.
     165        Will fail silently if pid does not exist or insufficient permisssions."""
    164166        if platform.system() == "Windows":
    165167            # According to http://docs.python.org/library/os.html
    … …  
    167169            # using Cygwin, it worked fine.  We should investigate whether
    168170            # we need this platform specific code here.
    169             subprocess.call(('taskkill.exe', '/f', '/pid', unicode(pid)),
    170                             stdin=open(os.devnull, 'r'),
    171                             stdout=subprocess.PIPE,
    172                             stderr=subprocess.PIPE)
     171            command = ["taskkill.exe", "/f", "/pid", str(pid)]
     172            # taskkill will exit 128 if the process is not found.
     173            self.run_command(command, error_handler=self.ignore_error)
    173174            return
    174         os.kill(pid, signal.SIGKILL)
     175        try:
     176            os.kill(pid, signal.SIGKILL)
     177        except OSError, e:
     178            # FIXME: We should make non-silent failure an option.
     179            pass
     180
     181    def kill_all(self, process_name):
     182        """Attempts to kill processes matching process_name.
     183        Will fail silently if no process are found."""
     184        if platform.system() == "Windows":
     185            # We might want to automatically append .exe?
     186            command = ["taskkill.exe", "/f", "/im", process_name]
     187            # taskkill will exit 128 if the process is not found.
     188            self.run_command(command, error_handler=self.ignore_error)
     189            return
     190
     191        # FIXME: This is inconsistent that kill_all uses TERM and kill_process
     192        # uses KILL.  Windows is always using /f (which seems like -KILL).
     193        # We should pick one mode, or add support for switching between them.
     194        # Note: Mac OS X 10.6 requires -SIGNALNAME before -u USER
     195        command = ["killall", "-TERM", "-u", os.getenv("USER"), process_name]
     196        self.run_command(command, error_handler=self.ignore_error)
    175197
    176198    # Error handlers do not need to be static methods once all callers are

  • trunk/WebKitTools/Scripts/webkitpy/common/system/executive_unittest.py

    r58210 r58397  
    2828# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
    2929
     30import signal
     31import subprocess
     32import sys
    3033import unittest
    3134
    … …  
    6770        output = executive.run_and_throw_if_fail(["echo", "-n", unicode_tor], quiet=True, decode_output=False)
    6871        self.assertEquals(output, utf8_tor)
     72
     73    def test_kill_process(self):
     74        executive = Executive()
     75        # FIXME: This may need edits to work right on windows.
     76        # We use "yes" because it loops forever.
     77        process = subprocess.Popen(["yes"], stdout=subprocess.PIPE)
     78        self.assertEqual(process.poll(), None)  # Process is running
     79        executive.kill_process(process.pid)
     80        self.assertEqual(process.wait(), -signal.SIGKILL)
     81        # Killing again should fail silently.
     82        executive.kill_process(process.pid)
     83
     84    def test_kill_all(self):
     85        executive = Executive()
     86        # FIXME: This may need edits to work right on windows.
     87        # We use "yes" because it loops forever.
     88        process = subprocess.Popen(["yes"], stdout=subprocess.PIPE)
     89        self.assertEqual(process.poll(), None)  # Process is running
     90        executive.kill_all("yes")
     91        self.assertEqual(process.wait(), -signal.SIGTERM)
     92        # Killing again should fail silently.
     93        executive.kill_all("yes")

  • trunk/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_linux.py

    r58036 r58397  
    125125        return result
    126126
    127 
    128     def _kill_all_process(self, process_name):
    129         # FIXME: This should use Executive.
    130         null = open(os.devnull)
    131         subprocess.call(['killall', '-TERM', '-u', os.getenv('USER'),
    132                         process_name], stderr=null)
    133         null.close()
    134 
    135127    def _path_to_apache(self):
    136128        if self._is_redhat_based():
    … …  
    189181            # lighttpd processes not started by http_server.py,
    190182            # but good enough for now.
    191             self._kill_all_process('lighttpd')
    192             self._kill_all_process('apache2')
     183            self._executive.kill_all("lighttpd")
     184            self._executive.kill_all("apache2")
    193185        else:
    194186            try:

  • trunk/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_mac.py

    r58036 r58397  
    116116                                            'mac', *comps)
    117117
    118     def _kill_all_process(self, process_name):
    119         """Kill any processes running under this name."""
    120         # FIXME: This should use Executive.
    121         # On Mac OS X 10.6, killall has a new constraint: -SIGNALNAME or
    122         # -SIGNALNUMBER must come first.  Example problem:
    123         #   $ killall -u $USER -TERM lighttpd
    124         #   killall: illegal option -- T
    125         # Use of the earlier -TERM placement is just fine on 10.5.
    126         null = open(os.devnull)
    127         subprocess.call(['killall', '-TERM', '-u', os.getenv('USER'),
    128                         process_name], stderr=null)
    129         null.close()
    130 
    131118    def _path_to_apache(self):
    132119        return '/usr/sbin/httpd'
    … …  
    180167            # lighttpd processes not started by http_server.py,
    181168            # but good enough for now.
    182             self._kill_all_process('lighttpd')
    183             self._kill_all_process('httpd')
     169            self._executive.kill_all('lighttpd')
     170            self._executive.kill_all('httpd')
    184171        else:
    185172            try:

  • trunk/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_win.py

    r58314 r58397  
    145145                                            'wdiff.exe')
    146146
    147     def _kill_all(self, process_name):
    148         cmd = ['taskkill.exe', '/f', '/im', process_name]
    149         self._executive.run_command(cmd)
    150 
    151147    def _shut_down_http_server(self, server_pid):
    152148        """Shut down the lighttpd web server. Blocks until it's fully
    … …  
    158154        # FIXME: Why are we ignoring server_pid and calling
    159155        # _kill_all instead of Executive.kill_process(pid)?
    160         self._kill_all("LightTPD.exe")
    161         self._kill_all("httpd.exe")
     156        self._executive.kill_all("LightTPD.exe")
     157        self._executive.kill_all("httpd.exe")

  • trunk/WebKitTools/Scripts/webkitpy/layout_tests/port/gtk.py

    r58036 r58397  
    6262                            'apache2-debian-httpd.conf')
    6363
    64     def _kill_all_process(self, process_name):
    65         # FIXME: This should use Executive.
    66         null = open(os.devnull)
    67         subprocess.call(['killall', '-TERM', '-u', os.getenv('USER'),
    68                         process_name], stderr=null)
    69         null.close()
    70 
    7164    def _shut_down_http_server(self, server_pid):
    7265        """Shut down the httpd web server. Blocks until it's fully
    … …  
    8174            # lighttpd processes not started by http_server.py,
    8275            # but good enough for now.
    83             self._kill_all_process('apache2')
     76            self._executive.kill_all('apache2')
    8477        else:
    8578            try:

  • trunk/WebKitTools/Scripts/webkitpy/layout_tests/port/mac.py

    r58036 r58397  
    132132        ]
    133133
    134     # FIXME: This doesn't have anything to do with WebKit.
    135     def _kill_all_process(self, process_name):
    136         # FIXME: This should use Executive.
    137         # On Mac OS X 10.6, killall has a new constraint: -SIGNALNAME or
    138         # -SIGNALNUMBER must come first.  Example problem:
    139         #   $ killall -u $USER -TERM lighttpd
    140         #   killall: illegal option -- T
    141         # Use of the earlier -TERM placement is just fine on 10.5.
    142         null = open(os.devnull)
    143         subprocess.call(['killall', '-TERM', '-u', os.getenv('USER'),
    144                         process_name], stderr=null)
    145         null.close()
    146 
    147134    def _path_to_apache_config_file(self):
    148135        return os.path.join(self.layout_tests_dir(), 'http', 'conf',
    … …  
    162149            # lighttpd processes not started by http_server.py,
    163150            # but good enough for now.
    164             self._kill_all_process('httpd')
     151            self._executive.kill_all('httpd')
    165152        else:
    166153            try:

  • trunk/WebKitTools/Scripts/webkitpy/layout_tests/port/qt.py

    r58036 r58397  
    6363                            'apache2-debian-httpd.conf')
    6464
    65     def _kill_all_process(self, process_name):
    66         # FIXME: This should use Executive.
    67         null = open(os.devnull)
    68         subprocess.call(['killall', '-TERM', '-u', os.getenv('USER'),
    69                         process_name], stderr=null)
    70         null.close()
    71 
    7265    def _shut_down_http_server(self, server_pid):
    7366        """Shut down the httpd web server. Blocks until it's fully
    … …  
    8275            # lighttpd processes not started by http_server.py,
    8376            # but good enough for now.
    84             self._kill_all_process('apache2')
     77            self._executive.kill_all('apache2')
    8578        else:
    8679            try:

  • trunk/WebKitTools/Scripts/webkitpy/layout_tests/port/win.py

    r57499 r58397  
    7070        # Looks like we ignore server_pid.
    7171        # Copy/pasted from chromium-win.
    72         subprocess.Popen(('taskkill.exe', '/f', '/im', 'httpd.exe'),
    73                         stdin=open(os.devnull, 'r'),
    74                         stdout=subprocess.PIPE,
    75                         stderr=subprocess.PIPE).wait()
     72        self._executive.kill_all("httpd.exe")
Note: See TracChangeset for help on using the changeset viewer.