Title: [102162] trunk/Tools
- Revision
- 102162
- Author
- [email protected]
- Date
- 2011-12-06 12:04:08 -0800 (Tue, 06 Dec 2011)
Log Message
Wait for Crash Reporter to finish even when it lets the crashed process die quickly
NRWT was only waiting for Crash Reporter in cases where it was keeping the crashed process
alive beyond the normal timeout limit. In cases where the crashed process was able to die
faster, NRWT would assume that Crash Reporter had finished even though it often was still
running, which would lead to an incorrect crash log being picked up.
Part of <http://webkit.org/b/71380> NRWT incorrectly associates crash logs with tests
Reviewed by Dirk Pranke.
* Scripts/webkitpy/layout_tests/port/server_process.py:
(ServerProcess._reset):
(ServerProcess.write):
(ServerProcess._check_for_crash): Changed to use new set_crashed function instead of setting
.crashed directly. Added wait_for_crash_reporter parameter, which we pass along to
set_crashed.
(ServerProcess._handle_timeout): Fixed a logic error that would cause .crashed and
.timed_out both to be set to True in cases where Crash Reporter took a long time to run. Now
we bail out of handling the failure as a timeout if we find out that the process in fact
crashed. We tell _check_for_crash not to wait for Crash Reporter because we've already done
so.
(ServerProcess.set_crashed): Added. When the process crashes, we wait for Crash Reporter to
finish running (unless directed otherwise) so we can be sure the crash log has been saved to
disk.
* Scripts/webkitpy/layout_tests/port/server_process_unittest.py:
(TrivialMockPort.is_crash_reporter): Added.
* Scripts/webkitpy/layout_tests/port/webkit.py:
(WebKitDriver._check_for_driver_crash): Changed to use set_crashed. (This also fixed a typo
that would have partially broken crash detection on Windows, if NRWT worked on Windows.)
Modified Paths
Diff
Modified: trunk/Tools/ChangeLog (102161 => 102162)
--- trunk/Tools/ChangeLog 2011-12-06 20:00:03 UTC (rev 102161)
+++ trunk/Tools/ChangeLog 2011-12-06 20:04:08 UTC (rev 102162)
@@ -1,3 +1,39 @@
+2011-12-06 Adam Roben <[email protected]>
+
+ Wait for Crash Reporter to finish even when it lets the crashed process die quickly
+
+ NRWT was only waiting for Crash Reporter in cases where it was keeping the crashed process
+ alive beyond the normal timeout limit. In cases where the crashed process was able to die
+ faster, NRWT would assume that Crash Reporter had finished even though it often was still
+ running, which would lead to an incorrect crash log being picked up.
+
+ Part of <http://webkit.org/b/71380> NRWT incorrectly associates crash logs with tests
+
+ Reviewed by Dirk Pranke.
+
+ * Scripts/webkitpy/layout_tests/port/server_process.py:
+ (ServerProcess._reset):
+ (ServerProcess.write):
+ (ServerProcess._check_for_crash): Changed to use new set_crashed function instead of setting
+ .crashed directly. Added wait_for_crash_reporter parameter, which we pass along to
+ set_crashed.
+
+ (ServerProcess._handle_timeout): Fixed a logic error that would cause .crashed and
+ .timed_out both to be set to True in cases where Crash Reporter took a long time to run. Now
+ we bail out of handling the failure as a timeout if we find out that the process in fact
+ crashed. We tell _check_for_crash not to wait for Crash Reporter because we've already done
+ so.
+ (ServerProcess.set_crashed): Added. When the process crashes, we wait for Crash Reporter to
+ finish running (unless directed otherwise) so we can be sure the crash log has been saved to
+ disk.
+
+ * Scripts/webkitpy/layout_tests/port/server_process_unittest.py:
+ (TrivialMockPort.is_crash_reporter): Added.
+
+ * Scripts/webkitpy/layout_tests/port/webkit.py:
+ (WebKitDriver._check_for_driver_crash): Changed to use set_crashed. (This also fixed a typo
+ that would have partially broken crash detection on Windows, if NRWT worked on Windows.)
+
2011-12-06 Adam Barth <[email protected]>
NRWT fails on unreleased versions of Mac OS X
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/server_process.py (102161 => 102162)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/server_process.py 2011-12-06 20:00:03 UTC (rev 102161)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/server_process.py 2011-12-06 20:04:08 UTC (rev 102162)
@@ -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.crashed = False
+ self.set_crashed(False)
self.timed_out = False
def process_name(self):
@@ -128,7 +128,7 @@
except IOError, e:
self.stop()
# stop() calls _reset(), so we have to set crashed to True after calling stop().
- self.crashed = True
+ self.set_crashed(True)
def _pop_stdout_line_if_ready(self):
index_after_newline = self._output.find('\n') + 1
@@ -178,9 +178,9 @@
return self._read(deadline, retrieve_bytes_from_stdout_buffer)
- def _check_for_crash(self):
+ def _check_for_crash(self, wait_for_crash_reporter=True):
if self._proc.poll() != None:
- self.crashed = True
+ self.set_crashed(True, wait_for_crash_reporter)
self.handle_interrupt()
def _log(self, message):
@@ -207,11 +207,11 @@
def _handle_timeout(self):
self._executive.wait_newest(self._port.is_crash_reporter)
- if not self.crashed:
- self._check_for_crash()
+ self._check_for_crash(wait_for_crash_reporter=False)
+ if self.crashed:
+ return
self.timed_out = True
- if not self.crashed:
- self._sample()
+ self._sample()
def _split_string_after_index(self, string, index):
return string[:index], string[index:]
@@ -288,3 +288,9 @@
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 (102161 => 102162)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/server_process_unittest.py 2011-12-06 20:00:03 UTC (rev 102161)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/server_process_unittest.py 2011-12-06 20:04:08 UTC (rev 102162)
@@ -50,7 +50,10 @@
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
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit.py (102161 => 102162)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit.py 2011-12-06 20:00:03 UTC (rev 102161)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit.py 2011-12-06 20:04:08 UTC (rev 102162)
@@ -501,7 +501,7 @@
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.crash = True
+ self._server_process.set_crashed(True)
elif error_line == "#CRASHED - WebProcess\n":
# WebKitTestRunner uses this to report that the WebProcess subprocess crashed.
self._subprocess_crashed("WebProcess")
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes
