On Wed, Oct 14, 2015 at 04:27:56PM +0100, Thomas Wood wrote: > On 9 October 2015 at 20:53, <[email protected]> wrote: > > From: Dylan Baker <[email protected]> > > > > 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 <[email protected]> > > --- > > 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 process is not killed when the timeout occurs, so the previous > code to implement that needs to be added here. This involves sending > SIGTERM to the child process and then sending SIGKILL to the process > group if SIGTERM failed to remove the process. The child processes > were placed in their own process group by the preexec_fn parameter > being set to __set_process_group. >
If you have subprocess32 it will be killed when the timeout expires.
This is a backport of the python3 behavior, from the python3 docs:
"""
The timeout argument is passed to Popen.communicate(). If the timeout
expires, the child process will be killed and waited for. The
TimeoutExpired exception will be re-raised after the child process has
terminated.
"""
Am I missing something here?
>
> >
> > # 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
> >
signature.asc
Description: PGP signature
_______________________________________________ Piglit mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/piglit
