Diff
Modified: trunk/Tools/ChangeLog (111598 => 111599)
--- trunk/Tools/ChangeLog 2012-03-21 20:24:11 UTC (rev 111598)
+++ trunk/Tools/ChangeLog 2012-03-21 20:29:02 UTC (rev 111599)
@@ -13,6 +13,40 @@
(addFontDirectory):
(addFontsToEnvironment):
+2012-03-21 Dirk Pranke <[email protected]>
+
+ webkitpy: get ServerProcess out of the reportcrash business
+ https://bugs.webkit.org/show_bug.cgi?id=81600
+
+ Unreviewed, build fix.
+
+ Re-land r111307 and r111293 with another fix for a crash in NRWT;
+ we need to check if the driver has crashed before attempting to
+ read from it.
+
+
+ * Scripts/webkitpy/layout_tests/port/server_process.py:
+ (ServerProcess._reset):
+ (ServerProcess._handle_possible_interrupt):
+ (ServerProcess.write):
+ (ServerProcess.read_stdout):
+ (ServerProcess.has_crashed):
+ (ServerProcess._read):
+ (ServerProcess.stop):
+ * Scripts/webkitpy/layout_tests/port/server_process_unittest.py:
+ (TrivialMockPort.check_for_leaks):
+ (TestServerProcess.test_broken_pipe):
+ * Scripts/webkitpy/layout_tests/port/webkit.py:
+ (WebKitPort._read_image_diff):
+ (WebKitDriver.has_crashed):
+ (WebKitDriver._check_for_driver_crash):
+ (WebKitDriver.run_test):
+ (WebKitDriver._read_block):
+ * Scripts/webkitpy/layout_tests/port/webkit_unittest.py:
+ (MockServerProcess.__init__):
+ (MockServerProcess):
+ (MockServerProcess.has_crashed):
+
2012-03-21 Zeno Albisser <[email protected]>
[Qt][Mac] ranlib segfaults when creating symbol tables for libWebCore.a.
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/server_process.py (111598 => 111599)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/server_process.py 2012-03-21 20:24:11 UTC (rev 111598)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/server_process.py 2012-03-21 20:29:02 UTC (rev 111599)
@@ -70,7 +70,7 @@
self._proc = None
self._output = str() # bytesarray() once we require Python 2.6
self._error = str() # bytesarray() once we require Python 2.6
- self.set_crashed(False)
+ self._crashed = False
self.timed_out = False
def process_name(self):
@@ -94,22 +94,17 @@
fl = fcntl.fcntl(fd, fcntl.F_GETFL)
fcntl.fcntl(fd, fcntl.F_SETFL, fl | os.O_NONBLOCK)
- def handle_interrupt(self):
+ def _handle_possible_interrupt(self):
"""This routine checks to see if the process crashed or exited
because of a keyboard interrupt and raises KeyboardInterrupt
accordingly."""
- if self.crashed:
- # This is hex code 0xc000001d, which is used for abrupt
- # termination. This happens if we hit ctrl+c from the prompt
- # and we happen to be waiting on the DumpRenderTree.
- # sdoyon: Not sure for which OS and in what circumstances the
- # above code is valid. What works for me under Linux to detect
- # ctrl+c is for the subprocess returncode to be negative
- # SIGINT. And that agrees with the subprocess documentation.
- if (-1073741510 == self._proc.returncode or
- - signal.SIGINT == self._proc.returncode):
- raise KeyboardInterrupt
- return
+ # FIXME: Linux and Mac set the returncode to -signal.SIGINT if a
+ # subprocess is killed with a ctrl^C. Previous comments in this
+ # routine said that supposedly Windows returns 0xc000001d, but that's not what
+ # -1073741510 evaluates to. Figure out what the right value is
+ # for win32 and cygwin here ...
+ if self._proc.returncode in (-1073741510, -signal.SIGINT):
+ raise KeyboardInterrupt
def poll(self):
"""Check to see if the underlying process is running; returns None
@@ -128,7 +123,7 @@
except IOError, e:
self.stop()
# stop() calls _reset(), so we have to set crashed to True after calling stop().
- self.set_crashed(True)
+ self._crashed = True
def _pop_stdout_line_if_ready(self):
index_after_newline = self._output.find('\n') + 1
@@ -178,11 +173,6 @@
return self._read(deadline, retrieve_bytes_from_stdout_buffer)
- def _check_for_crash(self, wait_for_crash_reporter=True):
- if self.poll() != None:
- self.set_crashed(True, wait_for_crash_reporter)
- self.handle_interrupt()
-
def _log(self, message):
# This is a bit of a hack, but we first log a blank line to avoid
# messing up the master process's output.
@@ -206,10 +196,6 @@
self._log('Unable to sample process.')
def _handle_timeout(self):
- self._executive.wait_newest(self._port.is_crash_reporter)
- self._check_for_crash(wait_for_crash_reporter=False)
- if self.crashed:
- return
self.timed_out = True
self._sample()
@@ -238,22 +224,24 @@
# FIXME: Why do we ignore all IOErrors here?
pass
- def _check_for_abort(self, deadline):
- self._check_for_crash()
+ def has_crashed(self):
+ if not self._crashed and self.poll():
+ self._crashed = True
+ self._handle_possible_interrupt()
+ return self._crashed
- if time.time() > deadline:
- self._handle_timeout()
-
- return self.crashed or self.timed_out
-
# This read function is a bit oddly-designed, as it polls both stdout and stderr, yet
# only reads/returns from one of them (buffering both in local self._output/self._error).
# It might be cleaner to pass in the file descriptor to poll instead.
def _read(self, deadline, fetch_bytes_from_buffers_callback):
while True:
- if self._check_for_abort(deadline):
+ if self.has_crashed():
return None
+ if time.time() > deadline:
+ self._handle_timeout()
+ return None
+
bytes = fetch_bytes_from_buffers_callback()
if bytes is not None:
return bytes
@@ -292,9 +280,3 @@
self._executive.kill_process(self._proc.pid)
_log.warning('killed')
self._reset()
-
- def set_crashed(self, crashed, wait_for_crash_reporter=True):
- self.crashed = crashed
- if not self.crashed or not wait_for_crash_reporter:
- return
- self._executive.wait_newest(self._port.is_crash_reporter)
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/server_process_unittest.py (111598 => 111599)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/server_process_unittest.py 2012-03-21 20:24:11 UTC (rev 111598)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/server_process_unittest.py 2012-03-21 20:29:02 UTC (rev 111599)
@@ -50,10 +50,7 @@
def check_for_leaks(self, process_name, process_pid):
pass
- def is_crash_reporter(self, process_name):
- return False
-
class MockFile(object):
def __init__(self, server_process):
self._server_process = server_process
@@ -91,7 +88,7 @@
def test_broken_pipe(self):
server_process = FakeServerProcess(port_obj=TrivialMockPort(), name="test", cmd=["test"])
server_process.write("should break")
- self.assertTrue(server_process.crashed)
+ self.assertTrue(server_process.has_crashed())
self.assertEquals(server_process._proc, None)
self.assertEquals(server_process.broken_pipes, [server_process.stdin])
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit.py (111598 => 111599)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit.py 2012-03-21 20:24:11 UTC (rev 111598)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit.py 2012-03-21 20:29:02 UTC (rev 111599)
@@ -194,7 +194,7 @@
while True:
output = sp.read_stdout_line(deadline)
- if sp.timed_out or sp.crashed or not output:
+ if sp.timed_out or sp.has_crashed() or not output:
break
if output.startswith('diff'): # This is the last line ImageDiff prints.
@@ -209,7 +209,7 @@
if sp.timed_out:
_log.error("ImageDiff timed out")
- if sp.crashed:
+ if sp.has_crashed():
_log.error("ImageDiff crashed")
# FIXME: There is no need to shut down the ImageDiff server after every diff.
sp.stop()
@@ -489,32 +489,25 @@
def has_crashed(self):
if self._server_process is None:
return False
- return self._server_process.poll() is not None
+ if self._crashed_subprocess_name:
+ return True
+ if self._server_process.has_crashed():
+ self._crashed_subprocess_name = self._port.driver_name()
+ return True
+ return False
def _check_for_driver_crash(self, error_line):
if error_line == "#CRASHED\n":
# This is used on Windows to report that the process has crashed
# See http://trac.webkit.org/changeset/65537.
- self._server_process.set_crashed(True)
- elif error_line == "#CRASHED - WebProcess\n":
+ self._crashed_subprocess_name = self._port.driver_name()
+ return True
+ if error_line == "#CRASHED - WebProcess\n":
# WebKitTestRunner uses this to report that the WebProcess subprocess crashed.
- self._subprocess_crashed("WebProcess")
- return self._detected_crash()
+ self._crashed_subprocess_name = "WebProcess"
+ return True
+ return self.has_crashed()
- def _detected_crash(self):
- # We can't just check self._server_process.crashed because WebKitTestRunner
- # can report subprocess crashes at any time by printing
- # "#CRASHED - WebProcess", we want to count those as crashes as well.
- return self._server_process.crashed or self._crashed_subprocess_name
-
- def _subprocess_crashed(self, subprocess_name):
- self._crashed_subprocess_name = subprocess_name
-
- def _crashed_process_name(self):
- if not self._detected_crash():
- return None
- return self._crashed_subprocess_name or self._server_process.process_name()
-
def _command_from_driver_input(self, driver_input):
if self.is_http_test(driver_input.test_name):
command = self.test_to_uri(driver_input.test_name)
@@ -563,9 +556,9 @@
self.error_from_test += self._server_process.pop_all_buffered_stderr()
return DriverOutput(text, image, actual_image_hash, audio,
- crash=self._detected_crash(), test_time=time.time() - start_time,
+ crash=self.has_crashed(), test_time=time.time() - start_time,
timeout=self._server_process.timed_out, error=self.error_from_test,
- crashed_process_name=self._crashed_process_name())
+ crashed_process_name=self._crashed_subprocess_name)
def _read_header(self, block, line, header_text, header_attr, header_filter=None):
if line.startswith(header_text) and getattr(block, header_attr) is None:
@@ -595,7 +588,7 @@
block = ContentBlock()
out_seen_eof = False
- while True:
+ while not self.has_crashed():
if out_seen_eof and (self.err_seen_eof or not wait_for_stderr_eof):
break
@@ -608,7 +601,7 @@
else:
out_line, err_line = self._server_process.read_either_stdout_or_stderr_line(deadline)
- if self._server_process.timed_out or self._detected_crash():
+ if self._server_process.timed_out or self.has_crashed():
break
if out_line:
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit_unittest.py (111598 => 111599)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit_unittest.py 2012-03-21 20:24:11 UTC (rev 111598)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit_unittest.py 2012-03-21 20:29:02 UTC (rev 111599)
@@ -240,9 +240,12 @@
class MockServerProcess(object):
def __init__(self, lines=None):
self.timed_out = False
+ self.lines = lines or []
self.crashed = False
- self.lines = lines or []
+ def has_crashed(self):
+ return self.crashed
+
def read_stdout_line(self, deadline):
return self.lines.pop(0) + "\n"
