Commit 648b0984 authored by ukai@chromium.org's avatar ukai@chromium.org
Browse files

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:

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@58397 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 780ddc97
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:
2010-04-28 Eric Seidel <eric@webkit.org>
 
Reviewed by Shinichiro Hamaji.
......@@ -161,17 +161,39 @@ class Executive(object):
return 2
def kill_process(self, pid):
"""Attempts to kill the given pid.
Will fail silently if pid does not exist or insufficient permisssions."""
if platform.system() == "Windows":
# According to http://docs.python.org/library/os.html
# os.kill isn't available on Windows. However, when I tried it
# using Cygwin, it worked fine. We should investigate whether
# we need this platform specific code here.
subprocess.call(('taskkill.exe', '/f', '/pid', unicode(pid)),
stdin=open(os.devnull, 'r'),
stdout=subprocess.PIPE,
stderr=subprocess.PIPE)
command = ["taskkill.exe", "/f", "/pid", str(pid)]
# taskkill will exit 128 if the process is not found.
self.run_command(command, error_handler=self.ignore_error)
return
os.kill(pid, signal.SIGKILL)
try:
os.kill(pid, signal.SIGKILL)
except OSError, e:
# FIXME: We should make non-silent failure an option.
pass
def kill_all(self, process_name):
"""Attempts to kill processes matching process_name.
Will fail silently if no process are found."""
if platform.system() == "Windows":
# We might want to automatically append .exe?
command = ["taskkill.exe", "/f", "/im", process_name]
# taskkill will exit 128 if the process is not found.
self.run_command(command, error_handler=self.ignore_error)
return
# FIXME: This is inconsistent that kill_all uses TERM and kill_process
# uses KILL. Windows is always using /f (which seems like -KILL).
# We should pick one mode, or add support for switching between them.
# Note: Mac OS X 10.6 requires -SIGNALNAME before -u USER
command = ["killall", "-TERM", "-u", os.getenv("USER"), process_name]
self.run_command(command, error_handler=self.ignore_error)
# Error handlers do not need to be static methods once all callers are
# updated to use an Executive object.
......
......@@ -27,6 +27,9 @@
# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
import signal
import subprocess
import sys
import unittest
from webkitpy.common.system.executive import Executive, run_command
......@@ -66,3 +69,25 @@ class ExecutiveTest(unittest.TestCase):
output = executive.run_and_throw_if_fail(["echo", "-n", unicode_tor], quiet=True, decode_output=False)
self.assertEquals(output, utf8_tor)
def test_kill_process(self):
executive = Executive()
# FIXME: This may need edits to work right on windows.
# We use "yes" because it loops forever.
process = subprocess.Popen(["yes"], stdout=subprocess.PIPE)
self.assertEqual(process.poll(), None) # Process is running
executive.kill_process(process.pid)
self.assertEqual(process.wait(), -signal.SIGKILL)
# Killing again should fail silently.
executive.kill_process(process.pid)
def test_kill_all(self):
executive = Executive()
# FIXME: This may need edits to work right on windows.
# We use "yes" because it loops forever.
process = subprocess.Popen(["yes"], stdout=subprocess.PIPE)
self.assertEqual(process.poll(), None) # Process is running
executive.kill_all("yes")
self.assertEqual(process.wait(), -signal.SIGTERM)
# Killing again should fail silently.
executive.kill_all("yes")
......@@ -124,14 +124,6 @@ class ChromiumLinuxPort(chromium.ChromiumPort):
_log.error('')
return result
def _kill_all_process(self, process_name):
# FIXME: This should use Executive.
null = open(os.devnull)
subprocess.call(['killall', '-TERM', '-u', os.getenv('USER'),
process_name], stderr=null)
null.close()
def _path_to_apache(self):
if self._is_redhat_based():
return '/usr/sbin/httpd'
......@@ -188,8 +180,8 @@ class ChromiumLinuxPort(chromium.ChromiumPort):
# TODO(mmoss) This isn't ideal, since it could conflict with
# lighttpd processes not started by http_server.py,
# but good enough for now.
self._kill_all_process('lighttpd')
self._kill_all_process('apache2')
self._executive.kill_all("lighttpd")
self._executive.kill_all("apache2")
else:
try:
os.kill(server_pid, signal.SIGTERM)
......
......@@ -115,19 +115,6 @@ class ChromiumMacPort(chromium.ChromiumPort):
return self.path_from_chromium_base('third_party', 'lighttpd',
'mac', *comps)
def _kill_all_process(self, process_name):
"""Kill any processes running under this name."""
# FIXME: This should use Executive.
# On Mac OS X 10.6, killall has a new constraint: -SIGNALNAME or
# -SIGNALNUMBER must come first. Example problem:
# $ killall -u $USER -TERM lighttpd
# killall: illegal option -- T
# Use of the earlier -TERM placement is just fine on 10.5.
null = open(os.devnull)
subprocess.call(['killall', '-TERM', '-u', os.getenv('USER'),
process_name], stderr=null)
null.close()
def _path_to_apache(self):
return '/usr/sbin/httpd'
......@@ -179,8 +166,8 @@ class ChromiumMacPort(chromium.ChromiumPort):
# TODO(mmoss) This isn't ideal, since it could conflict with
# lighttpd processes not started by http_server.py,
# but good enough for now.
self._kill_all_process('lighttpd')
self._kill_all_process('httpd')
self._executive.kill_all('lighttpd')
self._executive.kill_all('httpd')
else:
try:
os.kill(server_pid, signal.SIGTERM)
......
......@@ -144,10 +144,6 @@ class ChromiumWinPort(chromium.ChromiumPort):
return self.path_from_chromium_base('third_party', 'cygwin', 'bin',
'wdiff.exe')
def _kill_all(self, process_name):
cmd = ['taskkill.exe', '/f', '/im', process_name]
self._executive.run_command(cmd)
def _shut_down_http_server(self, server_pid):
"""Shut down the lighttpd web server. Blocks until it's fully
shut down.
......@@ -157,5 +153,5 @@ class ChromiumWinPort(chromium.ChromiumPort):
"""
# FIXME: Why are we ignoring server_pid and calling
# _kill_all instead of Executive.kill_process(pid)?
self._kill_all("LightTPD.exe")
self._kill_all("httpd.exe")
self._executive.kill_all("LightTPD.exe")
self._executive.kill_all("httpd.exe")
......@@ -61,13 +61,6 @@ class GtkPort(WebKitPort):
return os.path.join(self.layout_tests_dir(), 'http', 'conf',
'apache2-debian-httpd.conf')
def _kill_all_process(self, process_name):
# FIXME: This should use Executive.
null = open(os.devnull)
subprocess.call(['killall', '-TERM', '-u', os.getenv('USER'),
process_name], stderr=null)
null.close()
def _shut_down_http_server(self, server_pid):
"""Shut down the httpd web server. Blocks until it's fully
shut down.
......@@ -80,7 +73,7 @@ class GtkPort(WebKitPort):
# FIXME: This isn't ideal, since it could conflict with
# lighttpd processes not started by http_server.py,
# but good enough for now.
self._kill_all_process('apache2')
self._executive.kill_all('apache2')
else:
try:
os.kill(server_pid, signal.SIGTERM)
......
......@@ -131,19 +131,6 @@ class MacPort(WebKitPort):
"platform/win",
]
# FIXME: This doesn't have anything to do with WebKit.
def _kill_all_process(self, process_name):
# FIXME: This should use Executive.
# On Mac OS X 10.6, killall has a new constraint: -SIGNALNAME or
# -SIGNALNUMBER must come first. Example problem:
# $ killall -u $USER -TERM lighttpd
# killall: illegal option -- T
# Use of the earlier -TERM placement is just fine on 10.5.
null = open(os.devnull)
subprocess.call(['killall', '-TERM', '-u', os.getenv('USER'),
process_name], stderr=null)
null.close()
def _path_to_apache_config_file(self):
return os.path.join(self.layout_tests_dir(), 'http', 'conf',
'apache2-httpd.conf')
......@@ -161,7 +148,7 @@ class MacPort(WebKitPort):
# FIXME: This isn't ideal, since it could conflict with
# lighttpd processes not started by http_server.py,
# but good enough for now.
self._kill_all_process('httpd')
self._executive.kill_all('httpd')
else:
try:
os.kill(server_pid, signal.SIGTERM)
......
......@@ -62,13 +62,6 @@ class QtPort(WebKitPort):
return os.path.join(self.layout_tests_dir(), 'http', 'conf',
'apache2-debian-httpd.conf')
def _kill_all_process(self, process_name):
# FIXME: This should use Executive.
null = open(os.devnull)
subprocess.call(['killall', '-TERM', '-u', os.getenv('USER'),
process_name], stderr=null)
null.close()
def _shut_down_http_server(self, server_pid):
"""Shut down the httpd web server. Blocks until it's fully
shut down.
......@@ -81,7 +74,7 @@ class QtPort(WebKitPort):
# FIXME: This isn't ideal, since it could conflict with
# lighttpd processes not started by http_server.py,
# but good enough for now.
self._kill_all_process('apache2')
self._executive.kill_all('apache2')
else:
try:
os.kill(server_pid, signal.SIGTERM)
......
......@@ -69,7 +69,4 @@ class WinPort(WebKitPort):
"""
# Looks like we ignore server_pid.
# Copy/pasted from chromium-win.
subprocess.Popen(('taskkill.exe', '/f', '/im', 'httpd.exe'),
stdin=open(os.devnull, 'r'),
stdout=subprocess.PIPE,
stderr=subprocess.PIPE).wait()
self._executive.kill_all("httpd.exe")
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