Bug 1257476 - Marionette has to force close the process if it doesn't shut down itself. r=automatedtester
authorHenrik Skupin <mail@hskupin.info>
Fri, 22 Jul 2016 14:36:47 +0200
changeset 348841 aadff7c91cd6aa6a1a75108129af3a0bce00f4d4
parent 348840 928a0bdcea4c62fa1d192815199051281fd2c49f
child 348842 60de9112cad61ed4ca812931e4fa55257167b853
push id1230
push userjlund@mozilla.com
push dateMon, 31 Oct 2016 18:13:35 +0000
treeherdermozilla-release@5e06e3766db2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersautomatedtester
bugs1257476
milestone50.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1257476 - Marionette has to force close the process if it doesn't shut down itself. r=automatedtester Under some circumstances Marionette currently fails to stop the application in case of socket issues. To ensure that the application always gets closed - in the case when Marionette started it - the check for crashes decorator gets updated to do a full process check. MozReview-Commit-ID: DAiF2ZjAjT5
testing/marionette/client/marionette_driver/decorators.py
testing/marionette/client/marionette_driver/marionette.py
--- a/testing/marionette/client/marionette_driver/decorators.py
+++ b/testing/marionette/client/marionette_driver/decorators.py
@@ -1,55 +1,61 @@
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
-from errors import MarionetteException
+from errors import MarionetteException, TimeoutException
 from functools import wraps
 import socket
 import sys
 import traceback
 
 
 def _find_marionette_in_args(*args, **kwargs):
     try:
         m = [a for a in args + tuple(kwargs.values()) if hasattr(a, 'session')][0]
     except IndexError:
         print("Can only apply decorator to function using a marionette object")
         raise
     return m
 
 
-def do_crash_check(func, always=False):
-    """Decorator which checks for crashes after the function has run.
+def do_process_check(func, always=False):
+    """Decorator which checks the process after the function has run.
+
+    There is a check for crashes which always gets executed. And in the case of
+    connection issues the process will be force closed.
 
     :param always: If False, only checks for crashes if an exception
                    was raised. If True, always checks for crashes.
     """
     @wraps(func)
     def _(*args, **kwargs):
-        def check():
-            m = _find_marionette_in_args(*args, **kwargs)
+        m = _find_marionette_in_args(*args, **kwargs)
+
+        def check_for_crash():
             try:
                 m.check_for_crash()
             except:
                 # don't want to lose the original exception
                 traceback.print_exc()
 
         try:
             return func(*args, **kwargs)
         except (MarionetteException, socket.error, IOError) as e:
             exc, val, tb = sys.exc_info()
             if not isinstance(e, MarionetteException) or type(e) is MarionetteException:
                 if not always:
-                    check()
+                    check_for_crash()
+            if not isinstance(e, MarionetteException) or type(e) is TimeoutException:
+                m.force_shutdown()
             raise exc, val, tb
         finally:
             if always:
-                check()
+                check_for_crash(m)
     return _
 
 
 def uses_marionette(func):
     """Decorator which creates a marionette session and deletes it
     afterwards if one doesn't already exist.
     """
     @wraps(func)
--- a/testing/marionette/client/marionette_driver/marionette.py
+++ b/testing/marionette/client/marionette_driver/marionette.py
@@ -2,22 +2,23 @@
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 import base64
 import ConfigParser
 import json
 import os
 import socket
+import sys
 import traceback
 import warnings
 
 from contextlib import contextmanager
 
-from decorators import do_crash_check
+from decorators import do_process_check
 from keys import Keys
 
 import geckoinstance
 import errors
 import transport
 
 WEBELEMENT_KEY = "ELEMENT"
 W3C_WEBELEMENT_KEY = "element-6066-11e4-a52e-4f735466cecf"
@@ -639,22 +640,22 @@ class Marionette(object):
             return False
         finally:
             s.close()
 
     def wait_for_port(self, timeout=None):
         timeout = timeout or self.DEFAULT_STARTUP_TIMEOUT
         return transport.wait_for_port(self.host, self.port, timeout=timeout)
 
-    @do_crash_check
+    @do_process_check
     def raise_for_port(self, port_obtained):
         if not port_obtained:
             raise IOError("Timed out waiting for port!")
 
-    @do_crash_check
+    @do_process_check
     def _send_message(self, name, params=None, key=None):
         """Send a blocking message to the server.
 
         Marionette provides an asynchronous, non-blocking interface and
         this attempts to paper over this by providing a synchronous API
         to the user.
 
         :param name: Requested command key.
@@ -674,24 +675,16 @@ class Marionette(object):
                 if params:
                     data["parameters"] = params
                 self.client.send(data)
                 msg = self.client.receive()
 
             else:
                 msg = self.client.request(name, params)
 
-        except IOError:
-            if self.instance:
-                # If we've launched the binary we've connected to, wait
-                # for it to shut down.
-                returncode = self.instance.runner.wait(timeout=self.DEFAULT_STARTUP_TIMEOUT)
-                raise IOError("process died with returncode %s" % returncode)
-            raise
-
         except socket.timeout:
             self.session = None
             self.window = None
             self.client.close()
             raise errors.TimeoutException("Connection timed out")
 
         res, err = msg.result, msg.error
         if err:
@@ -746,16 +739,38 @@ class Marionette(object):
             if self.instance.runner.check_for_crashes(
                     test_name=self.test_name):
                 crashed = True
         if returncode is not None:
             print ('PROCESS-CRASH | %s | abnormal termination with exit code %d' %
                    (name, returncode))
         return crashed
 
+    def force_shutdown(self):
+        """Force a shutdown of the running instance.
+
+        If we've launched the binary we are connected to, wait for it to shut down.
+        In the case when it doesn't happen, force its shut down.
+
+        """
+        if self.instance:
+            exc, val, tb = sys.exc_info()
+
+            returncode = self.instance.runner.returncode
+            if returncode is None:
+                self.instance.runner.stop()
+                message = 'Process killed because the connection was lost'
+            else:
+                message = 'Process died with returncode "{returncode}"'
+
+            if exc:
+                message += ' (Reason: {reason})'
+
+            raise exc, message.format(returncode=returncode, reason=val), tb
+
     @staticmethod
     def convert_keys(*string):
         typing = []
         for val in string:
             if isinstance(val, Keys):
                 typing.append(val)
             elif isinstance(val, int):
                 val = str(val)