Bug 1265584 - Move wptrunner marionette usage onto a single thread, r=ato
Running marionette on a background thread is problematic in the case
that a test times out. In this case the background thread is not
terminated. If we then call into marionette again on the main thread
we may race with something that happens on the runner thread. The
marionette client isn't threadsafe, so this leads to buggy behaviour.
The simplest fx for the problem is just to move all the marionette
calls onto the main thread and instead of waiting on the main thread,
spin up a thread with a timer.
MozReview-Commit-ID: 3vVlMcwPHSx
--- a/testing/web-platform/tests/tools/wptrunner/wptrunner/executors/executormarionette.py
+++ b/testing/web-platform/tests/tools/wptrunner/wptrunner/executors/executormarionette.py
@@ -432,25 +432,28 @@ class ExecuteAsyncScriptRun(object):
# We just want it to never time out, really, but marionette doesn't
# make that possible. It also seems to time out immediately if the
# timeout is set too high. This works at least.
self.protocol.base.set_timeout(2**28 - 1)
except IOError:
self.logger.error("Lost marionette connection before starting test")
return Stop
- executor = threading.Thread(target = self._run)
- executor.start()
-
if timeout is not None:
wait_timeout = timeout + 2 * extra_timeout
else:
wait_timeout = None
- flag = self.result_flag.wait(wait_timeout)
+ timer = threading.Timer(wait_timeout, self._timeout)
+ timer.start()
+
+ self._run()
+
+ self.result_flag.wait()
+ timer.cancel()
if self.result == (None, None):
self.logger.debug("Timed out waiting for a result")
self.result = False, ("EXTERNAL-TIMEOUT", None)
elif self.result[1] is None:
# We didn't get any data back from the test, so check if the
# browser is still responsive
if self.protocol.is_alive:
@@ -475,16 +478,20 @@ class ExecuteAsyncScriptRun(object):
if message:
message += "\n"
message += traceback.format_exc(e)
self.logger.warning(traceback.format_exc())
self.result = False, ("INTERNAL-ERROR", e)
finally:
self.result_flag.set()
+ def _timeout(self):
+ self.result = False, ("EXTERNAL-TIMEOUT", None)
+ self.result_flag.set()
+
class MarionetteTestharnessExecutor(TestharnessExecutor):
supports_testdriver = True
def __init__(self, browser, server_config, timeout_multiplier=1,
close_after_done=True, debug_info=None, capabilities=None,
**kwargs):
"""Marionette-based executor for testharness.js tests"""