[issue5673] Add timeout option to subprocess.Popen

[Alexander Belopolsky]

Alexander Belopolsky <belopolsky at users.sourceforge.net> added the comment:

The documentation should mention somewhere that timeout can be a float.  For example, as in time.sleep docstring:

"""
    sleep(seconds)
    
    Delay execution for a given number of seconds.  The argument may be
    a floating point number for subsecond precision.
"""

I would also like to see some discussion of supported precision.  Is is the same as for time.sleep()?  Does float precision ever affect timeout precision? (On systems with nanosleep it may, but probably this has no practical concequences.)

This can be done as a future enhancement, but I would like to see datetime.timedelta as an acceptable type for timeout.  This can be done by adding duck-typed code in the error branch which would attempt to call timeout.total_seconds() to extract a float.

Looking further, it appears that timeout can be anything that can be added to a float to produce float.  Is this an accident of implementation or a design decision?  Note that a result Fraction can be used as timeout but Decimal cannot.

Zero and negative timeouts are accepted by subprocess.call(), but the result is not documented.  It looks like this still starts the process, but kills it immediately. An alternative would be to not start the process at all or disallow negative or maybe even zero timeouts altogether.  I don't mind the current choice, but it should be documented at least in Popen.wait(timeout=None) section.

+        def wait(self, timeout=None, endtime=None):
             """Wait for child process to terminate.  Returns returncode
             attribute."""

Docstring should describe timeout and endtime arguments.  In fact I don't see endtime documented anywhere.  It is not an obvious choice
that endtime is ignored when timeout is given.  An alternative would be to terminate at min(now + timeout, endtime).

+                delay = 0.0005 # 500 us -> initial delay of 1 ms

I think this should be an argument to wait() and the use of busy loop should be documented.

+                    delay = min(delay * 2, remaining, .05)

Why .05?  It would probably be an overkill to make this another argument, but maybe make it an attribute of Popen, say self._max_busy_loop_delay or a shorter descriptive name of your choice.  If you start it with '_', you don't need to document it, but users may be able to mess with it if they suspect that 0.05 is not the right choice.

+                endtime = time.time() + timeout

Did you consider using datetime module instead of time module here?  (I know, you still need time.sleep() later, but you won't need to worry about variable precision of time.time().)

----------

_______________________________________
Python tracker <report at bugs.python.org>
<http://bugs.python.org/issue5673>
_______________________________________