[issue5673] Add timeout option to subprocess.Popen
Thomas Guettler added the comment: 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(). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5673 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
Reid Kleckner r...@mit.edu added the comment: 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]]. -- status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5673 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
STINNER Victor victor.stin...@haypocalc.com added the comment: 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) -- nosy: +haypo status: closed - open ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5673 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
Reid Kleckner r...@mit.edu added the comment: I updated and committed the patch to the cpython hg repo in revision [c4a0fa6e687c]. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5673 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
Sridhar Ratnakumar sridh...@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/ -- Added file: http://bugs.python.org/file21121/unnamed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5673 ___htmlhead/headbody style=word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; brdivdivOn 2011-03-14, at 9:18 AM, Reid Kleckner wrote:/divbr class=Apple-interchange-newlineblockquote type=citespan class=Apple-style-span style=border-collapse: separate; font-family: Helvetica; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-border-horizontal-spacing: 0px; -webkit-border-vertical-spacing: 0px; -webkit-text-decorations-in-effect: none; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; font-size: medium; I updated and committed the patch to the cpython hg repo in revision [c4a0fa6e687c]./span/blockquote/divbrdivDoes this go to the main branch (py3.3) only? It is not clear from just looking atnbsp;a href=http://hg.python.o rg/cpython/rev/c4a0fa6e687c/http://hg.python.org/cpython/rev/c4a0fa6e687c//anbsp;/div/body/html___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
Reid Kleckner r...@mit.edu added the comment: On Mon, Mar 14, 2011 at 12:31 PM, Sridhar Ratnakumar rep...@bugs.python.org wrote: Sridhar Ratnakumar sridh...@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 -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5673 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
Changes by Reid Kleckner r...@mit.edu: -- status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5673 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
Changes by Sridhar Ratnakumar sridh...@activestate.com: Removed file: http://bugs.python.org/file21121/unnamed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5673 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
Reid Kleckner r...@mit.edu added the comment: 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. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5673 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
Changes by Antoine Pitrou pit...@free.fr: -- versions: +Python 3.3 -Python 3.2 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5673 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
Pablo Bitton pablo.bit...@gmail.com added the comment: Has anybody had a chance to look into the problem I encountered yet? Do you need more information? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5673 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
Pablo Bitton pablo.bit...@gmail.com added the comment: Has anybody had a chance to look into the problem I encountered yet? Do you need more information? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5673 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
Reid Kleckner r...@mit.edu added the comment: 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! =/ -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5673 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
Pablo Bitton pablo.bit...@gmail.com added the comment: 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. -- Added file: http://bugs.python.org/file18468/tcpdump error.txt ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5673 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
Pablo Bitton pablo.bit...@gmail.com added the comment: 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. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5673 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
Changes by Tim Golden m...@timgolden.me.uk: -- nosy: +tim.golden ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5673 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
Brian Curtin cur...@acm.org added the comment: 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. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5673 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
Changes by Pablo Bitton pablo.bit...@gmail.com: -- nosy: +Pablo.Bitton ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5673 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
Pablo Bitton pablo.bit...@gmail.com added the comment: 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. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5673 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
Brian Curtin cur...@acm.org added the comment: 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. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5673 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
Reid Kleckner r...@mit.edu added the comment: On Thu, Jul 22, 2010 at 9:05 AM, Alexander Belopolsky rep...@bugs.python.org wrote: Alexander Belopolsky belopol...@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 -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5673 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
Changes by Matthieu Labbé bugs.python@mattlabbe.com: -- nosy: +matthieu.labbe ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5673 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
Alexander Belopolsky belopol...@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 rep...@bugs.python.org http://bugs.python.org/issue5673 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
Alexander Belopolsky belopol...@users.sourceforge.net added the comment: s/Note that a result Fraction/Note that as a result, Fraction/ -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5673 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
Changes by Stefan Krah stefan-use...@bytereef.org: -- nosy: +gd2shoe, ragnar ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5673 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
Reid Kleckner r...@mit.edu added the comment: 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. -- Added file: http://bugs.python.org/file18101/subprocess-timeout-py3k-v7.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5673 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
Brian Curtin cur...@acm.org added the comment: Looks good to me. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5673 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
Brian Curtin cur...@acm.org added the comment: 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. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5673 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
Reid Kleckner r...@mit.edu added the comment: 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. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5673 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
Reid Kleckner r...@mit.edu added the comment: 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. -- Added file: http://bugs.python.org/file18042/subprocess-timeout-py3k-v6.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5673 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
Reid Kleckner r...@mit.edu added the comment:
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.
--
Added file: http://bugs.python.org/file18028/subprocess-timeout-v5.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue5673
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
Brian Curtin cur...@acm.org added the comment: 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. -- assignee: - rnk ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5673 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
Reid Kleckner r...@mit.edu added the comment: 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. -- Added file: http://bugs.python.org/file17994/subprocess-timeout-v4.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5673 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
Changes by Antoine Pitrou pit...@free.fr: -- nosy: +brian.curtin versions: -Python 2.7 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5673 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
Brian Curtin cur...@acm.org added the comment: 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. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5673 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
Dave Malcolm dmalc...@redhat.com added the comment: 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? -- nosy: +dmalcolm Added file: http://bugs.python.org/file17832/subprocess-timeout-v3.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5673 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
Changes by Filippo Giunchedi fgiunch...@gmail.com: -- nosy: +filippo ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5673 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
Antoine Pitrou pit...@free.fr added the comment: 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. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5673 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
Reid Kleckner r...@mit.edu added the comment:
- 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).
--
Added file: http://bugs.python.org/file16093/subprocess-timeout.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue5673
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
Changes by Antoine Pitrou pit...@free.fr: -- nosy: +astrand stage: test needed - patch review ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5673 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
Antoine Pitrou pit...@free.fr added the comment:
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
--
nosy: +pitrou
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue5673
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
Changes by Senthil Kumaran orsent...@gmail.com: -- nosy: +orsenthil ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5673 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
Changes by Lev Shamardin shamar...@gmail.com: -- nosy: +abbot ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5673 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
Changes by Thomas Guettler guet...@thomas-guettler.de: -- nosy: +guettli ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5673 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
Changes by Sridhar Ratnakumar sridh...@activestate.com: -- nosy: +srid ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5673 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
Sridhar Ratnakumar sridh...@activestate.com added the comment: See http://code.google.com/p/python-process/ for some ideas. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5673 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
Changes by R. David Murray rdmur...@bitdance.com: -- priority: - normal ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5673 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
Changes by R. David Murray rdmur...@bitdance.com: -- versions: +Python 3.2 -Python 3.1 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5673 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
R. David Murray rdmur...@bitdance.com added the comment: 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. -- nosy: +r.david.murray stage: - test needed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5673 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
New submission from Reid Kleckner r...@mit.edu: 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? -- components: Library (Lib) messages: 85256 nosy: rnk severity: normal status: open title: Add timeout option to subprocess.Popen type: feature request versions: Python 2.7, Python 3.1 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5673 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
Changes by Giampaolo Rodola' billiej...@users.sourceforge.net: -- nosy: +giampaolo.rodola ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5673 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
Reid Kleckner r...@mit.edu added the comment: 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. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5673 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5673] Add timeout option to subprocess.Popen
Reid Kleckner r...@mit.edu added the comment: 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. -- keywords: +patch Added file: http://bugs.python.org/file13592/subprocess-timeout.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5673 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
