Tools/ChangeLog

112012-03-19 Dirk Pranke <dpranke@chromium.org>
22
 3 webkitpy: get ServerProcess out of the reportcrash business
 4 https://bugs.webkit.org/show_bug.cgi?id=81600
 5
 6 Reviewed by NOBODY (OOPS!).
 7
 8 ServerProcess was half-aware that ReportCrash might run
 9 sometimes, and that the process ServerProcess was talking to
 10 might have its own crashing subprocesses; neither of these
 11 things really worked right and it made the logic convoluted, so
 12 this change makes handling crashes completely separate from the
 13 server_process code, so that it can focus on just I/O to the
 14 subprocess.
 15
 16 There should be no functional changes resulting from this patch.
 17
 18 * Scripts/webkitpy/layout_tests/port/server_process.py:
 19 (ServerProcess._reset):
 20 (ServerProcess._handle_possible_interrupt):
 21 (ServerProcess.write):
 22 (ServerProcess.read_stdout):
 23 (ServerProcess.has_crashed):
 24 (ServerProcess._read):
 25 (ServerProcess.stop):
 26 * Scripts/webkitpy/layout_tests/port/server_process_unittest.py:
 27 (TrivialMockPort.check_for_leaks):
 28 (TestServerProcess.test_broken_pipe):
 29 * Scripts/webkitpy/layout_tests/port/webkit.py:
 30 (WebKitPort._read_image_diff):
 31 (WebKitDriver.has_crashed):
 32 (WebKitDriver._check_for_driver_crash):
 33 (WebKitDriver.run_test):
 34 (WebKitDriver._read_block):
 35 * Scripts/webkitpy/layout_tests/port/webkit_unittest.py:
 36 (MockServerProcess.__init__):
 37 (MockServerProcess):
 38 (MockServerProcess.has_crashed):
 39
 402012-03-19 Dirk Pranke <dpranke@chromium.org>
 41
342 NRWT runs some tests that are skipped with -i command line option
443 https://bugs.webkit.org/show_bug.cgi?id=81535
544

Tools/Scripts/webkitpy/layout_tests/port/server_process.py

@@class ServerProcess:
7070 self._proc = None
7171 self._output = str() # bytesarray() once we require Python 2.6
7272 self._error = str() # bytesarray() once we require Python 2.6
73  self.set_crashed(False)
 73 self._crashed = False
7474 self.timed_out = False
7575
7676 def process_name(self):

@@class ServerProcess:
9494 fl = fcntl.fcntl(fd, fcntl.F_GETFL)
9595 fcntl.fcntl(fd, fcntl.F_SETFL, fl | os.O_NONBLOCK)
9696
97  def handle_interrupt(self):
 97 def _handle_possible_interrupt(self):
9898 """This routine checks to see if the process crashed or exited
9999 because of a keyboard interrupt and raises KeyboardInterrupt
100100 accordingly."""
101  if self.crashed:
102  # This is hex code 0xc000001d, which is used for abrupt
103  # termination. This happens if we hit ctrl+c from the prompt
104  # and we happen to be waiting on the DumpRenderTree.
105  # sdoyon: Not sure for which OS and in what circumstances the
106  # above code is valid. What works for me under Linux to detect
107  # ctrl+c is for the subprocess returncode to be negative
108  # SIGINT. And that agrees with the subprocess documentation.
109  if (-1073741510 == self._proc.returncode or
110  - signal.SIGINT == self._proc.returncode):
111  raise KeyboardInterrupt
112  return
 101 # FIXME: Linux and Mac set the returncode to -signal.SIGINT if a
 102 # subprocess is killed with a ctrl^C. Previous comments in this
 103 # routine said that supposedly Windows returns 0xc000001d, but that's not what
 104 # -1073741510 evaluates to. Figure out what the right value is
 105 # for win32 and cygwin here ...
 106 if self._proc.returncode in (-1073741510, -signal.SIGINT):
 107 raise KeyboardInterrupt
113108
114109 def poll(self):
115110 """Check to see if the underlying process is running; returns None

@@class ServerProcess:
128123 except IOError, e:
129124 self.stop()
130125 # stop() calls _reset(), so we have to set crashed to True after calling stop().
131  self.set_crashed(True)
 126 self._crashed = True
132127
133128 def _pop_stdout_line_if_ready(self):
134129 index_after_newline = self._output.find('\n') + 1

@@class ServerProcess:
178173
179174 return self._read(deadline, retrieve_bytes_from_stdout_buffer)
180175
181  def _check_for_crash(self, wait_for_crash_reporter=True):
182  if self.poll() != None:
183  self.set_crashed(True, wait_for_crash_reporter)
184  self.handle_interrupt()
185 
186176 def _log(self, message):
187177 # This is a bit of a hack, but we first log a blank line to avoid
188178 # messing up the master process's output.

@@class ServerProcess:
206196 self._log('Unable to sample process.')
207197
208198 def _handle_timeout(self):
209  self._executive.wait_newest(self._port.is_crash_reporter)
210  self._check_for_crash(wait_for_crash_reporter=False)
211  if self.crashed:
212  return
213199 self.timed_out = True
214200 self._sample()
215201

@@class ServerProcess:
238224 # FIXME: Why do we ignore all IOErrors here?
239225 pass
240226
241  def _check_for_abort(self, deadline):
242  self._check_for_crash()
243 
244  if time.time() > deadline:
245  self._handle_timeout()
246 
247  return self.crashed or self.timed_out
 227 def has_crashed(self):
 228 if not self._crashed and self.poll():
 229 self._crashed = True
 230 self._handle_possible_interrupt()
 231 return self._crashed
248232
249233 # This read function is a bit oddly-designed, as it polls both stdout and stderr, yet
250234 # only reads/returns from one of them (buffering both in local self._output/self._error).
251235 # It might be cleaner to pass in the file descriptor to poll instead.
252236 def _read(self, deadline, fetch_bytes_from_buffers_callback):
253237 while True:
254  if self._check_for_abort(deadline):
 238 if self.has_crashed():
 239 return None
 240
 241 if time.time() > deadline:
 242 self._handle_timeout()
255243 return None
256244
257245 bytes = fetch_bytes_from_buffers_callback()

@@class ServerProcess:
292280 self._executive.kill_process(self._proc.pid)
293281 _log.warning('killed')
294282 self._reset()
295 
296  def set_crashed(self, crashed, wait_for_crash_reporter=True):
297  self.crashed = crashed
298  if not self.crashed or not wait_for_crash_reporter:
299  return
300  self._executive.wait_newest(self._port.is_crash_reporter)

Tools/Scripts/webkitpy/layout_tests/port/server_process_unittest.py

@@class TrivialMockPort(object):
5050 def check_for_leaks(self, process_name, process_pid):
5151 pass
5252
53  def is_crash_reporter(self, process_name):
54  return False
55 
5653
5754class MockFile(object):
5855 def __init__(self, server_process):

@@class TestServerProcess(unittest.TestCase):
9188 def test_broken_pipe(self):
9289 server_process = FakeServerProcess(port_obj=TrivialMockPort(), name="test", cmd=["test"])
9390 server_process.write("should break")
94  self.assertTrue(server_process.crashed)
 91 self.assertTrue(server_process.has_crashed())
9592 self.assertEquals(server_process._proc, None)
9693 self.assertEquals(server_process.broken_pipes, [server_process.stdin])
9794

Tools/Scripts/webkitpy/layout_tests/port/webkit.py

@@class WebKitPort(Port):
194194
195195 while True:
196196 output = sp.read_stdout_line(deadline)
197  if sp.timed_out or sp.crashed or not output:
 197 if sp.timed_out or sp.has_crashed() or not output:
198198 break
199199
200200 if output.startswith('diff'): # This is the last line ImageDiff prints.

@@class WebKitPort(Port):
209209
210210 if sp.timed_out:
211211 _log.error("ImageDiff timed out")
212  if sp.crashed:
 212 if sp.has_crashed():
213213 _log.error("ImageDiff crashed")
214214 # FIXME: There is no need to shut down the ImageDiff server after every diff.
215215 sp.stop()

@@class WebKitDriver(Driver):
489489 def has_crashed(self):
490490 if self._server_process is None:
491491 return False
492  return self._server_process.poll() is not None
 492 if self._crashed_subprocess_name:
 493 return True
 494 return self._server_process.has_crashed()
493495
494496 def _check_for_driver_crash(self, error_line):
495497 if error_line == "#CRASHED\n":
496498 # This is used on Windows to report that the process has crashed
497499 # See http://trac.webkit.org/changeset/65537.
498  self._server_process.set_crashed(True)
 500 self._crashed_subprocess_name = self._port.driver_name()
499501 elif error_line == "#CRASHED - WebProcess\n":
500502 # WebKitTestRunner uses this to report that the WebProcess subprocess crashed.
501  self._subprocess_crashed("WebProcess")
502  return self._detected_crash()
503 
504  def _detected_crash(self):
505  # We can't just check self._server_process.crashed because WebKitTestRunner
506  # can report subprocess crashes at any time by printing
507  # "#CRASHED - WebProcess", we want to count those as crashes as well.
508  return self._server_process.crashed or self._crashed_subprocess_name
509 
510  def _subprocess_crashed(self, subprocess_name):
511  self._crashed_subprocess_name = subprocess_name
512 
513  def _crashed_process_name(self):
514  if not self._detected_crash():
515  return None
516  return self._crashed_subprocess_name or self._server_process.process_name()
 503 self._crashed_subprocess_name = "WebProcess"
 504 return self._has_crashed()
517505
518506 def _command_from_driver_input(self, driver_input):
519507 if self.is_http_test(driver_input.test_name):

@@class WebKitDriver(Driver):
563551 self.error_from_test += self._server_process.pop_all_buffered_stderr()
564552
565553 return DriverOutput(text, image, actual_image_hash, audio,
566  crash=self._detected_crash(), test_time=time.time() - start_time,
 554 crash=self.has_crashed(), test_time=time.time() - start_time,
567555 timeout=self._server_process.timed_out, error=self.error_from_test,
568  crashed_process_name=self._crashed_process_name())
 556 crashed_process_name=self._crashed_subprocess_name)
569557
570558 def _read_header(self, block, line, header_text, header_attr, header_filter=None):
571559 if line.startswith(header_text) and getattr(block, header_attr) is None:

@@class WebKitDriver(Driver):
608596 else:
609597 out_line, err_line = self._server_process.read_either_stdout_or_stderr_line(deadline)
610598
611  if self._server_process.timed_out or self._detected_crash():
 599 if self._server_process.timed_out or self.has_crashed():
612600 break
613601
614602 if out_line:

Tools/Scripts/webkitpy/layout_tests/port/webkit_unittest.py

@@class WebKitPortTest(port_testcase.PortTestCase):
240240class MockServerProcess(object):
241241 def __init__(self, lines=None):
242242 self.timed_out = False
243  self.crashed = False
244243 self.lines = lines or []
 244 self.crashed = False
 245
 246 def has_crashed(self):
 247 return self.crashed
245248
246249 def read_stdout_line(self, deadline):
247250 return self.lines.pop(0) + "\n"