|Summary:||webkitpy: ScriptError('Failed to run "[u\'taskkill.exe\', u\'/f\', u\'/im\', u\'httpd.exe\']" exit_code: 128',)|
|Product:||WebKit||Reporter:||Fumitoshi Ukai <ukai>|
|Component:||Tools / Tests||Assignee:||Nobody <webkit-unassigned>|
|Severity:||Normal||CC:||abarth, cjerdonek, dpranke, eric, jorlow, ojan|
|Version:||528+ (Nightly build)|
Description Fumitoshi Ukai 2010-04-28 01:11:01 PDT
Webkit (webkit.org) builder on chromium failed to kill httpd at the end of webkit tests, and left websocket server running. http://build.chromium.org/buildbot/waterfall.fyi/builders/Webkit%20(webkit.org)/builds/26489/steps/webkit_tests/logs/stdio 100428 00:54:31 run_webkit_tests.py:243 DEBUG flushing stdout 100428 00:54:31 run_webkit_tests.py:245 DEBUG flushing stderr 100428 00:54:31 run_webkit_tests.py:247 DEBUG stopping http server Exception webkitpy.common.system.executive.ScriptError: ScriptError('Failed to run "[u\'taskkill.exe\', u\'/f\', u\'/im\', u\'httpd.exe\']" exit_code: 128',) in <bound method TestRunner.__del__ of <webkitpy.layout_tests.run_webkit_tests.TestRunner instance at 0x00E21918>> ignored command timed out: 600 seconds without output, killing pid 18336 SIGKILL failed to kill process using fake rc=-1 program finished with exit code -1 remoteFailed: [Failure instance: Traceback from remote host -- Traceback (most recent call last): Failure: buildbot.slave.commands.TimeoutError: SIGKILL failed to kill process Next webkit tests run would fail to remove output files of websocket before launching websocket server, because output file is used in running process.
Comment 1 Eric Seidel (no email) 2010-04-28 01:14:01 PDT
Thank you for filing this! I'm working on a fix now.
Comment 2 Eric Seidel (no email) 2010-04-28 02:23:34 PDT
I'm not sure I understand. Should the httpd server be running? Are we failing to kill it? The old code would have ignored this error. So I'm fixing the new code to ignore it as well.
Comment 4 Eric Seidel (no email) 2010-04-28 02:34:33 PDT
I think the problem here was that killall "httpd" is never going to succeed on windows since we use lighttpd. :)
Comment 5 Fumitoshi Ukai 2010-04-28 02:40:54 PDT
(In reply to comment #3) > Created an attachment (id=54539) [details] > Patch LGTM. Thanks for fixing!
Comment 6 Eric Seidel (no email) 2010-04-28 02:44:25 PDT
The better fix would have probably been to stop trying to kill httpd when we know we didn't start it and then actually make the Executive.kill_all API throw an exception if no process exists. But we can do that sort of change later.
Comment 7 Fumitoshi Ukai 2010-04-28 02:54:53 PDT
Comment on attachment 54539 [details] Patch > Reviewed by Darin Adler and Eric Seidel. > diff --git a/WebKitTools/Scripts/webkitpy/common/system/executive.py b/WebKitTools/Scripts/webkitpy/common/system/executive.py > index 5c1d28ca6f5ae45cebaad330018bc3b6c3c6cb1e..cb29834f4c970cecd63c105a373b2626a6391002 100644 > --- a/WebKitTools/Scripts/webkitpy/common/system/executive.py > +++ b/WebKitTools/Scripts/webkitpy/common/system/executive.py > @@ -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", pid] str(pid) or unicode(pid) ? are we sure pid is already string?
Comment 8 Jeremy Orlow 2010-04-28 02:55:54 PDT
Comment on attachment 54539 [details] Patch Looks fine to me, Ukai did an unofficial review, and this is breaking the Chromium bots. So r=me
Comment 9 Jeremy Orlow 2010-04-28 02:56:50 PDT
Comment on attachment 54539 [details] Patch Wait...didn't see Ukai's comment.
Comment 10 Jeremy Orlow 2010-04-28 03:06:09 PDT
Comment on attachment 54539 [details] Patch r=me. Ukia, please fix while landing.
Comment 11 Fumitoshi Ukai 2010-04-28 03:09:25 PDT
Committed r58397: <http://trac.webkit.org/changeset/58397>
Comment 12 Eric Seidel (no email) 2010-04-28 11:49:36 PDT
command = ["taskkill.exe", "/f", "/pid", str(pid)] is wrong. run_command already knows how to convert arguments to strings. I'll remove the "fix" in a later checkin.