Diff
Modified: trunk/Tools/ChangeLog (88847 => 88848)
--- trunk/Tools/ChangeLog 2011-06-14 20:35:15 UTC (rev 88847)
+++ trunk/Tools/ChangeLog 2011-06-14 20:36:57 UTC (rev 88848)
@@ -1,3 +1,22 @@
+2011-06-14 Dirk Pranke <[email protected]>
+
+ Reviewed by Tony Chang.
+
+ nrwt: handle worker exceptions cleanly
+ https://bugs.webkit.org/show_bug.cgi?id=62614
+
+ This change modifiers new-run-webkit-tests to handle exceptions
+ raised by worker threads better, by:
+ - capturing the worker's stack and logging it in the manager
+ - propagating the worker's exception in the caller correctly
+ - attempting to cancel the workers and clean up even when
+ we get an unexpected exception
+
+ * Scripts/webkitpy/layout_tests/layout_package/manager.py:
+ * Scripts/webkitpy/layout_tests/layout_package/worker.py:
+ * Scripts/webkitpy/layout_tests/run_webkit_tests.py:
+ * Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py:
+
2011-06-14 Qi Zhang <[email protected]>
Reviewed by Laszlo Gombos.
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/layout_package/manager.py (88847 => 88848)
--- trunk/Tools/Scripts/webkitpy/layout_tests/layout_package/manager.py 2011-06-14 20:35:15 UTC (rev 88847)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/layout_package/manager.py 2011-06-14 20:36:57 UTC (rev 88848)
@@ -229,6 +229,11 @@
return self.__class__, (self.reason,)
+class WorkerException(Exception):
+ """Raised when we receive an unexpected/unknown exception from a worker."""
+ pass
+
+
class Manager:
"""A class for managing running a series of tests on a series of layout
test files."""
@@ -687,9 +692,13 @@
_log.info(e.reason)
self.cancel_workers()
interrupted = True
+ except WorkerException:
+ self.cancel_workers()
+ raise
except:
# Unexpected exception; don't try to clean up workers.
- _log.info("Exception raised, exiting")
+ _log.error("Exception raised, exiting")
+ self.cancel_workers()
raise
thread_timings = [worker_state.stats for worker_state in self._worker_states.values()]
@@ -1309,9 +1318,15 @@
worker_state = self._worker_states[source]
worker_state.done = True
- def handle_exception(self, source, exception_info):
- exception_type, exception_value, exception_traceback = exception_info
- raise exception_type, exception_value, exception_traceback
+ def handle_exception(self, source, exception_type, exception_value, stack):
+ if exception_type in (KeyboardInterrupt, TestRunInterruptedException):
+ raise exception_type(exception_value)
+ _log.error("%s raised %s('%s'):" % (
+ source,
+ exception_value.__class__.__name__,
+ str(exception_value)))
+ self._log_worker_stack(stack)
+ raise WorkerException(str(exception_value))
def handle_finished_list(self, source, list_name, num_tests, elapsed_time):
self._group_stats[list_name] = (num_tests, elapsed_time)
@@ -1330,6 +1345,13 @@
self._all_results.append(result)
self._update_summary_with_result(self._current_result_summary, result)
+ def _log_worker_stack(self, stack):
+ webkitpydir = self._port.path_from_webkit_base('Tools', 'Scripts', 'webkitpy') + self._port._filesystem.sep
+ for filename, line_number, function_name, text in stack:
+ if filename.startswith(webkitpydir):
+ filename = filename.replace(webkitpydir, '')
+ _log.error(' %s:%u (in %s)' % (filename, line_number, function_name))
+ _log.error(' %s' % text)
def read_test_files(fs, files):
tests = []
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/layout_package/worker.py (88847 => 88848)
--- trunk/Tools/Scripts/webkitpy/layout_tests/layout_package/worker.py 2011-06-14 20:35:15 UTC (rev 88847)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/layout_package/worker.py 2011-06-14 20:36:57 UTC (rev 88848)
@@ -32,6 +32,7 @@
import sys
import threading
import time
+import traceback
from webkitpy.common.system import stack_utils
from webkitpy.layout_tests.layout_package import manager_worker_broker
@@ -99,20 +100,23 @@
% self._name)
except KeyboardInterrupt:
exception_msg = ", interrupted"
+ self.post_exception()
except:
exception_msg = ", exception raised"
+ self.post_exception()
finally:
_log.debug("%s done with message loop%s" % (self._name, exception_msg))
- if exception_msg:
- exception_type, exception_value, exception_traceback = sys.exc_info()
- stack_utils.log_traceback(_log.debug, exception_traceback)
- # FIXME: Figure out how to send a message with a traceback.
- self._worker_connection.post_message('exception',
- (exception_type, exception_value, None))
self._worker_connection.post_message('done')
self.cleanup()
_log.debug("%s exiting" % self._name)
+ def post_exception(self):
+ # Since tracebacks aren't picklable, send the extracted stack instead.
+ exception_type, exception_value, exception_traceback = sys.exc_info()
+ stack_utils.log_traceback(_log.debug, exception_traceback)
+ stack = traceback.extract_tb(exception_traceback)
+ self._worker_connection.post_message('exception', exception_type, exception_value, stack)
+
def handle_test_list(self, src, list_name, test_list):
if list_name == "tests_to_http_lock":
self.start_servers_with_lock()
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py (88847 => 88848)
--- trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py 2011-06-14 20:35:15 UTC (rev 88847)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py 2011-06-14 20:36:57 UTC (rev 88848)
@@ -52,6 +52,8 @@
_log = logging.getLogger(__name__)
+WorkerException = manager.WorkerException
+
def run(port, options, args, regular_output=sys.stderr,
buildbot_output=sys.stdout):
"""Run the tests.
@@ -459,5 +461,11 @@
try:
sys.exit(main())
except KeyboardInterrupt:
- # this mirrors what the shell normally does
- sys.exit(signal.SIGINT + 128)
+ # This mirrors what the shell normally does.
+ INTERRUPTED_EXIT_STATUS = signal.SIGINT + 128
+ sys.exit(INTERRUPTED_EXIT_STATUS)
+ except WorkerException, e:
+ # This is a randomly chosen exit code that can be tested against to
+ # indicate that an unexpected exception occurred.
+ EXCEPTIONAL_EXIT_STATUS = 254
+ sys.exit(EXCEPTIONAL_EXIT_STATUS)
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py (88847 => 88848)
--- trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py 2011-06-14 20:35:15 UTC (rev 88847)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py 2011-06-14 20:36:57 UTC (rev 88848)
@@ -220,7 +220,7 @@
self.assertEqual(batch_tests_run, [])
def test_exception_raised(self):
- self.assertRaises(ValueError, logging_run,
+ self.assertRaises(run_webkit_tests.WorkerException, logging_run,
['failures/expected/exception.html'], tests_included=True)
def test_full_results_html(self):
@@ -319,7 +319,7 @@
def test_run_force(self):
# This raises an exception because we run
# failures/expected/exception.html, which is normally SKIPped.
- self.assertRaises(ValueError, logging_run, ['--force'])
+ self.assertRaises(run_webkit_tests.WorkerException, logging_run, ['--force'])
def test_run_part(self):
# Test that we actually select the right part
