Include the timeout value in TimeoutExpired.

This was the original intention, but it wasn't threaded all the way through due
to 'endtime'.  Also added a trivial assertion to get coverage of __str__.
This commit is contained in:
Reid Kleckner 2011-03-16 16:57:54 -04:00
parent 252183e4b6
commit 2b228f0d9b
2 changed files with 35 additions and 23 deletions

View File

@ -371,8 +371,9 @@ class TimeoutExpired(SubprocessError):
"""This exception is raised when the timeout expires while waiting for a """This exception is raised when the timeout expires while waiting for a
child process. child process.
""" """
def __init__(self, cmd, output=None): def __init__(self, cmd, timeout, output=None):
self.cmd = cmd self.cmd = cmd
self.timeout = timeout
self.output = output self.output = output
def __str__(self): def __str__(self):
@ -533,7 +534,7 @@ def check_output(*popenargs, timeout=None, **kwargs):
except TimeoutExpired: except TimeoutExpired:
process.kill() process.kill()
output, unused_err = process.communicate() output, unused_err = process.communicate()
raise TimeoutExpired(process.args, output=output) raise TimeoutExpired(process.args, timeout, output=output)
retcode = process.poll() retcode = process.poll()
if retcode: if retcode:
raise CalledProcessError(retcode, process.args, output=output) raise CalledProcessError(retcode, process.args, output=output)
@ -844,7 +845,7 @@ class Popen(object):
return (stdout, stderr) return (stdout, stderr)
try: try:
stdout, stderr = self._communicate(input, endtime) stdout, stderr = self._communicate(input, endtime, timeout)
finally: finally:
self._communication_started = True self._communication_started = True
@ -865,12 +866,12 @@ class Popen(object):
return endtime - time.time() return endtime - time.time()
def _check_timeout(self, endtime): def _check_timeout(self, endtime, orig_timeout):
"""Convenience for checking if a timeout has expired.""" """Convenience for checking if a timeout has expired."""
if endtime is None: if endtime is None:
return return
if time.time() > endtime: if time.time() > endtime:
raise TimeoutExpired(self.args) raise TimeoutExpired(self.args, orig_timeout)
if mswindows: if mswindows:
@ -1063,9 +1064,11 @@ class Popen(object):
return self.returncode return self.returncode
def wait(self, timeout=None): def wait(self, timeout=None, endtime=None):
"""Wait for child process to terminate. Returns returncode """Wait for child process to terminate. Returns returncode
attribute.""" attribute."""
if endtime is not None:
timeout = self._remaining_time(endtime)
if timeout is None: if timeout is None:
timeout = _subprocess.INFINITE timeout = _subprocess.INFINITE
else: else:
@ -1073,7 +1076,7 @@ class Popen(object):
if self.returncode is None: if self.returncode is None:
result = _subprocess.WaitForSingleObject(self._handle, timeout) result = _subprocess.WaitForSingleObject(self._handle, timeout)
if result == _subprocess.WAIT_TIMEOUT: if result == _subprocess.WAIT_TIMEOUT:
raise TimeoutExpired(self.args) raise TimeoutExpired(self.args, timeout)
self.returncode = _subprocess.GetExitCodeProcess(self._handle) self.returncode = _subprocess.GetExitCodeProcess(self._handle)
return self.returncode return self.returncode
@ -1083,7 +1086,7 @@ class Popen(object):
fh.close() fh.close()
def _communicate(self, input, endtime): def _communicate(self, input, endtime, orig_timeout):
# Start reader threads feeding into a list hanging off of this # Start reader threads feeding into a list hanging off of this
# object, unless they've already been started. # object, unless they've already been started.
if self.stdout and not hasattr(self, "_stdout_buff"): if self.stdout and not hasattr(self, "_stdout_buff"):
@ -1489,13 +1492,18 @@ class Popen(object):
def wait(self, timeout=None, endtime=None): def wait(self, timeout=None, endtime=None):
"""Wait for child process to terminate. Returns returncode """Wait for child process to terminate. Returns returncode
attribute.""" attribute."""
# If timeout was passed but not endtime, compute endtime in terms of
# timeout.
if endtime is None and timeout is not None:
endtime = time.time() + timeout
if self.returncode is not None: if self.returncode is not None:
return self.returncode return self.returncode
elif endtime is not None:
# endtime is preferred to timeout. timeout is only used for
# printing.
if endtime is not None or timeout is not None:
if endtime is None:
endtime = time.time() + timeout
elif timeout is None:
timeout = self._remaining_time(endtime)
if endtime is not None:
# Enter a busy loop if we have a timeout. This busy loop was # Enter a busy loop if we have a timeout. This busy loop was
# cribbed from Lib/threading.py in Thread.wait() at r71065. # cribbed from Lib/threading.py in Thread.wait() at r71065.
delay = 0.0005 # 500 us -> initial delay of 1 ms delay = 0.0005 # 500 us -> initial delay of 1 ms
@ -1507,7 +1515,7 @@ class Popen(object):
break break
remaining = self._remaining_time(endtime) remaining = self._remaining_time(endtime)
if remaining <= 0: if remaining <= 0:
raise TimeoutExpired(self.args) raise TimeoutExpired(self.args, timeout)
delay = min(delay * 2, remaining, .05) delay = min(delay * 2, remaining, .05)
time.sleep(delay) time.sleep(delay)
elif self.returncode is None: elif self.returncode is None:
@ -1516,7 +1524,7 @@ class Popen(object):
return self.returncode return self.returncode
def _communicate(self, input, endtime): def _communicate(self, input, endtime, orig_timeout):
if self.stdin and not self._communication_started: if self.stdin and not self._communication_started:
# Flush stdio buffer. This might block, if the user has # Flush stdio buffer. This might block, if the user has
# been writing to .stdin in an uncontrolled fashion. # been writing to .stdin in an uncontrolled fashion.
@ -1525,9 +1533,11 @@ class Popen(object):
self.stdin.close() self.stdin.close()
if _has_poll: if _has_poll:
stdout, stderr = self._communicate_with_poll(input, endtime) stdout, stderr = self._communicate_with_poll(input, endtime,
orig_timeout)
else: else:
stdout, stderr = self._communicate_with_select(input, endtime) stdout, stderr = self._communicate_with_select(input, endtime,
orig_timeout)
self.wait(timeout=self._remaining_time(endtime)) self.wait(timeout=self._remaining_time(endtime))
@ -1550,7 +1560,7 @@ class Popen(object):
return (stdout, stderr) return (stdout, stderr)
def _communicate_with_poll(self, input, endtime): def _communicate_with_poll(self, input, endtime, orig_timeout):
stdout = None # Return stdout = None # Return
stderr = None # Return stderr = None # Return
@ -1601,7 +1611,7 @@ class Popen(object):
if e.args[0] == errno.EINTR: if e.args[0] == errno.EINTR:
continue continue
raise raise
self._check_timeout(endtime) self._check_timeout(endtime, orig_timeout)
# XXX Rewrite these to use non-blocking I/O on the # XXX Rewrite these to use non-blocking I/O on the
# file objects; they are no longer using C stdio! # file objects; they are no longer using C stdio!
@ -1625,7 +1635,7 @@ class Popen(object):
return (stdout, stderr) return (stdout, stderr)
def _communicate_with_select(self, input, endtime): def _communicate_with_select(self, input, endtime, orig_timeout):
if not self._communication_started: if not self._communication_started:
self._read_set = [] self._read_set = []
self._write_set = [] self._write_set = []
@ -1667,9 +1677,9 @@ class Popen(object):
# According to the docs, returning three empty lists indicates # According to the docs, returning three empty lists indicates
# that the timeout expired. # that the timeout expired.
if not (rlist or wlist or xlist): if not (rlist or wlist or xlist):
raise TimeoutExpired(self.args) raise TimeoutExpired(self.args, orig_timeout)
# We also check what time it is ourselves for good measure. # We also check what time it is ourselves for good measure.
self._check_timeout(endtime) self._check_timeout(endtime, orig_timeout)
# XXX Rewrite these to use non-blocking I/O on the # XXX Rewrite these to use non-blocking I/O on the
# file objects; they are no longer using C stdio! # file objects; they are no longer using C stdio!

View File

@ -651,7 +651,9 @@ class ProcessTestCase(BaseTestCase):
def test_wait_timeout(self): def test_wait_timeout(self):
p = subprocess.Popen([sys.executable, p = subprocess.Popen([sys.executable,
"-c", "import time; time.sleep(0.1)"]) "-c", "import time; time.sleep(0.1)"])
self.assertRaises(subprocess.TimeoutExpired, p.wait, timeout=0.01) with self.assertRaises(subprocess.TimeoutExpired) as c:
p.wait(timeout=0.01)
self.assertIn("0.01", str(c.exception)) # For coverage of __str__.
self.assertEqual(p.wait(timeout=2), 0) self.assertEqual(p.wait(timeout=2), 0)