I was looking for a way to run a subprocess with a timeout.  While there
are a variety of solutions on Google, I feel like this functionality
should live in the standard library module.  Apparently Guido thought
this would be good in 2005 but no one did it:
http://mail.python.org/pipermail/python-dev/2005-December/058784.html

I'd be willing to implement it, but I'm not a core dev and I'd need
someone to review it.  I'll start working on a patch now, and if people
think this is a good idea I'll submit it for review.

My plan was to add a 'timeout' optional keyword argument to wait() and
propagate that backwards through communicate(), call(), and
check_call().  Does anyone object to this approach?

Ugh.  I made the assumption that there must be some natural and easy way
to wait for a child process with a timeout in C, and it turns out it's
actually a hard problem, which is why this isn't already implemented.

So my initial hack for solving this problem in my own project was to run
the subprocess, spawn a thread to wait on it, and then use the thread's
wait method, which does accept a timeout.  On further inspection, it
turns out that Thread.wait() actually uses a busy loop to implement the
timeout, which is what I was trying to avoid.  If it's okay to have a
busy loop there, is it okay to have one in Popen.wait()?  Obviously, you
don't need to busy loop if there is no timeout.

I'd like some feedback on this patch.  Is the API acceptable?

Would it be better to throw an exception in wait() instead of returning
None?  

What should communicate() return if it times out?  I can't decide if it
should try to return partial output, return None, or raise an exception.
 If it doesn't return partial output, that output is not recoverable. 
Maybe it should go into the exception object.  On the other hand, the
way that communicate() is implemented with threads on Windows makes it
hard to "interrupt" the file descriptor read and return partial output.
 For that matter, I'm not even sure how to stop those extra threads, as
you can see in the patch.  If anyone can think of a way to avoid using
threads entirely, that would be even better.

What should call() and check_call() return when they timeout?  If they
return None, which is the current returncode attribute, there is no way
of interacting with the process.  They could throw an exception with a
reference to the Popen object.

I think taking this to python-ideas to discuss the API (and the
implementation) would be the best way forward.  You can probably get
help on the Windows stuff there, too.

You are also going to need unit tests.

See http://code.google.com/p/python-process/ for some ideas.

Some comments:

- why do you say Thread.join() uses a busy loop? is it because it uses Condition.wait()? If so, this will be solved in py3k by issue7316 (which you are welcome to review). Otherwise, I think there should be an upper bound on the sleeping granularity (say 2 seconds).

- the "if 'timeout' in kwargs" dance is a bit complicated. Why not simply "kwargs.pop('timeout', None)"?

- if it times out, communicate() should raise a specific exception. Bonus points if the exception holds the partial output as attributes (that's what we do for non-blocking IO in py3k), but if it's too difficult we can leave that out. I don't think returning None would be very good.

- for consistency, other methods should probably raise the same exception. I think we can leave out the more complex scenarios such as "timing out but still processing the beginning of the output".

- I agree that further input from python-dev or python-ideas would be nice

> - why do you say Thread.join() uses a busy loop? is it because it uses
>   Condition.wait()? If so, this will be solved in py3k by issue7316 (which you
>   are welcome to review). Otherwise, I think there should be an upper bound on
>   the sleeping granularity (say 2 seconds).

Yes, that's what I was referring to.  I'm glad to hear the situation will
improve in the future!

> - the "if 'timeout' in kwargs" dance is a bit complicated. Why not simply
>   "kwargs.pop('timeout', None)"?

Good call, done.

> - if it times out, communicate() should raise a specific exception. Bonus points
>   if the exception holds the partial output as attributes (that's what we do for
>   non-blocking IO in py3k), but if it's too difficult we can leave that out. I
>   don't think returning None would be very good.

I agree.  Does subprocess.TimeoutExpired sound good?

It won't be possible with the current implementation to put the partial output
in the exception, because read blocks.  For example, in the Windows threaded
implementation, there's a background thread that just calls self.stdout.read(),
which blocks until its finished.

> - for consistency, other methods should probably raise the same exception. I
>   think we can leave out the more complex scenarios such as "timing out but
>   still processing the beginning of the output".

What do you mean still processing?  I agree, they should all throw the same
exception.  I think call and check_call should clean up after themselves by
killing the child processes they create, while communicate and wait should leave
that to the user.

I'm imagining something like this for communicate:

try:
    (stdout, stderr) = p.communicate(timeout=123)
except subprocess.TimeoutExpired:
    p.kill()
    (stdout, stderr) = p.communicate()  # Should not block long

And nothing special for check_call(cmd=[...], timeout=123).

> I agree.  Does subprocess.TimeoutExpired sound good?

Yes.

> It won't be possible with the current implementation to put the partial output
> in the exception, because read blocks.

Fair enough :)

> I think call and check_call should clean up after themselves by
> killing the child processes they create, while communicate and wait should leave
> that to the user.

It sounds good.

The patch has bitrotted somewhat; I've had a go at reworking it so it applies against the latest version of trunk (r82429).

All tests pass (or are skipped) on this x86_64 Linux box --with-pydebug (Fedora 13)

There are still some TODOs in the code:

Popen.wait():
  # TODO(rnk): Test this on Windows.

Popen._communicate():
            # TODO: Somebody needs to research what happens to those
            # threads if they are still running.  Also, what happens if
            # you close a file descriptor on Windows in one thread?
            # Will it interrupt the other, or does the other keep its
            # own handle?

I went through the trouble of building and testing Python on Windows Vista, and with some small modifications I got the tests I added to pass.

Here's an updated patch.  I'm still not really sure how those threads work on Windows, so I'd rather leave that TODO in until someone with Windows expertise checks it.

I'm looking into the TODO details right now, but the patch as-is didn't pass for me.

The last line of test_communicate_timeout fails on Windows 7 with "pineapple\r\npear\r\n" not matching "pineapple\npear\n". Creating the Popen object with universal_newlines=1 fixes the issue locally (haven't tried on other OSes), but it should probably follow the convention laid out in test_universal_newlines.

I forgot that I had to tweak the test as well as subprocess.py.  I did a .replace('\r', ''), but universal newlines is better.

Looking at the open questions I had about the Windows threads, I think it'll be OK if the user follows the pattern of:
proc = subprocess.Popen(...)
try:
    stdout, stderr = proc.communicate(timeout=...)
except subprocess.TimeoutExpired:
    proc.kill()
    stdout, stderr = proc.communicate()

If the child process is deadlocked and the user doesn't kill it, then the file descriptors will be leaked and the daemon threads will also live on forever.  I *think* that's the worst that could happen.  Or they could of course wakeup during interpreter shutdown and cause tracebacks, but that's highly unlikely, and already possible currently.

Anyway, I would say we can't avoid leaking the fds in that situation, because we can't know if the user will eventually ask us for the data or not.  If they want to avoid the leak, they can clean up after themselves.

What's the next step for getting this in?  Thanks to those who've taken time to look at this so far.

The pattern you mention should probably be documented as an example, if that's how we intend for people to use it. Other than that, I've got nothing else here.

I don't imagine this is going into 2.7.>0 at this point, so I ported the patch to py3k.  I also added support to check_output for the timeout parameter and added docs for all of the methods/functions that now take a timeout in the module.

The communicate docs include the pattern of:
try:
    outs, errs = p.communicate(timeout=15)
except subprocess.TimeoutExpired:
    p.kill()
    outs, errs = p.communicate()

And check_output uses it.

You forgot "self." on at least lines 1042 and 1044 in Lib/subprocess.py -- multiple test failures occur on Windows 7 due to a NameError for the global stdout_thread not being defined. It seems "self." would also be needed on 1049 and 1053, but beyond adding those, the test suite seems to hang indefinitely on test_subprocess.

Uh oh, that was one of the fixes I made when I tested it on Windows.  I may have failed to pick up those changes when I ported to py3k.  I'll check it out tonight.

When I ported the patch I tested on trunk + Windows to py3k, I messed that stuff up.  I also had to fix a bunch of str vs. bytes issues this time around.  On Windows, it uses TextIOWrapper to do the encoding, and on POSIX it uses os.write, so I have to do the encoding myself.  :p

This patch has been tested on Windows Vista and Mac OS X 10.5.

Looks good to me.

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().)

s/Note that a result Fraction/Note that as a result, Fraction/

On Thu, Jul 22, 2010 at 9:05 AM, Alexander Belopolsky
<report@bugs.python.org> wrote:
>
> Alexander Belopolsky <belopolsky@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.)

I added info to wait and communicate, but left the docs for call,
check_call, check_output all saying that their arguments are the same
as Popen(...) and wait(...).

> 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.

I'd prefer to leave it as a future enhancement.

> 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.

Implementation detail.  I don't think it should matter.

> 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).

I didn't intend for the endtime parameter to be documented, it is just
a convenience for implementing communicate, which gets woken up at
various times so it is easier to remember the final deadline rather
than recompute the timeout frequently.

> +                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.

*Points to whoever impelemented it for Thread.wait(timeout=...)*.  If
it was good enough for that (until we got real lock acquisitions with
timeouts), then I think it's good enough for this.

> +                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().)

How does the datetime module help here?  It seems like time.time uses
roughly the same time sources that datetime.datetime.now does.

One other thing I'm worried about here is that time.time can be
non-increasing if the system clock is adjusted.  :(

Maybe someone should file a feature request for a monotonic clock.

Reid

I'd like to report a problem I encountered with the discussed use pattern using subprocess-timeout-v5.patch on linux. I don't have python3 installed at work, sorry.

When running:
p = subprocess.Popen("tcpdump -i eth0 > file &", stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True, universal_newlines=True)
try:
    out, err = p.communicate(timeout=1)
except subprocess.TimeoutExpired:
    p.kill()
    out, err = p.communicate()

After the timeout happens, the last line raises a ValueError: I/O operation on closed file.

The exception is thrown from the register_and_append call for self.stdout in _communicate_with_poll. I'm sorry again for not being able to attach the full traceback.

The problem doesn't reproduce without the '&' or the '> file', and doesn't reproduce with other executables I've tried.

"I don't have python3 installed at work, sorry."

Does that mean you have been using the patch with 2.x? If so, that's not valid.

I've been using the subprocess-timeout-v5.patch patch with 2.x. Isn't that version supposed to work with 2.x?

Other than the stated problem, the patched module generally works, and is very helpful.

"I've been using the subprocess-timeout-v5.patch patch with 2.x. Isn't that version supposed to work with 2.x?"

Actually, yes, so I was wrong at first. The v5 patch will work with 2.x, but that's not the most up to date or correct patch. This feature will only be going into the py3k branch (aka 3.2), so issues with prior patches on 2.x aren't as valuable as those possibly found on the latest patch on 3.x.

I reproduced the problem with the latest patch, subprocess-timeout-py3k-v7.patch, on py3k on Linux.

Attached is the code to reproduce the problem and the resulting traceback.

Has anybody had a chance to look into the problem I encountered yet?

Do you need more information?

No, sorry, I just haven't gotten around to reproducing it on Linux.

And I've even needed this functionality in the mean time, and we worked around it with the standard alarm trick!  =/

Has anybody had a chance to look into the problem I encountered yet?

Do you need more information?

Pablo, so if I understand the issue you've run into correctly, you are using shell redirection to redirect stdout to a file, and then attempting to read from it using stdout=subprocess.PIPE.

It seems to me like this behavior is expected, because the shell will close it's current stdout file descriptor and open a new one pointing at "file".  When python tries to read from its end of the pipe, it complains that the fd has been closed.

I can avoid the problem here either by not reading stdout or by not redirecting to a file.

I updated and committed the patch to the cpython hg repo in revision [c4a0fa6e687c].

On 2011-03-14, at 9:18 AM, Reid Kleckner wrote:

> I updated and committed the patch to the cpython hg repo in revision [c4a0fa6e687c].

Does this go to the main branch (py3.3) only? It is not clear from just looking at http://hg.python.org/cpython/rev/c4a0fa6e687c/

On Mon, Mar 14, 2011 at 12:31 PM, Sridhar Ratnakumar
<report@bugs.python.org> wrote:
>
> Sridhar Ratnakumar <sridharr@activestate.com> added the comment:
>
> On 2011-03-14, at 9:18 AM, Reid Kleckner wrote:
>
>> I updated and committed the patch to the cpython hg repo in revision [c4a0fa6e687c].
>
> Does this go to the main branch (py3.3) only? It is not clear from just looking at http://hg.python.org/cpython/rev/c4a0fa6e687c/

Yes, it's a new feature, so I don't think it's appropriate to backport.

Actually, I just noticed I forgot the update the doc patches.  They
should all say added in 3.3, not 3.2.

Reid

I fixed a bug in _communicate_with_poll(): raise an error if the endtime-time() is negative. If poll() is called with a negative timeout, it blocks until it gets an event.

New changeset 3664fc29e867 by Victor Stinner in branch 'default':
Issue #11757: subprocess ensures that select() and poll() timeout >= 0
http://hg.python.org/cpython/rev/3664fc29e867

By the way, the doc modified by [c4a0fa6e687c] is still wrong: the timeout feature was introduced in 3.3, not in 3.2. And the change is not documented in Misc/NEWS.

@Reid Kleckner: Can you fix that? (I reopen this issue to not forget)

Thanks for fixing the negative timeout issue.  I assumed incorrectly that a negative timeout would cause it to check and return immediately if it would otherwise block.

As for the docs, the 3.2/3.3 issue was fixed in [[72e49cb7fcf5]].

I just added a Misc/NEWS entry for 3.3's What's New in [[9140f2363623]].

For Python 2.x there is a backport of the subprocess module of 3.x: https://pypi.python.org/pypi/subprocess32.

It has the timeout argument for call() and pipe.wait().