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 331805 aadff7c91cd6aa6a1a75108129af3a0bce00f4d4
parent 331804 928a0bdcea4c62fa1d192815199051281fd2c49f
child 331806 60de9112cad61ed4ca812931e4fa55257167b853
push id9858
push userjlund@mozilla.com
push dateMon, 01 Aug 2016 14:37:10 +0000
treeherdermozilla-aurora@203106ef6cb6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
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
--- 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):
         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")
     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.
     def _(*args, **kwargs):
-        def check():
-            m = _find_marionette_in_args(*args, **kwargs)
+        m = _find_marionette_in_args(*args, **kwargs)
+        def check_for_crash():
                 # don't want to lose the original exception
             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
             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.
--- 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
 W3C_WEBELEMENT_KEY = "element-6066-11e4-a52e-4f735466cecf"
@@ -639,22 +640,22 @@ class Marionette(object):
             return False
     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
                 msg = self.client.receive()
                 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
             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(
                 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
     def convert_keys(*string):
         typing = []
         for val in string:
             if isinstance(val, Keys):
             elif isinstance(val, int):
                 val = str(val)