[Piglit] [PATCH 2/2] framework/test/base.py: try to use subprocess32

baker.dylan.c at gmail.com baker.dylan.c at gmail.com
Fri Oct 9 12:53:38 PDT 2015


From: Dylan Baker <baker.dylan.c at gmail.com>

This adds subprocess32 as an optional dependency to get timeouts. This
module is a backport of the feature from python 3.2 that adds the
timeout argument to Popen.communicate (among others).

There is a caveat to this feature. Unlike the python 3.x version, this
doesn't work on windows, although that isn't a change from the current
state of affairs, as the current timeouts don't work on windows

What this does do though, is get us timeouts that work with concurrency
on posix systems (like Linux and OSX).

Signed-off-by: Dylan Baker <dylanx.c.baker at intel.com>
---
 README                        |   5 +-
 framework/test/base.py        | 123 +++++++++++++-----------------------------
 framework/tests/base_tests.py |   2 +
 tox.ini                       |   1 +
 4 files changed, 43 insertions(+), 88 deletions(-)

diff --git a/README b/README
index be8f4df..5813af5 100644
--- a/README
+++ b/README
@@ -47,9 +47,12 @@ Optionally, you can install the following:
   - lxml. An accelerated python xml library using libxml2 (http://lxml.de/)
   - simplejson. A fast C based implementation of the python json library.
     (https://simplejson.readthedocs.org/en/latest/)
-  - backports.lzma. A packport of python3's lzma module to python2,
+  - backports.lzma. A backport of python3's lzma module to python2,
     this enables fast native xz (de)compression in piglit for results files
     (https://github.com/peterjc/backports.lzma)
+  - subprocess32. A backport of python 3.2's subprocess module to python 2.
+    This provides the timeout mechanism.
+    (https://pypi.python.org/pypi/subprocess32/)
 
 Now configure the build system:
 
diff --git a/framework/test/base.py b/framework/test/base.py
index bf00396..0f01ccc 100644
--- a/framework/test/base.py
+++ b/framework/test/base.py
@@ -29,13 +29,33 @@ import subprocess
 import time
 import sys
 import traceback
-from datetime import datetime
-import threading
-import signal
 import itertools
 import abc
 import copy
 
+try:
+    from subprocess32 import Popen, TimeoutExpired
+except ImportError:
+    from subprocess import Popen as _Popen
+
+    class Popen(_Popen):
+        """Wrapper to provide timout on communicate.
+
+        This allows us to use the same code without a plethora of if statements
+        to special case windows an non subprocess32 cases.
+
+        """
+        def communicate(self, *args, **kwargs):
+            try:
+                del kwargs['timeout']
+            except KeyError:
+                pass
+
+            super(Popen, self).communicate(*args, **kwargs)
+
+    class TimeoutExpired(Exception):
+        pass
+
 from framework import exceptions
 from framework.core import Options
 from framework.results import TestResult
@@ -62,56 +82,6 @@ class TestRunError(exceptions.PiglitException):
         self.status = status
 
 
-class ProcessTimeout(threading.Thread):
-    """ Timeout class for test processes
-
-    This class is for terminating tests that run for longer than a certain
-    amount of time. Create an instance by passing it a timeout in seconds
-    and the Popen object that is running your test. Then call the start
-    method (inherited from Thread) to start the timer. The Popen object is
-    killed if the timeout is reached and it has not completed. Wait for the
-    outcome by calling the join() method from the parent.
-
-    """
-
-    def __init__(self, timeout, proc):
-        threading.Thread.__init__(self)
-        self.proc = proc
-        self.timeout = timeout
-        self.status = 0
-
-    def run(self):
-        start_time = datetime.now()
-        delta = 0
-
-        # poll() returns the returncode attribute, which is either the return
-        # code of the child process (which could be zero), or None if the
-        # process has not yet terminated.
-
-        while delta < self.timeout and self.proc.poll() is None:
-            time.sleep(1)
-            delta = (datetime.now() - start_time).total_seconds()
-
-        # if the test is not finished after timeout, first try to terminate it
-        # and if that fails, send SIGKILL to all processes in the test's
-        # process group
-
-        if self.proc.poll() is None:
-            self.status = 1
-            self.proc.terminate()
-            time.sleep(5)
-
-        if self.proc.poll() is None:
-            self.status = 2
-            if hasattr(os, 'killpg'):
-                os.killpg(self.proc.pid, signal.SIGKILL)
-            self.proc.wait()
-
-    def join(self):
-        threading.Thread.join(self)
-        return self.status
-
-
 def _is_crash_returncode(returncode):
     """Determine whether the given process return code correspond to a
     crash.
@@ -147,8 +117,7 @@ class Test(object):
     """
     OPTS = Options()
     __metaclass__ = abc.ABCMeta
-    __slots__ = ['run_concurrent', 'env', 'result', 'cwd', '_command',
-                 '__proc_timeout']
+    __slots__ = ['run_concurrent', 'env', 'result', 'cwd', '_command']
     timeout = 0
 
     def __init__(self, command, run_concurrent=False):
@@ -159,7 +128,6 @@ class Test(object):
         self.env = {}
         self.result = TestResult()
         self.cwd = None
-        self.__proc_timeout = None
 
     def execute(self, path, log, dmesg):
         """ Run a test
@@ -205,11 +173,7 @@ class Test(object):
         """Convert the raw output of the test into a form piglit understands.
         """
         if _is_crash_returncode(self.result.returncode):
-            # check if the process was terminated by the timeout
-            if self.timeout > 0 and self.__proc_timeout.join() > 0:
-                self.result.result = 'timeout'
-            else:
-                self.result.result = 'crash'
+            self.result.result = 'crash'
         elif self.result.returncode != 0 and self.result.result == 'pass':
             self.result.result = 'warn'
 
@@ -255,10 +219,6 @@ class Test(object):
         """
         pass
 
-    def __set_process_group(self):
-        if hasattr(os, 'setpgrp'):
-            os.setpgrp()
-
     def _run_command(self):
         """ Run the test command and get the result
 
@@ -285,29 +245,14 @@ class Test(object):
                                           self.env.iteritems()):
             fullenv[key] = str(value)
 
-        # preexec_fn is not supported on Windows platforms
-        if sys.platform == 'win32':
-            preexec_fn = None
-        else:
-            preexec_fn = self.__set_process_group
-
         try:
-            proc = subprocess.Popen(self.command,
-                                    stdout=subprocess.PIPE,
-                                    stderr=subprocess.PIPE,
-                                    cwd=self.cwd,
-                                    env=fullenv,
-                                    universal_newlines=True,
-                                    preexec_fn=preexec_fn)
-            # create a ProcessTimeout object to watch out for test hang if the
-            # process is still going after the timeout, then it will be killed
-            # forcing the communicate function (which is a blocking call) to
-            # return
-            if self.timeout > 0:
-                self.__proc_timeout = ProcessTimeout(self.timeout, proc)
-                self.__proc_timeout.start()
-
-            out, err = proc.communicate()
+            proc = Popen(self.command,
+                         stdout=subprocess.PIPE,
+                         stderr=subprocess.PIPE,
+                         cwd=self.cwd,
+                         env=fullenv,
+                         universal_newlines=True)
+            out, err = proc.communicate(timeout=self.timeout)
             returncode = proc.returncode
         except OSError as e:
             # Different sets of tests get built under different build
@@ -317,6 +262,10 @@ class Test(object):
                 raise TestRunError("Test executable not found.\n", 'skip')
             else:
                 raise e
+        except TimeoutExpired:
+            raise TestRunError(
+                'Test time out exceeded timeout of {}\n'.format(self.timeout),
+                'timeout')
 
         # The setter handles the bytes/unicode conversion
         self.result.out = out
diff --git a/framework/tests/base_tests.py b/framework/tests/base_tests.py
index caef9e3..95d35bd 100644
--- a/framework/tests/base_tests.py
+++ b/framework/tests/base_tests.py
@@ -63,6 +63,7 @@ def test_run_return_early():
 @attr('slow')
 def test_timeout():
     """test.base.Test.run(): kills tests that exceed timeout when set"""
+    utils.module_check('subprocess32')
     utils.binary_check('sleep', 1)
 
     class _Test(Test):
@@ -79,6 +80,7 @@ def test_timeout():
 def test_timeout_pass():
     """test.base.Test.run(): Result is returned when timeout is set but not exceeded
     """
+    utils.module_check('subprocess32')
     utils.binary_check('true')
 
     def helper():
diff --git a/tox.ini b/tox.ini
index ca97a51..366af9b 100644
--- a/tox.ini
+++ b/tox.ini
@@ -27,4 +27,5 @@ deps =
     simplejson
     lxml
     backports.lzma
+    subprocess32
 commands = nosetests framework/tests --attr="!privliged" --with-cover --cover-package=framework --cover-tests
-- 
2.6.1



More information about the Piglit mailing list